From ac8fc08cc2ad5e8205745f9bd6676c6f957abe45 Mon Sep 17 00:00:00 2001 From: poma Date: Tue, 5 Nov 2019 00:04:22 +0300 Subject: [PATCH] use bytes32 for hashes --- cli.js | 4 +- contracts/MerkleTreeWithHistory.sol | 32 ++++++++-------- contracts/Mixer.sol | 16 ++++---- contracts/Mocks/MerkleTreeWithHistoryMock.sol | 4 +- test/ERC20Mixer.test.js | 22 +++++------ test/ETHMixer.test.js | 36 +++++++++--------- test/MerkleTreeWithHistory.test.js | 38 ++++++++++++------- 7 files changed, 81 insertions(+), 71 deletions(-) diff --git a/cli.js b/cli.js index 793072f..3a5dbc2 100755 --- a/cli.js +++ b/cli.js @@ -170,11 +170,11 @@ async function withdraw(note, receiver) { const tree = new merkleTree(MERKLE_TREE_HEIGHT, leaves) // Find current commitment in the tree - let depositEvent = events.find(e => e.returnValues.commitment.eq(paddedCommitment)) + let depositEvent = events.find(e => e.returnValues.commitment === paddedCommitment) let leafIndex = depositEvent ? depositEvent.returnValues.leafIndex : -1 // Validate that our data is correct - const isValidRoot = await mixer.methods.isKnownRoot(await tree.root()).call() + const isValidRoot = await mixer.methods.isKnownRoot(toHex(await tree.root())).call() const isSpent = await mixer.methods.isSpent(paddedNullifierHash).call() assert(isValidRoot === true) // Merkle tree assembled correctly assert(isSpent === false) // The note is not spent diff --git a/contracts/MerkleTreeWithHistory.sol b/contracts/MerkleTreeWithHistory.sol index 36161a4..da1fdfa 100644 --- a/contracts/MerkleTreeWithHistory.sol +++ b/contracts/MerkleTreeWithHistory.sol @@ -24,18 +24,18 @@ contract MerkleTreeWithHistory { // the following variables are made public for easier testing and debugging and // are not supposed to be accessed in regular code uint32 public constant ROOT_HISTORY_SIZE = 100; - uint256[ROOT_HISTORY_SIZE] public roots; + bytes32[ROOT_HISTORY_SIZE] public roots; uint32 public currentRootIndex = 0; uint32 public nextIndex = 0; - uint256[] public filledSubtrees; - uint256[] public zeros; + bytes32[] public filledSubtrees; + bytes32[] public zeros; constructor(uint32 _treeLevels) public { require(_treeLevels > 0, "_treeLevels should be greater than zero"); require(_treeLevels < 32, "_treeLevels should be less than 32"); levels = _treeLevels; - uint256 currentZero = ZERO_VALUE; + bytes32 currentZero = bytes32(ZERO_VALUE); zeros.push(currentZero); filledSubtrees.push(currentZero); @@ -51,24 +51,24 @@ contract MerkleTreeWithHistory { /** @dev Hash 2 tree leaves, returns MiMC(_left, _right) */ - function hashLeftRight(uint256 _left, uint256 _right) public pure returns (uint256) { - require(_left < FIELD_SIZE, "_left should be inside the field"); - require(_right < FIELD_SIZE, "_right should be inside the field"); - uint256 R = _left; + function hashLeftRight(bytes32 _left, bytes32 _right) public pure returns (bytes32) { + require(uint256(_left) < FIELD_SIZE, "_left should be inside the field"); + require(uint256(_right) < FIELD_SIZE, "_right should be inside the field"); + uint256 R = uint256(_left); uint256 C = 0; (R, C) = Hasher.MiMCSponge(R, C, 0); - R = addmod(R, _right, FIELD_SIZE); + R = addmod(R, uint256(_right), FIELD_SIZE); (R, C) = Hasher.MiMCSponge(R, C, 0); - return R; + return bytes32(R); } - function _insert(uint256 _leaf) internal returns(uint32 index) { + function _insert(bytes32 _leaf) internal returns(uint32 index) { uint32 currentIndex = nextIndex; require(currentIndex != uint32(2)**levels, "Merkle tree is full. No more leafs can be added"); nextIndex += 1; - uint256 currentLevelHash = _leaf; - uint256 left; - uint256 right; + bytes32 currentLevelHash = _leaf; + bytes32 left; + bytes32 right; for (uint32 i = 0; i < levels; i++) { if (currentIndex % 2 == 0) { @@ -94,7 +94,7 @@ contract MerkleTreeWithHistory { /** @dev Whether the root is present in the root history */ - function isKnownRoot(uint256 _root) public view returns(bool) { + function isKnownRoot(bytes32 _root) public view returns(bool) { if (_root == 0) { return false; } @@ -114,7 +114,7 @@ contract MerkleTreeWithHistory { /** @dev Returns the last root */ - function getLastRoot() public view returns(uint256) { + function getLastRoot() public view returns(bytes32) { return roots[currentRootIndex]; } } diff --git a/contracts/Mixer.sol b/contracts/Mixer.sol index c6e3f2a..22a9180 100644 --- a/contracts/Mixer.sol +++ b/contracts/Mixer.sol @@ -19,9 +19,9 @@ contract IVerifier { contract Mixer is MerkleTreeWithHistory { uint256 public denomination; - mapping(uint256 => bool) public nullifierHashes; + mapping(bytes32 => bool) public nullifierHashes; // we store all commitments just to prevent accidental deposits with the same commitment - mapping(uint256 => bool) public commitments; + mapping(bytes32 => bool) public commitments; IVerifier public verifier; // operator can @@ -35,8 +35,8 @@ contract Mixer is MerkleTreeWithHistory { _; } - event Deposit(uint256 indexed commitment, uint32 leafIndex, uint256 timestamp); - event Withdrawal(address to, uint256 nullifierHash, address indexed relayer, uint256 fee); + event Deposit(bytes32 indexed commitment, uint32 leafIndex, uint256 timestamp); + event Withdrawal(address to, bytes32 nullifierHash, address indexed relayer, uint256 fee); /** @dev The constructor @@ -61,7 +61,7 @@ contract Mixer is MerkleTreeWithHistory { @dev Deposit funds into mixer. The caller must send (for ETH) or approve (for ERC20) value equal to or `denomination` of this mixer. @param _commitment the note commitment, which is PedersenHash(nullifier + secret) */ - function deposit(uint256 _commitment) public payable { + function deposit(bytes32 _commitment) public payable { require(!isDepositsDisabled, "deposits are disabled"); require(!commitments[_commitment], "The commitment has been submitted"); @@ -83,11 +83,11 @@ contract Mixer is MerkleTreeWithHistory { - the receiver of funds - optional fee that goes to the transaction sender (usually a relay) */ - function withdraw(bytes memory _proof, uint256 _root, uint256 _nullifierHash, address payable _receiver, address payable _relayer, uint256 _fee, uint256 _refund) public payable { + function withdraw(bytes memory _proof, bytes32 _root, bytes32 _nullifierHash, address payable _receiver, address payable _relayer, uint256 _fee, uint256 _refund) public payable { 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 - require(verifier.verifyProof(_proof, [_root, _nullifierHash, uint256(_receiver), uint256(_relayer), _fee, _refund]), "Invalid withdraw proof"); + require(verifier.verifyProof(_proof, [uint256(_root), uint256(_nullifierHash), uint256(_receiver), uint256(_relayer), _fee, _refund]), "Invalid withdraw proof"); nullifierHashes[_nullifierHash] = true; _processWithdraw(_receiver, _relayer, _fee, _refund); @@ -98,7 +98,7 @@ 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 _nullifierHash) public view returns(bool) { + function isSpent(bytes32 _nullifierHash) public view returns(bool) { return nullifierHashes[_nullifierHash]; } diff --git a/contracts/Mocks/MerkleTreeWithHistoryMock.sol b/contracts/Mocks/MerkleTreeWithHistoryMock.sol index 33c37c8..43bfced 100644 --- a/contracts/Mocks/MerkleTreeWithHistoryMock.sol +++ b/contracts/Mocks/MerkleTreeWithHistoryMock.sol @@ -4,9 +4,9 @@ import '../MerkleTreeWithHistory.sol'; contract MerkleTreeWithHistoryMock is MerkleTreeWithHistory { - constructor (uint8 _treeLevels) MerkleTreeWithHistory(_treeLevels) public {} + constructor (uint32 _treeLevels) MerkleTreeWithHistory(_treeLevels) public {} - function insert(uint256 _leaf) public { + function insert(bytes32 _leaf) public { _insert(_leaf); } } diff --git a/test/ERC20Mixer.test.js b/test/ERC20Mixer.test.js index 8681682..f6cc04c 100644 --- a/test/ERC20Mixer.test.js +++ b/test/ERC20Mixer.test.js @@ -98,18 +98,18 @@ contract('ERC20Mixer', accounts => { describe('#deposit', () => { it('should work', async () => { - const commitment = 43 + const commitment = toFixedHex(43) await token.approve(mixer.address, tokenDenomination) let { logs } = await mixer.deposit(commitment, { from: sender }) logs[0].event.should.be.equal('Deposit') - logs[0].args.commitment.should.be.eq.BN(toBN(commitment)) - logs[0].args.leafIndex.should.be.eq.BN(toBN(0)) + logs[0].args.commitment.should.be.equal(commitment) + logs[0].args.leafIndex.should.be.eq.BN(0) }) it('should not allow to send ether on deposit', async () => { - const commitment = 43 + const commitment = toFixedHex(43) await token.approve(mixer.address, tokenDenomination) let error = await mixer.deposit(commitment, { from: sender, value: 1e6 }).should.be.rejected @@ -129,7 +129,7 @@ contract('ERC20Mixer', accounts => { // Uncomment to measure gas usage // let gas = await mixer.deposit.estimateGas(toBN(deposit.commitment.toString()), { from: user, gasPrice: '0' }) // console.log('deposit gas:', gas) - await mixer.deposit(toBN(deposit.commitment.toString()), { from: user, gasPrice: '0' }) + await mixer.deposit(toFixedHex(deposit.commitment), { from: user, gasPrice: '0' }) const balanceUserAfter = await token.balanceOf(user) balanceUserAfter.should.be.eq.BN(toBN(balanceUserBefore).sub(toBN(tokenDenomination))) @@ -163,7 +163,7 @@ contract('ERC20Mixer', accounts => { const ethBalanceOperatorBefore = await web3.eth.getBalance(operator) const ethBalanceRecieverBefore = await web3.eth.getBalance(toHex(receiver.toString())) const ethBalanceRelayerBefore = await web3.eth.getBalance(relayer) - let isSpent = await mixer.isSpent(input.nullifierHash.toString(16).padStart(66, '0x00000')) + let isSpent = await mixer.isSpent(toFixedHex(input.nullifierHash)) isSpent.should.be.equal(false) // Uncomment to measure gas usage // gas = await mixer.withdraw.estimateGas(proof, publicSignals, { from: relayer, gasPrice: '0' }) @@ -194,10 +194,10 @@ contract('ERC20Mixer', accounts => { ethBalanceRelayerAfter.should.be.eq.BN(toBN(ethBalanceRelayerBefore).sub(toBN(refund))) logs[0].event.should.be.equal('Withdrawal') - logs[0].args.nullifierHash.should.be.eq.BN(toBN(input.nullifierHash.toString())) + logs[0].args.nullifierHash.should.be.equal(toFixedHex(input.nullifierHash)) logs[0].args.relayer.should.be.eq.BN(relayer) logs[0].args.fee.should.be.eq.BN(feeBN) - isSpent = await mixer.isSpent(input.nullifierHash.toString(16).padStart(66, '0x00000')) + isSpent = await mixer.isSpent(toFixedHex(input.nullifierHash)) isSpent.should.be.equal(true) }) @@ -207,7 +207,7 @@ contract('ERC20Mixer', accounts => { await tree.insert(deposit.commitment) await token.mint(user, tokenDenomination) await token.approve(mixer.address, tokenDenomination, { from: user }) - await mixer.deposit(toBN(deposit.commitment.toString()), { from: user, gasPrice: '0' }) + await mixer.deposit(toFixedHex(deposit.commitment), { from: user, gasPrice: '0' }) const { root, path_elements, path_index } = await tree.path(0) @@ -270,7 +270,7 @@ contract('ERC20Mixer', accounts => { console.log('approve done') const allowanceUser = await usdtToken.allowance(user, mixer.address) console.log('allowanceUser', allowanceUser.toString()) - await mixer.deposit(toBN(deposit.commitment.toString()), { from: user, gasPrice: '0' }) + await mixer.deposit(toFixedHex(deposit.commitment), { from: user, gasPrice: '0' }) console.log('deposit done') const balanceUserAfter = await usdtToken.balanceOf(user) @@ -359,7 +359,7 @@ contract('ERC20Mixer', accounts => { console.log('balanceUserBefore', balanceUserBefore.toString()) await token.approve(mixer.address, tokenDenomination, { from: user }) console.log('approve done') - await mixer.deposit(toBN(deposit.commitment.toString()), { from: user, gasPrice: '0' }) + await mixer.deposit(toFixedHex(deposit.commitment), { from: user, gasPrice: '0' }) console.log('deposit done') const balanceUserAfter = await token.balanceOf(user) diff --git a/test/ETHMixer.test.js b/test/ETHMixer.test.js index bd897e5..4064f9b 100644 --- a/test/ETHMixer.test.js +++ b/test/ETHMixer.test.js @@ -103,23 +103,23 @@ contract('ETHMixer', accounts => { describe('#deposit', () => { it('should emit event', async () => { - let commitment = 42 + let commitment = toFixedHex(42) let { logs } = await mixer.deposit(commitment, { value, from: sender }) logs[0].event.should.be.equal('Deposit') - logs[0].args.commitment.should.be.eq.BN(toBN(commitment)) - logs[0].args.leafIndex.should.be.eq.BN(toBN(0)) + logs[0].args.commitment.should.be.equal(commitment) + logs[0].args.leafIndex.should.be.eq.BN(0) - commitment = 12; + commitment = toFixedHex(12); ({ logs } = await mixer.deposit(commitment, { value, from: accounts[2] })) logs[0].event.should.be.equal('Deposit') - logs[0].args.commitment.should.be.eq.BN(toBN(commitment)) - logs[0].args.leafIndex.should.be.eq.BN(toBN(1)) + logs[0].args.commitment.should.be.equal(commitment) + logs[0].args.leafIndex.should.be.eq.BN(1) }) it('should not deposit if disabled', async () => { - let commitment = 42; + let commitment = toFixedHex(42); (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.') @@ -134,7 +134,7 @@ contract('ETHMixer', accounts => { }) it('should throw if there is a such commitment', async () => { - const commitment = 42 + const commitment = toFixedHex(42) await mixer.deposit(commitment, { value, from: sender }).should.be.fulfilled const error = await mixer.deposit(commitment, { value, from: sender }).should.be.rejected error.reason.should.be.equal('The commitment has been submitted') @@ -196,7 +196,7 @@ contract('ETHMixer', accounts => { // Uncomment to measure gas usage // let gas = await mixer.deposit.estimateGas(toBN(deposit.commitment.toString()), { value, from: user, gasPrice: '0' }) // console.log('deposit gas:', gas) - await mixer.deposit(toBN(deposit.commitment.toString()), { value, from: user, gasPrice: '0' }) + await mixer.deposit(toFixedHex(deposit.commitment), { value, from: user, gasPrice: '0' }) const balanceUserAfter = await web3.eth.getBalance(user) balanceUserAfter.should.be.eq.BN(toBN(balanceUserBefore).sub(toBN(value))) @@ -228,7 +228,7 @@ contract('ETHMixer', accounts => { const balanceRelayerBefore = await web3.eth.getBalance(relayer) const balanceOperatorBefore = await web3.eth.getBalance(operator) const balanceRecieverBefore = await web3.eth.getBalance(toHex(receiver.toString())) - let isSpent = await mixer.isSpent(input.nullifierHash.toString(16).padStart(66, '0x00000')) + let isSpent = await mixer.isSpent(toFixedHex(input.nullifierHash)) isSpent.should.be.equal(false) // Uncomment to measure gas usage @@ -256,17 +256,17 @@ contract('ETHMixer', accounts => { logs[0].event.should.be.equal('Withdrawal') - logs[0].args.nullifierHash.should.be.eq.BN(toBN(input.nullifierHash.toString())) + logs[0].args.nullifierHash.should.be.equal(toFixedHex(input.nullifierHash)) logs[0].args.relayer.should.be.eq.BN(operator) logs[0].args.fee.should.be.eq.BN(feeBN) - isSpent = await mixer.isSpent(input.nullifierHash.toString(16).padStart(66, '0x00000')) + isSpent = await mixer.isSpent(toFixedHex(input.nullifierHash)) isSpent.should.be.equal(true) }) it('should prevent double spend', async () => { const deposit = generateDeposit() await tree.insert(deposit.commitment) - await mixer.deposit(toBN(deposit.commitment.toString()), { value, from: sender }) + await mixer.deposit(toFixedHex(deposit.commitment), { value, from: sender }) const { root, path_elements, path_index } = await tree.path(0) @@ -300,7 +300,7 @@ contract('ETHMixer', accounts => { it('should prevent double spend with overflow', async () => { const deposit = generateDeposit() await tree.insert(deposit.commitment) - await mixer.deposit(toBN(deposit.commitment.toString()), { value, from: sender }) + await mixer.deposit(toFixedHex(deposit.commitment), { value, from: sender }) const { root, path_elements, path_index } = await tree.path(0) @@ -333,7 +333,7 @@ contract('ETHMixer', accounts => { it('fee should be less or equal transfer value', async () => { const deposit = generateDeposit() await tree.insert(deposit.commitment) - await mixer.deposit(toBN(deposit.commitment.toString()), { value, from: sender }) + await mixer.deposit(toFixedHex(deposit.commitment), { value, from: sender }) const { root, path_elements, path_index } = await tree.path(0) const oneEtherFee = bigInt(1e18) // 1 ether @@ -367,7 +367,7 @@ contract('ETHMixer', accounts => { it('should throw for corrupted merkle tree root', async () => { const deposit = generateDeposit() await tree.insert(deposit.commitment) - await mixer.deposit(toBN(deposit.commitment.toString()), { value, from: sender }) + await mixer.deposit(toFixedHex(deposit.commitment), { value, from: sender }) const { root, path_elements, path_index } = await tree.path(0) @@ -402,7 +402,7 @@ contract('ETHMixer', accounts => { it('should reject with tampered public inputs', async () => { const deposit = generateDeposit() await tree.insert(deposit.commitment) - await mixer.deposit(toBN(deposit.commitment.toString()), { value, from: sender }) + await mixer.deposit(toFixedHex(deposit.commitment), { value, from: sender }) let { root, path_elements, path_index } = await tree.path(0) @@ -478,7 +478,7 @@ contract('ETHMixer', accounts => { it('should reject with non zero refund', async () => { const deposit = generateDeposit() await tree.insert(deposit.commitment) - await mixer.deposit(toBN(deposit.commitment.toString()), { value, from: sender }) + await mixer.deposit(toFixedHex(deposit.commitment), { value, from: sender }) const { root, path_elements, path_index } = await tree.path(0) diff --git a/test/MerkleTreeWithHistory.test.js b/test/MerkleTreeWithHistory.test.js index b10630e..dc019fc 100644 --- a/test/MerkleTreeWithHistory.test.js +++ b/test/MerkleTreeWithHistory.test.js @@ -12,6 +12,9 @@ const hasherContract = artifacts.require('./Hasher.sol') const MerkleTree = require('../lib/MerkleTree') const hasherImpl = require('../lib/MiMC') +const snarkjs = require('snarkjs') +const bigInt = snarkjs.bigInt + const { ETH_AMOUNT, MERKLE_TREE_HEIGHT } = process.env // eslint-disable-next-line no-unused-vars @@ -23,6 +26,13 @@ function BNArrayToStringArray(array) { return arrayToPrint } +function toFixedHex(number, length = 32) { + let str = bigInt(number).toString(16) + while (str.length < length * 2) str = '0' + str + str = '0x' + str + return str +} + contract('MerkleTreeWithHistory', accounts => { let merkleTreeWithHistory let hasherInstance @@ -51,9 +61,9 @@ contract('MerkleTreeWithHistory', accounts => { it('should initialize', async () => { const zeroValue = await merkleTreeWithHistory.ZERO_VALUE() const firstSubtree = await merkleTreeWithHistory.filledSubtrees(0) - firstSubtree.should.be.eq.BN(zeroValue) + firstSubtree.should.be.equal(toFixedHex(zeroValue)) const firstZero = await merkleTreeWithHistory.zeros(0) - firstZero.should.be.eq.BN(zeroValue) + firstZero.should.be.equal(toFixedHex(zeroValue)) }) }) @@ -72,7 +82,7 @@ contract('MerkleTreeWithHistory', accounts => { null, prefix, ) - await tree.insert('5') + await tree.insert(toFixedHex('5')) let { root, path_elements } = await tree.path(0) const calculated_root = hasher.hash(null, hasher.hash(null, '5', path_elements[0]), @@ -162,11 +172,11 @@ contract('MerkleTreeWithHistory', accounts => { let rootFromContract for (let i = 1; i < 11; i++) { - await merkleTreeWithHistory.insert(i, { from: sender }) + await merkleTreeWithHistory.insert(toFixedHex(i), { from: sender }) await tree.insert(i) let { root } = await tree.path(i - 1) rootFromContract = await merkleTreeWithHistory.getLastRoot() - root.should.be.equal(rootFromContract.toString()) + toFixedHex(root).should.be.equal(rootFromContract.toString()) } }) @@ -175,13 +185,13 @@ contract('MerkleTreeWithHistory', accounts => { const merkleTreeWithHistory = await MerkleTreeWithHistory.new(levels) for (let i = 0; i < 2**levels; i++) { - await merkleTreeWithHistory.insert(i+42).should.be.fulfilled + await merkleTreeWithHistory.insert(toFixedHex(i+42)).should.be.fulfilled } - let error = await merkleTreeWithHistory.insert(1337).should.be.rejected + let error = await merkleTreeWithHistory.insert(toFixedHex(1337)).should.be.rejected error.reason.should.be.equal('Merkle tree is full. No more leafs can be added') - error = await merkleTreeWithHistory.insert(1).should.be.rejected + error = await merkleTreeWithHistory.insert(toFixedHex(1)).should.be.rejected error.reason.should.be.equal('Merkle tree is full. No more leafs can be added') }) @@ -200,22 +210,22 @@ contract('MerkleTreeWithHistory', accounts => { let path for (let i = 1; i < 5; i++) { - await merkleTreeWithHistory.insert(i, { from: sender }).should.be.fulfilled + await merkleTreeWithHistory.insert(toFixedHex(i), { from: sender }).should.be.fulfilled await tree.insert(i) path = await tree.path(i - 1) - let isKnown = await merkleTreeWithHistory.isKnownRoot(path.root) + let isKnown = await merkleTreeWithHistory.isKnownRoot(toFixedHex(path.root)) isKnown.should.be.equal(true) } - await merkleTreeWithHistory.insert(42, { from: sender }).should.be.fulfilled + await merkleTreeWithHistory.insert(toFixedHex(42), { from: sender }).should.be.fulfilled // check outdated root - let isKnown = await merkleTreeWithHistory.isKnownRoot(path.root) + let isKnown = await merkleTreeWithHistory.isKnownRoot(toFixedHex(path.root)) isKnown.should.be.equal(true) }) it('should not return uninitialized roots', async () => { - await merkleTreeWithHistory.insert(42, { from: sender }).should.be.fulfilled - let isKnown = await merkleTreeWithHistory.isKnownRoot(0) + await merkleTreeWithHistory.insert(toFixedHex(42), { from: sender }).should.be.fulfilled + let isKnown = await merkleTreeWithHistory.isKnownRoot(toFixedHex(0)) isKnown.should.be.equal(false) }) })