Git-Mirrors/tornado-core
1
0
Fork 0
mirror of https://github.com/tornadocash/tornado-core.git synced 2025-04-02 12:05:35 -04:00
Code Issues Packages Projects Releases Wiki Activity

Address audit comments

Browse Source 
- operator -> owner
 - Add VerifierChanged events
This commit is contained in:
Brian Li 2021-04-20 20:50:39 -07:00
parent bb71360374
commit 4d028922e6
6 changed files with 34 additions and 189 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
contracts/ERC20Tornado.sol
View File

@ -11,8 +11,8 @@
pragma solidity 0.5.17;
import "@openzeppelin/contracts/math/SafeMath.sol";
import "./Tornado.sol";
import "./SafeMath.sol";
contract ERC20Tornado is Tornado {
address public token;
@ -22,9 +22,9 @@ contract ERC20Tornado is Tornado {
IFeeManager _feeManager,
uint256 _denomination,
uint32 _merkleTreeHeight,
address _operator,
address _owner,
address _token
) Tornado(_verifier, _feeManager, _denomination, _merkleTreeHeight, _operator) public {
) Tornado(_verifier, _feeManager, _denomination, _merkleTreeHeight, _owner) public {
token = _token;
}

12
contracts/FeeManager.sol
View File

@ -13,25 +13,25 @@ contract FeeManager {
}
function setFeeTo(address _feeTo) external {
require(msg.sender == feeToSetter, 'FeeManager: FORBIDDEN');
require(_feeTo != address(0), 'FeeManager: new feeTo is the zero address');
require(msg.sender == feeToSetter, 'FeeManager: Sender not authorized to change feeTo');
require(_feeTo != address(0), 'FeeManager: New feeTo is the zero address');
feeTo = _feeTo;
}
function setFeeToSetter(address _feeToSetter) external {
require(msg.sender == feeToSetter, 'FeeManager: FORBIDDEN');
require(_feeToSetter != address(0), 'FeeManager: new feeToSetter is the zero address');
require(msg.sender == feeToSetter, 'FeeManager: Sender not authorized to change feeToSetter ');
require(_feeToSetter != address(0), 'FeeManager: New feeToSetter is the zero address');
feeToSetter = _feeToSetter;
}
function setProtocolFeeDivisor(uint256 _protocolFeeDivisor) external {
require(msg.sender == feeToSetter, 'FeeManager: FORBIDDEN');
require(msg.sender == feeToSetter, 'FeeManager: Sender not authorized to change protocolFeeDivisor');
require(_protocolFeeDivisor >= MIN_PROTOCOL_FEE_DIVISOR, 'FeeManager: Protocol fee too high');
protocolFeeDivisor = _protocolFeeDivisor;
}
function clearFee() external {
require(msg.sender == feeToSetter, 'FeeManager: FORBIDDEN');
require(msg.sender == feeToSetter, 'FeeManager: Sender not authorized to clear protocolFeeDivisor');
protocolFeeDivisor = 0;
}
}

157
contracts/SafeMath.sol
View File

@ -1,157 +0,0 @@
pragma solidity ^0.5.0;
// Source: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v2.5.0/contracts/math/SafeMath.sol
/**
* @dev Wrappers over Solidity's arithmetic operations with added overflow
* checks.
*
* Arithmetic operations in Solidity wrap on overflow. This can easily result
* in bugs, because programmers usually assume that an overflow raises an
* error, which is the standard behavior in high level programming languages.
* `SafeMath` restores this intuition by reverting the transaction when an
* operation overflows.
*
* Using this library instead of the unchecked operations eliminates an entire
* class of bugs, so it's recommended to use it always.
*/
library SafeMath {
/**
* @dev Returns the addition of two unsigned integers, reverting on
* overflow.
*
* Counterpart to Solidity's `+` operator.
*
* Requirements:
* - Addition cannot overflow.
*/
function add(uint256 a, uint256 b) internal pure returns (uint256) {
uint256 c = a + b;
require(c >= a, "SafeMath: addition overflow");
return c;
}
/**
* @dev Returns the subtraction of two unsigned integers, reverting on
* overflow (when the result is negative).
*
* Counterpart to Solidity's `-` operator.
*
* Requirements:
* - Subtraction cannot overflow.
*/
function sub(uint256 a, uint256 b) internal pure returns (uint256) {
return sub(a, b, "SafeMath: subtraction overflow");
}
/**
* @dev Returns the subtraction of two unsigned integers, reverting with custom message on
* overflow (when the result is negative).
*
* Counterpart to Solidity's `-` operator.
*
* Requirements:
* - Subtraction cannot overflow.
*
* _Available since v2.4.0._
*/
function sub(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
require(b <= a, errorMessage);
uint256 c = a - b;
return c;
}
/**
* @dev Returns the multiplication of two unsigned integers, reverting on
* overflow.
*
* Counterpart to Solidity's `*` operator.
*
* Requirements:
* - Multiplication cannot overflow.
*/
function mul(uint256 a, uint256 b) internal pure returns (uint256) {
// Gas optimization: this is cheaper than requiring 'a' not being zero, but the
// benefit is lost if 'b' is also tested.
// See: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/522
if (a == 0) {
return 0;
}
uint256 c = a * b;
require(c / a == b, "SafeMath: multiplication overflow");
return c;
}
/**
* @dev Returns the integer division of two unsigned integers. Reverts on
* division by zero. The result is rounded towards zero.
*
* Counterpart to Solidity's `/` operator. Note: this function uses a
* `revert` opcode (which leaves remaining gas untouched) while Solidity
* uses an invalid opcode to revert (consuming all remaining gas).
*
* Requirements:
* - The divisor cannot be zero.
*/
function div(uint256 a, uint256 b) internal pure returns (uint256) {
return div(a, b, "SafeMath: division by zero");
}
/**
* @dev Returns the integer division of two unsigned integers. Reverts with custom message on
* division by zero. The result is rounded towards zero.
*
* Counterpart to Solidity's `/` operator. Note: this function uses a
* `revert` opcode (which leaves remaining gas untouched) while Solidity
* uses an invalid opcode to revert (consuming all remaining gas).
*
* Requirements:
* - The divisor cannot be zero.
*
* _Available since v2.4.0._
*/
function div(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
// Solidity only automatically asserts when dividing by 0
require(b > 0, errorMessage);
uint256 c = a / b;
// assert(a == b * c + a % b); // There is no case in which this doesn't hold
return c;
}
/**
* @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo),
* Reverts when dividing by zero.
*
* Counterpart to Solidity's `%` operator. This function uses a `revert`
* opcode (which leaves remaining gas untouched) while Solidity uses an
* invalid opcode to revert (consuming all remaining gas).
*
* Requirements:
* - The divisor cannot be zero.
*/
function mod(uint256 a, uint256 b) internal pure returns (uint256) {
return mod(a, b, "SafeMath: modulo by zero");
}
/**
* @dev Returns the remainder of dividing two unsigned integers. (unsigned integer modulo),
* Reverts with custom message when dividing by zero.
*
* Counterpart to Solidity's `%` operator. This function uses a `revert`
* opcode (which leaves remaining gas untouched) while Solidity uses an
* invalid opcode to revert (consuming all remaining gas).
*
* Requirements:
* - The divisor cannot be zero.
*
* _Available since v2.4.0._
*/
function mod(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
require(b != 0, errorMessage);
return a % b;
}
}

34
contracts/Tornado.sol
View File

@ -1,7 +1,7 @@
pragma solidity 0.5.17;
import "./MerkleTreeWithHistory.sol";
import "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
import "./MerkleTreeWithHistory.sol";
contract IVerifier {
function verifyProof(bytes memory _proof, uint256[6] memory _input) public returns(bool);
@ -20,17 +20,18 @@ contract Tornado is MerkleTreeWithHistory, ReentrancyGuard {
IVerifier public verifier;
IFeeManager public feeManager;
// operator can update snark verification key
// after the final trusted setup ceremony operator rights are supposed to be transferred to zero address
address public operator;
modifier onlyOperator {
require(msg.sender == operator, "Only operator can call this function.");
// owner can update snark verification key
// after the final trusted setup ceremony owner rights are supposed to be transferred to zero address
address public owner;
modifier onlyOwner {
require(msg.sender == owner, "Only owner can call this function.");
_;
}
event Deposit(bytes32 indexed commitment, uint32 leafIndex, uint256 timestamp);
event Withdrawal(address to, bytes32 nullifierHash, address indexed relayer, uint256 fee);
event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);
event VerifierChanged(address indexed previousVerifier, address indexed newVerifier);
event EncryptedNote(address indexed sender, bytes encryptedNote);
/**
@ -38,19 +39,19 @@ contract Tornado is MerkleTreeWithHistory, ReentrancyGuard {
@param _verifier the address of SNARK verifier for this contract
@param _denomination transfer amount for each deposit
@param _merkleTreeHeight the height of deposits' Merkle Tree
@param _operator operator address (see operator comment above)
@param _owner owner address (see owner comment above)
*/
constructor(
IVerifier _verifier,
IFeeManager _feeManager,
uint256 _denomination,
uint32 _merkleTreeHeight,
address _operator
address _owner
) MerkleTreeWithHistory(_merkleTreeHeight) public {
require(_denomination > 0, "denomination should be greater than 0");
verifier = _verifier;
feeManager = _feeManager;
operator = _operator;
owner = _owner;
denomination = _denomination;
}
@ -110,16 +111,17 @@ contract Tornado is MerkleTreeWithHistory, ReentrancyGuard {
}
/**
@dev allow operator to update SNARK verification keys. This is needed to update keys after the final trusted setup ceremony is held.
After that operator rights are supposed to be transferred to zero address
@dev allow owner to update SNARK verification keys. This is needed to update keys after the final trusted setup ceremony is held.
After that owner rights are supposed to be transferred to zero address
*/
function updateVerifier(address _newVerifier) external onlyOperator {
function updateVerifier(address _newVerifier) external onlyOwner {
emit VerifierChanged(address(verifier), _newVerifier);
verifier = IVerifier(_newVerifier);
}
/** @dev operator can change his address */
function changeOperator(address _newOperator) external onlyOperator {
emit OwnershipTransferred(operator, _newOperator);
operator = _newOperator;
/** @dev owner can change his address */
function changeOwner(address _newOwner) external onlyOwner {
emit OwnershipTransferred(owner, _newOwner);
owner = _newOwner;
}
}

2
package-lock.json generated
View File

@ -1,5 +1,5 @@
{
"name": "circuits",
"name": "@poofcash/poof-core",
"version": "1.0.0",
"lockfileVersion": 1,
"requires": true,

12
test/FeeManager.test.js
View File

@ -17,7 +17,7 @@ contract('FeeManager', accounts => {
try {
await feeManager.setFeeTo(accounts[1], {from: accounts[1]})
} catch (e) {
expectErrorMessage(e, "FeeManager: FORBIDDEN")
expectErrorMessage(e, "FeeManager: Sender not authorized to change feeTo")
}
const feeTo = await feeManager.feeTo()
@ -34,7 +34,7 @@ contract('FeeManager', accounts => {
try {
await feeManager.setFeeTo(ZERO_ADDRESS, {from: accounts[0]})
} catch (e) {
expectErrorMessage(e, "FeeManager: new feeTo is the zero address")
expectErrorMessage(e, "FeeManager: New feeTo is the zero address")
}
const feeTo = await feeManager.feeTo()
@ -47,7 +47,7 @@ contract('FeeManager', accounts => {
try {
await feeManager.setFeeToSetter(accounts[1], {from: accounts[1]})
} catch (e) {
expectErrorMessage(e, "FeeManager: FORBIDDEN")
expectErrorMessage(e, "FeeManager: Sender not authorized to change feeToSetter ")
}
const feeToSetter = await feeManager.feeToSetter()
@ -68,7 +68,7 @@ contract('FeeManager', accounts => {
try {
await feeManager.setFeeToSetter(ZERO_ADDRESS, {from: accounts[0]})
} catch (e) {
expectErrorMessage(e, "FeeManager: new feeToSetter is the zero address")
expectErrorMessage(e, "FeeManager: New feeToSetter is the zero address")
}
const feeToSetter = await feeManager.feeToSetter()
@ -81,7 +81,7 @@ contract('FeeManager', accounts => {
try {
await feeManager.setProtocolFeeDivisor(200, {from: accounts[1]})
} catch (e) {
expectErrorMessage(e, "FeeManager: FORBIDDEN")
expectErrorMessage(e, "FeeManager: Sender not authorized to change protocolFeeDivisor")
}
const protocolFeeDivisor = await feeManager.protocolFeeDivisor()
@ -119,7 +119,7 @@ contract('FeeManager', accounts => {
try {
await feeManager.clearFee({from: accounts[1]})
} catch (e) {
expectErrorMessage(e, "FeeManager: FORBIDDEN")
expectErrorMessage(e, "FeeManager: Sender not authorized to clear protocolFeeDivisor")
}
const protocolFeeDivisor = await feeManager.protocolFeeDivisor()