From d019e48da363229facaa3eaca274e0bca361354f Mon Sep 17 00:00:00 2001 From: poma Date: Fri, 1 Nov 2019 04:11:08 +0300 Subject: [PATCH] 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