From bec65f217fcfc1863e2989b55cfd98690dcf11ae Mon Sep 17 00:00:00 2001 From: poma Date: Fri, 1 Nov 2019 03:54:29 +0300 Subject: [PATCH 01/18] allow fee equal to denomination --- contracts/Mixer.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/Mixer.sol b/contracts/Mixer.sol index 4a5b9ef..caffbcb 100644 --- a/contracts/Mixer.sol +++ b/contracts/Mixer.sol @@ -90,7 +90,7 @@ contract Mixer is MerkleTreeWithHistory { address payable relayer = address(input[3]); uint256 fee = input[4]; uint256 refund = input[5]; - require(fee < denomination, "Fee exceeds transfer value"); + require(fee <= denomination, "Fee exceeds transfer value"); require(!nullifierHashes[nullifierHash], "The note has been already spent"); require(isKnownRoot(root), "Cannot find your merkle root"); // Make sure to use a recent one From b578213b0c2e7b2867c1d4416bf1511b2357cdd6 Mon Sep 17 00:00:00 2001 From: poma Date: Sat, 2 Nov 2019 11:39:07 +0300 Subject: [PATCH 02/18] check that eth value in ERC20 deposit is zero --- contracts/ERC20Mixer.sol | 1 + test/ERC20Mixer.test.js | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/contracts/ERC20Mixer.sol b/contracts/ERC20Mixer.sol index 5941f0e..fdee640 100644 --- a/contracts/ERC20Mixer.sol +++ b/contracts/ERC20Mixer.sol @@ -28,6 +28,7 @@ contract ERC20Mixer is Mixer { } function _processDeposit() internal { + require(msg.value == 0, "ETH value is supposed to be 0 for ETH mixer"); safeErc20TransferFrom(msg.sender, address(this), denomination); } diff --git a/test/ERC20Mixer.test.js b/test/ERC20Mixer.test.js index 8e0d42d..8818a79 100644 --- a/test/ERC20Mixer.test.js +++ b/test/ERC20Mixer.test.js @@ -102,6 +102,14 @@ contract('ERC20Mixer', accounts => { logs[0].args.commitment.should.be.eq.BN(toBN(commitment)) logs[0].args.leafIndex.should.be.eq.BN(toBN(0)) }) + + it('should not allow to send ether on deposit', async () => { + const commitment = 43 + await token.approve(mixer.address, tokenDenomination) + + let error = await mixer.deposit(commitment, { from: sender, value: 1e6 }).should.be.rejected + error.reason.should.be.equal('ETH value is supposed to be 0 for ETH mixer') + }) }) describe('#withdraw', () => { From 1d258715e0ccb8ac19a81bab1c0d08adaadc12d5 Mon Sep 17 00:00:00 2001 From: poma Date: Fri, 1 Nov 2019 03:56:24 +0300 Subject: [PATCH 03/18] make operator not payable --- contracts/ERC20Mixer.sol | 2 +- contracts/ETHMixer.sol | 2 +- contracts/Mixer.sol | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/contracts/ERC20Mixer.sol b/contracts/ERC20Mixer.sol index fdee640..0a03c4d 100644 --- a/contracts/ERC20Mixer.sol +++ b/contracts/ERC20Mixer.sol @@ -21,7 +21,7 @@ contract ERC20Mixer is Mixer { uint256 _denomination, uint8 _merkleTreeHeight, uint256 _emptyElement, - address payable _operator, + address _operator, address _token ) Mixer(_verifier, _denomination, _merkleTreeHeight, _emptyElement, _operator) public { token = _token; diff --git a/contracts/ETHMixer.sol b/contracts/ETHMixer.sol index 44ebc60..b575ce5 100644 --- a/contracts/ETHMixer.sol +++ b/contracts/ETHMixer.sol @@ -19,7 +19,7 @@ contract ETHMixer is Mixer { uint256 _denomination, uint8 _merkleTreeHeight, uint256 _emptyElement, - address payable _operator + address _operator ) Mixer(_verifier, _denomination, _merkleTreeHeight, _emptyElement, _operator) public { } diff --git a/contracts/Mixer.sol b/contracts/Mixer.sol index caffbcb..619889e 100644 --- a/contracts/Mixer.sol +++ b/contracts/Mixer.sol @@ -28,7 +28,7 @@ contract Mixer is MerkleTreeWithHistory { // - receive a relayer fee // - disable new deposits in case of emergency // - update snark verification key until this ability is permanently disabled - address payable public operator; + address public operator; bool public isDepositsEnabled = true; bool public isVerifierUpdateAllowed = true; modifier onlyOperator { @@ -51,7 +51,7 @@ contract Mixer is MerkleTreeWithHistory { uint256 _denomination, uint8 _merkleTreeHeight, uint256 _emptyElement, - address payable _operator + address _operator ) MerkleTreeWithHistory(_merkleTreeHeight, _emptyElement) public { verifier = IVerifier(_verifier); operator = _operator; @@ -134,7 +134,7 @@ contract Mixer is MerkleTreeWithHistory { } /** @dev operator can change his address */ - function changeOperator(address payable _newAccount) external onlyOperator { + function changeOperator(address _newAccount) external onlyOperator { operator = _newAccount; } } From b8d22464e37c6690e54ae0485f3f0aeed0574920 Mon Sep 17 00:00:00 2001 From: poma Date: Fri, 1 Nov 2019 04:00:32 +0300 Subject: [PATCH 04/18] process deposit after changing state to prevent reentrancy --- contracts/Mixer.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/Mixer.sol b/contracts/Mixer.sol index 619889e..4f273ad 100644 --- a/contracts/Mixer.sol +++ b/contracts/Mixer.sol @@ -65,9 +65,9 @@ contract Mixer is MerkleTreeWithHistory { function deposit(uint256 commitment) public payable { require(isDepositsEnabled, "deposits are disabled"); require(!commitments[commitment], "The commitment has been submitted"); - _processDeposit(); _insert(commitment); commitments[commitment] = true; + _processDeposit(); emit Deposit(commitment, next_index - 1, block.timestamp); } From 54e2c5f890cafbf0fc6f9c34195168a0b4b88109 Mon Sep 17 00:00:00 2001 From: poma Date: Fri, 1 Nov 2019 04:07:11 +0300 Subject: [PATCH 05/18] return inserted index from merkle tree --- contracts/MerkleTreeWithHistory.sol | 3 ++- contracts/Mixer.sol | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/contracts/MerkleTreeWithHistory.sol b/contracts/MerkleTreeWithHistory.sol index 9d2f189..39145b9 100644 --- a/contracts/MerkleTreeWithHistory.sol +++ b/contracts/MerkleTreeWithHistory.sol @@ -56,7 +56,7 @@ contract MerkleTreeWithHistory { hash = R; } - function _insert(uint256 leaf) internal { + function _insert(uint256 leaf) internal returns(uint256 index) { uint32 current_index = next_index; require(current_index != 2**levels, "Merkle tree is full. No more leafs can be added"); next_index += 1; @@ -82,6 +82,7 @@ contract MerkleTreeWithHistory { current_root = (current_root + 1) % ROOT_HISTORY_SIZE; _roots[current_root] = current_level_hash; + return next_index - 1; } function isKnownRoot(uint256 root) public view returns(bool) { diff --git a/contracts/Mixer.sol b/contracts/Mixer.sol index 4f273ad..8da900e 100644 --- a/contracts/Mixer.sol +++ b/contracts/Mixer.sol @@ -65,11 +65,11 @@ contract Mixer is MerkleTreeWithHistory { function deposit(uint256 commitment) public payable { require(isDepositsEnabled, "deposits are disabled"); require(!commitments[commitment], "The commitment has been submitted"); - _insert(commitment); + uint256 insertedIndex = _insert(commitment); commitments[commitment] = true; _processDeposit(); - emit Deposit(commitment, next_index - 1, block.timestamp); + emit Deposit(commitment, insertedIndex, block.timestamp); } /** @dev this function is defined in a child contract */ From d019e48da363229facaa3eaca274e0bca361354f Mon Sep 17 00:00:00 2001 From: poma Date: Fri, 1 Nov 2019 04:11:08 +0300 Subject: [PATCH 06/18] inverted flags to reduce deploy cost, explicitly set state in toggleDeposits --- contracts/Mixer.sol | 14 +++++++------- test/ETHMixer.test.js | 28 ++++++++++++++++------------ 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/contracts/Mixer.sol b/contracts/Mixer.sol index 8da900e..11a0ad3 100644 --- a/contracts/Mixer.sol +++ b/contracts/Mixer.sol @@ -29,8 +29,8 @@ contract Mixer is MerkleTreeWithHistory { // - disable new deposits in case of emergency // - update snark verification key until this ability is permanently disabled address public operator; - bool public isDepositsEnabled = true; - bool public isVerifierUpdateAllowed = true; + bool public isDepositsDisabled; + bool public isVerifierUpdateDisabled; modifier onlyOperator { require(msg.sender == operator, "Only operator can call this function."); _; @@ -63,7 +63,7 @@ contract Mixer is MerkleTreeWithHistory { @param commitment the note commitment, which is PedersenHash(nullifier + secret) */ function deposit(uint256 commitment) public payable { - require(isDepositsEnabled, "deposits are disabled"); + require(!isDepositsDisabled, "deposits are disabled"); require(!commitments[commitment], "The commitment has been submitted"); uint256 insertedIndex = _insert(commitment); commitments[commitment] = true; @@ -112,8 +112,8 @@ contract Mixer is MerkleTreeWithHistory { @dev Allow operator to temporarily disable new deposits. This is needed to protect users funds in case a vulnerability is discovered. It does not affect existing deposits. */ - function toggleDeposits() external onlyOperator { - isDepositsEnabled = !isDepositsEnabled; + function toggleDeposits(bool _state) external onlyOperator { + isDepositsDisabled = _state; } /** @@ -121,7 +121,7 @@ contract Mixer is MerkleTreeWithHistory { After that operator is supposed to permanently disable this ability. */ function updateVerifier(address newVerifier) external onlyOperator { - require(isVerifierUpdateAllowed, "Verifier updates have been disabled."); + require(!isVerifierUpdateDisabled, "Verifier updates have been disabled."); verifier = IVerifier(newVerifier); } @@ -130,7 +130,7 @@ contract Mixer is MerkleTreeWithHistory { This is supposed to be called after the final trusted setup ceremony is held. */ function disableVerifierUpdate() external onlyOperator { - isVerifierUpdateAllowed = false; + isVerifierUpdateDisabled = true; } /** @dev operator can change his address */ diff --git a/test/ETHMixer.test.js b/test/ETHMixer.test.js index 2462b23..b1c00f2 100644 --- a/test/ETHMixer.test.js +++ b/test/ETHMixer.test.js @@ -115,11 +115,15 @@ contract('ETHMixer', accounts => { it('should not deposit if disabled', async () => { let commitment = 42; - (await mixer.isDepositsEnabled()).should.be.equal(true) - const err = await mixer.toggleDeposits({ from: accounts[1] }).should.be.rejected + (await mixer.isDepositsDisabled()).should.be.equal(false) + const err = await mixer.toggleDeposits(true, { from: accounts[1] }).should.be.rejected err.reason.should.be.equal('Only operator can call this function.') - await mixer.toggleDeposits({ from: sender }); - (await mixer.isDepositsEnabled()).should.be.equal(false) + await mixer.toggleDeposits(false, { from: sender }); + (await mixer.isDepositsDisabled()).should.be.equal(false) + await mixer.toggleDeposits(true, { from: sender }); + (await mixer.isDepositsDisabled()).should.be.equal(true) + await mixer.toggleDeposits(true, { from: sender }); + (await mixer.isDepositsDisabled()).should.be.equal(true) let error = await mixer.deposit(commitment, { value, from: sender }).should.be.rejected error.reason.should.be.equal('deposits are disabled') }) @@ -484,26 +488,26 @@ contract('ETHMixer', accounts => { let operator = await mixer.operator() operator.should.be.equal(sender) - let isVerifierUpdateAllowed = await mixer.isVerifierUpdateAllowed() - isVerifierUpdateAllowed.should.be.equal(true) + let isVerifierUpdateDisabled = await mixer.isVerifierUpdateDisabled() + isVerifierUpdateDisabled.should.be.equal(false) await mixer.disableVerifierUpdate().should.be.fulfilled - const newValue = await mixer.isVerifierUpdateAllowed() - newValue.should.be.equal(false) + const newValue = await mixer.isVerifierUpdateDisabled() + newValue.should.be.equal(true) }) it('cannot update verifier after this function is called', async () => { let operator = await mixer.operator() operator.should.be.equal(sender) - let isVerifierUpdateAllowed = await mixer.isVerifierUpdateAllowed() - isVerifierUpdateAllowed.should.be.equal(true) + let isVerifierUpdateDisabled = await mixer.isVerifierUpdateDisabled() + isVerifierUpdateDisabled.should.be.equal(false) await mixer.disableVerifierUpdate().should.be.fulfilled - const newValue = await mixer.isVerifierUpdateAllowed() - newValue.should.be.equal(false) + const newValue = await mixer.isVerifierUpdateDisabled() + newValue.should.be.equal(true) const newVerifier = accounts[7] const error = await mixer.updateVerifier(newVerifier).should.be.rejected From 02e76a1ce6b44958e050cdb9cd755d1dba6ce475 Mon Sep 17 00:00:00 2001 From: poma Date: Fri, 1 Nov 2019 04:14:01 +0300 Subject: [PATCH 07/18] pass verifier address as IVerifier --- contracts/ERC20Mixer.sol | 2 +- contracts/ETHMixer.sol | 2 +- contracts/Mixer.sol | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/ERC20Mixer.sol b/contracts/ERC20Mixer.sol index 0a03c4d..a7c6c58 100644 --- a/contracts/ERC20Mixer.sol +++ b/contracts/ERC20Mixer.sol @@ -17,7 +17,7 @@ contract ERC20Mixer is Mixer { address public token; constructor( - address _verifier, + IVerifier _verifier, uint256 _denomination, uint8 _merkleTreeHeight, uint256 _emptyElement, diff --git a/contracts/ETHMixer.sol b/contracts/ETHMixer.sol index b575ce5..56a9aed 100644 --- a/contracts/ETHMixer.sol +++ b/contracts/ETHMixer.sol @@ -15,7 +15,7 @@ import "./Mixer.sol"; contract ETHMixer is Mixer { constructor( - address _verifier, + IVerifier _verifier, uint256 _denomination, uint8 _merkleTreeHeight, uint256 _emptyElement, diff --git a/contracts/Mixer.sol b/contracts/Mixer.sol index 11a0ad3..b16383c 100644 --- a/contracts/Mixer.sol +++ b/contracts/Mixer.sol @@ -47,13 +47,13 @@ contract Mixer is MerkleTreeWithHistory { @param _operator operator address (see operator above) */ constructor( - address _verifier, + IVerifier _verifier, uint256 _denomination, uint8 _merkleTreeHeight, uint256 _emptyElement, address _operator ) MerkleTreeWithHistory(_merkleTreeHeight, _emptyElement) public { - verifier = IVerifier(_verifier); + verifier = _verifier; operator = _operator; denomination = _denomination; } From 111c966c1eb256a97998069d99c7ac9ff94283ac Mon Sep 17 00:00:00 2001 From: poma Date: Fri, 1 Nov 2019 04:15:55 +0300 Subject: [PATCH 08/18] add assert on denomination --- contracts/Mixer.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/contracts/Mixer.sol b/contracts/Mixer.sol index b16383c..d21e85d 100644 --- a/contracts/Mixer.sol +++ b/contracts/Mixer.sol @@ -53,6 +53,7 @@ contract Mixer is MerkleTreeWithHistory { uint256 _emptyElement, address _operator ) MerkleTreeWithHistory(_merkleTreeHeight, _emptyElement) public { + require(_denomination > 0, "denomination should be greater than 0"); verifier = _verifier; operator = _operator; denomination = _denomination; From 8e8243823a2939226e91f4f40c376cbbdd58f150 Mon Sep 17 00:00:00 2001 From: poma Date: Fri, 1 Nov 2019 04:17:00 +0300 Subject: [PATCH 09/18] make inheritable functions abstract --- contracts/Mixer.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/Mixer.sol b/contracts/Mixer.sol index d21e85d..760564c 100644 --- a/contracts/Mixer.sol +++ b/contracts/Mixer.sol @@ -74,7 +74,7 @@ contract Mixer is MerkleTreeWithHistory { } /** @dev this function is defined in a child contract */ - function _processDeposit() internal {} + function _processDeposit() internal; /** @dev Withdraw deposit from the mixer. `proof` is a zkSNARK proof data, and input is an array of circuit public inputs @@ -102,7 +102,7 @@ contract Mixer is MerkleTreeWithHistory { } /** @dev this function is defined in a child contract */ - function _processWithdraw(address payable _receiver, address payable _relayer, uint256 _fee, uint256 _refund) internal {} + function _processWithdraw(address payable _receiver, address payable _relayer, uint256 _fee, uint256 _refund) internal; /** @dev whether a note is already spent */ function isSpent(uint256 nullifier) public view returns(bool) { From 35500ac5bb83c0bff0a23e22001f4ac781c9ab6c Mon Sep 17 00:00:00 2001 From: poma Date: Sat, 2 Nov 2019 13:09:02 +0300 Subject: [PATCH 10/18] change arg name --- contracts/Mixer.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/Mixer.sol b/contracts/Mixer.sol index 760564c..310432f 100644 --- a/contracts/Mixer.sol +++ b/contracts/Mixer.sol @@ -105,8 +105,8 @@ contract Mixer is MerkleTreeWithHistory { function _processWithdraw(address payable _receiver, address payable _relayer, uint256 _fee, uint256 _refund) internal; /** @dev whether a note is already spent */ - function isSpent(uint256 nullifier) public view returns(bool) { - return nullifierHashes[nullifier]; + function isSpent(uint256 nullifierHash) public view returns(bool) { + return nullifierHashes[nullifierHash]; } /** From 48cc57fad7f0f18ae98e5f47054f335f0c9c33f8 Mon Sep 17 00:00:00 2001 From: poma Date: Fri, 1 Nov 2019 04:28:46 +0300 Subject: [PATCH 11/18] change arg name --- contracts/Mixer.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/Mixer.sol b/contracts/Mixer.sol index 310432f..1fd9466 100644 --- a/contracts/Mixer.sol +++ b/contracts/Mixer.sol @@ -135,7 +135,7 @@ contract Mixer is MerkleTreeWithHistory { } /** @dev operator can change his address */ - function changeOperator(address _newAccount) external onlyOperator { - operator = _newAccount; + function changeOperator(address _newOperator) external onlyOperator { + operator = _newOperator; } } From 2bb751bfd173686c0f8b6d55b01d1570c7851198 Mon Sep 17 00:00:00 2001 From: poma Date: Fri, 1 Nov 2019 04:30:07 +0300 Subject: [PATCH 12/18] change ROOT_HISTORY_SIZE type --- contracts/MerkleTreeWithHistory.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/MerkleTreeWithHistory.sol b/contracts/MerkleTreeWithHistory.sol index 39145b9..6fc6b3a 100644 --- a/contracts/MerkleTreeWithHistory.sol +++ b/contracts/MerkleTreeWithHistory.sol @@ -18,7 +18,7 @@ library Hasher { contract MerkleTreeWithHistory { uint256 public levels; - uint8 constant ROOT_HISTORY_SIZE = 100; + uint256 constant ROOT_HISTORY_SIZE = 100; uint256[] private _roots; uint256 public current_root = 0; From 1364762b930eca00cbee7d6307fcb73466780969 Mon Sep 17 00:00:00 2001 From: poma Date: Fri, 1 Nov 2019 04:31:57 +0300 Subject: [PATCH 13/18] make _roots constant sized array --- contracts/MerkleTreeWithHistory.sol | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/contracts/MerkleTreeWithHistory.sol b/contracts/MerkleTreeWithHistory.sol index 6fc6b3a..3ff3b0b 100644 --- a/contracts/MerkleTreeWithHistory.sol +++ b/contracts/MerkleTreeWithHistory.sol @@ -19,7 +19,7 @@ contract MerkleTreeWithHistory { uint256 public levels; uint256 constant ROOT_HISTORY_SIZE = 100; - uint256[] private _roots; + uint256[ROOT_HISTORY_SIZE] private _roots; uint256 public current_root = 0; uint256[] private _filled_subtrees; @@ -38,7 +38,6 @@ contract MerkleTreeWithHistory { _filled_subtrees.push(_zeros[i]); } - _roots = new uint256[](ROOT_HISTORY_SIZE); _roots[0] = hashLeftRight(_zeros[levels - 1], _zeros[levels - 1]); } @@ -122,7 +121,7 @@ contract MerkleTreeWithHistory { return _roots[current_root]; } - function roots() public view returns(uint256[] memory) { + function roots() public view returns(uint256[ROOT_HISTORY_SIZE] memory) { return _roots; } From 6571f54768cdf3b44f90a0bdce63dbff2d7a6b25 Mon Sep 17 00:00:00 2001 From: poma Date: Fri, 1 Nov 2019 04:32:26 +0300 Subject: [PATCH 14/18] make _roots public --- contracts/MerkleTreeWithHistory.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/MerkleTreeWithHistory.sol b/contracts/MerkleTreeWithHistory.sol index 3ff3b0b..8917fda 100644 --- a/contracts/MerkleTreeWithHistory.sol +++ b/contracts/MerkleTreeWithHistory.sol @@ -19,7 +19,7 @@ contract MerkleTreeWithHistory { uint256 public levels; uint256 constant ROOT_HISTORY_SIZE = 100; - uint256[ROOT_HISTORY_SIZE] private _roots; + uint256[ROOT_HISTORY_SIZE] public _roots; uint256 public current_root = 0; uint256[] private _filled_subtrees; From 609510654975487dcaf9be4dbe3588bd6f9f8534 Mon Sep 17 00:00:00 2001 From: poma Date: Fri, 1 Nov 2019 04:33:02 +0300 Subject: [PATCH 15/18] rename current root index --- contracts/MerkleTreeWithHistory.sol | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/contracts/MerkleTreeWithHistory.sol b/contracts/MerkleTreeWithHistory.sol index 8917fda..f294719 100644 --- a/contracts/MerkleTreeWithHistory.sol +++ b/contracts/MerkleTreeWithHistory.sol @@ -20,7 +20,7 @@ contract MerkleTreeWithHistory { uint256 constant ROOT_HISTORY_SIZE = 100; uint256[ROOT_HISTORY_SIZE] public _roots; - uint256 public current_root = 0; + uint256 public current_root_index = 0; uint256[] private _filled_subtrees; uint256[] private _zeros; @@ -79,8 +79,8 @@ contract MerkleTreeWithHistory { current_index /= 2; } - current_root = (current_root + 1) % ROOT_HISTORY_SIZE; - _roots[current_root] = current_level_hash; + current_root_index = (current_root_index + 1) % ROOT_HISTORY_SIZE; + _roots[current_root_index] = current_level_hash; return next_index - 1; } @@ -90,14 +90,14 @@ contract MerkleTreeWithHistory { } // search most recent first uint256 i; - for(i = current_root; i < 2**256 - 1; i--) { + for(i = current_root_index; i < 2**256 - 1; i--) { if (root == _roots[i]) { return true; } } // process the rest of roots - for(i = ROOT_HISTORY_SIZE - 1; i > current_root; i--) { + for(i = ROOT_HISTORY_SIZE - 1; i > current_root_index; i--) { if (root == _roots[i]) { return true; } @@ -118,7 +118,7 @@ contract MerkleTreeWithHistory { } function getLastRoot() public view returns(uint256) { - return _roots[current_root]; + return _roots[current_root_index]; } function roots() public view returns(uint256[ROOT_HISTORY_SIZE] memory) { From c47408ebd703bb68491c5d0db0b6bdef27c944cc Mon Sep 17 00:00:00 2001 From: poma Date: Sat, 2 Nov 2019 14:29:43 +0300 Subject: [PATCH 16/18] cache current_zero value to prevent excessive SLOADs --- contracts/MerkleTreeWithHistory.sol | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/contracts/MerkleTreeWithHistory.sol b/contracts/MerkleTreeWithHistory.sol index f294719..8a6b705 100644 --- a/contracts/MerkleTreeWithHistory.sol +++ b/contracts/MerkleTreeWithHistory.sol @@ -28,17 +28,20 @@ contract MerkleTreeWithHistory { uint32 public next_index = 0; constructor(uint256 tree_levels, uint256 zero_value) public { + require(tree_levels > 0, "tree_levels should be greater than zero"); levels = tree_levels; + uint256 current_zero = zero_value; _zeros.push(zero_value); - _filled_subtrees.push(_zeros[0]); + _filled_subtrees.push(current_zero); for (uint8 i = 1; i < levels; i++) { - _zeros.push(hashLeftRight(_zeros[i-1], _zeros[i-1])); - _filled_subtrees.push(_zeros[i]); + current_zero = hashLeftRight(current_zero, current_zero); + _zeros.push(current_zero); + _filled_subtrees.push(current_zero); } - _roots[0] = hashLeftRight(_zeros[levels - 1], _zeros[levels - 1]); + _roots[0] = hashLeftRight(current_zero, current_zero); } function hashLeftRight(uint256 left, uint256 right) public pure returns (uint256 hash) { From 91adb03131a986da19a976960e8094ed6535b214 Mon Sep 17 00:00:00 2001 From: poma Date: Sat, 2 Nov 2019 11:12:11 +0300 Subject: [PATCH 17/18] add solidity version to IUSDT --- contracts/Mocks/IUSDT.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contracts/Mocks/IUSDT.sol b/contracts/Mocks/IUSDT.sol index 8bd9ead..4425677 100644 --- a/contracts/Mocks/IUSDT.sol +++ b/contracts/Mocks/IUSDT.sol @@ -1,3 +1,5 @@ +pragma solidity ^0.5.8; + contract ERC20Basic { uint public _totalSupply; function totalSupply() public view returns (uint); From c92ac97ff277d5b3fa0f5aca2cae20fd41d1bae3 Mon Sep 17 00:00:00 2001 From: poma Date: Sat, 2 Nov 2019 13:19:06 +0300 Subject: [PATCH 18/18] make field_size constant, return extra addmod, refactor return --- contracts/MerkleTreeWithHistory.sol | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/contracts/MerkleTreeWithHistory.sol b/contracts/MerkleTreeWithHistory.sol index 8a6b705..0deb5a7 100644 --- a/contracts/MerkleTreeWithHistory.sol +++ b/contracts/MerkleTreeWithHistory.sol @@ -18,6 +18,7 @@ library Hasher { contract MerkleTreeWithHistory { uint256 public levels; + uint256 constant FIELD_SIZE = 21888242871839275222246405745257275088548364400416034343698204186575808495617; uint256 constant ROOT_HISTORY_SIZE = 100; uint256[ROOT_HISTORY_SIZE] public _roots; uint256 public current_root_index = 0; @@ -45,17 +46,15 @@ contract MerkleTreeWithHistory { } function hashLeftRight(uint256 left, uint256 right) public pure returns (uint256 hash) { - uint256 k = 21888242871839275222246405745257275088548364400416034343698204186575808495617; - uint256 R = 0; + uint256 R = left; // left is already checked to be less than field_size by snark verifier uint256 C = 0; - R = addmod(R, left, k); (R, C) = Hasher.MiMCSponge(R, C, 0); - R = addmod(R, right, k); + R = addmod(R, right, FIELD_SIZE); (R, C) = Hasher.MiMCSponge(R, C, 0); - hash = R; + return R; } function _insert(uint256 leaf) internal returns(uint256 index) {