# Tornado lottery period audit

###### 6 October 2021, Igor Gulamov

## Introduction

Igor Gulamov conducted the audit of tornado.cash smart contracts and circuits.

This review was performed by an independent reviewer under fixed rate.

## Scope

Solidity contracts from [tornado-lottery-period](https://github.com/h-ivor/tornado-lottery-period/tree/d635cdd5d0b7f77c8a43be88842d1a06de5fd262/contracts) excluding `LotteryRandomNumberConsumer.sol`, `TornadoLottery.sol`.

## Issues

We find 1 major and no critical issues. Major issue is fixed.

We consider commit [2c592962e4266bf718bf9a73e90c4e924b69241d](https://github.com/h-ivor/tornado-lottery-period/tree/2c592962e4266bf718bf9a73e90c4e924b69241d) as a safe version from the informational security point of view.

### Major

##### [GovernanceGasUpgrade.sol#L55](https://github.com/h-ivor/tornado-lottery-period/blob/d635cdd5d0b7f77c8a43be88842d1a06de5fd262/contracts/gas/GovernanceGasUpgrade.sol#L55)

Malicious users can burn out gas compensations, replaying castDelegateVote without voting by themselves.

**[Fixed, method:](https://github.com/h-ivor/tornado-lottery-period/blob/de706a5da782b98e686eb23b361041a6bd8d5ec4/contracts/gas/GovernanceGasUpgrade.sol#L148-L153)** In order to assure that delegator votes are used once and to compensate for the call of castDelegateVote in one go, instead of applying the modifier multiple times we require that each address has not voted on the proposal, [otherwise revert](https://github.com/h-ivor/tornado-lottery-period/blob/de706a5da782b98e686eb23b361041a6bd8d5ec4/contracts/gas/GovernanceGasUpgrade.sol#L151). It is possible for the delegatee to call the function again and change votes, but this will not be compensated. The delegatee gives notice to the function that the second time around he is not demanding gasCompensations by including himself, as he did in the first run, in the from array and showing that he has voted, meaning he is not eligible for compensations, this can be seen [here](https://github.com/h-ivor/tornado-lottery-period/blob/de706a5da782b98e686eb23b361041a6bd8d5ec4/contracts/gas/GovernanceGasUpgrade.sol#L91).

### Warnings

#### [GasCompensationVault.sol#L28](https://github.com/h-ivor/tornado-lottery-period/blob/d635cdd5d0b7f77c8a43be88842d1a06de5fd262/contracts/gas/GasCompensationVault.sol#L28)

We propose caching the balance for gas optimization.

**[Implemented.](https://github.com/h-ivor/tornado-lottery-period/blob/de706a5da782b98e686eb23b361041a6bd8d5ec4/contracts/gas/GasCompensationVault.sol#L40)**

#### [GasCompensator.sol#L32](https://github.com/h-ivor/tornado-lottery-period/blob/d635cdd5d0b7f77c8a43be88842d1a06de5fd262/contracts/gas/GasCompensator.sol#L32)

We recommend moving all `_baseFee()` logic into `gasCompensationVault.compensateGas` to reduce number of internal transactions.

**[Implemented.](https://github.com/h-ivor/tornado-lottery-period/commit/2c592962e4266bf718bf9a73e90c4e924b69241d)**

### Comments

#### [GasCompensationVault.sol#L24](https://github.com/h-ivor/tornado-lottery-period/blob/d635cdd5d0b7f77c8a43be88842d1a06de5fd262/contracts/gas/GasCompensationVault.sol#L24)

Failture condition is ignored here. We recommend to handle failture condition as boolean return value.

**Not implemented:** we choose to gracefully downgrade the function in case of a failed transfer as to not impact voting in case of transfer failure for any reason.

#### [VaultAndGasProposal.sol#L34](https://github.com/h-ivor/tornado-lottery-period/blob/d635cdd5d0b7f77c8a43be88842d1a06de5fd262/contracts/VaultAndGasProposal.sol#L34)

We propose using `returnPayableGovernance()` here for better readability.

**[Implemented.](https://github.com/h-ivor/tornado-lottery-period/blob/de706a5da782b98e686eb23b361041a6bd8d5ec4/contracts/VaultAndGasProposal.sol#L42)**

## Severity Terms

### Comment

Comment issues are generally subjective in nature, or potentially deal with topics like "best practices" or "readability". Comment issues in general will not indicate an actual problem or bug in code.

The maintainers should use their own judgment as to whether addressing these issues improves the codebase.

### Warning

Warning issues are generally objective in nature but do not represent actual bugs or security problems.

These issues should be addressed unless there is a clear reason not to.

### Major

Major issues will be things like bugs or security vulnerabilities. These issues may not be directly exploitable, or may require a certain condition to arise in order to be exploited.

Left unaddressed these issues are highly likely to cause problems with the operation of the contract or lead to a situation which allows the system to be exploited in some way.

### Critical

Critical issues are directly exploitable bugs or security vulnerabilities.

Left unaddressed these issues are highly likely or guaranteed to cause major problems or potentially a full failure in the operations of the contract.