diff --git a/contracts/apps/AppStorage.sol b/contracts/apps/AppStorage.sol index 1a7c060b8..c1c6c9c7e 100644 --- a/contracts/apps/AppStorage.sol +++ b/contracts/apps/AppStorage.sol @@ -11,15 +11,17 @@ import "../kernel/IKernel.sol"; contract AppStorage { using UnstructuredStorage for bytes32; - /* Hardcoded constants to save gas - bytes32 internal constant KERNEL_POSITION = keccak256("aragonOS.appStorage.kernel"); - bytes32 internal constant APP_ID_POSITION = keccak256("aragonOS.appStorage.appId"); + /* + * Hardcoded constants to save gas + * bytes32 internal constant KERNEL_POSITION = keccak256("aragonOS.appStorage.kernel"); + * bytes32 internal constant APP_ID_POSITION = keccak256("aragonOS.appStorage.appId"); + * bytes32 internal constant LAST_NONCE_POSITION_BASE = keccak256("aragonOS.appStorage.lastNonce"); + * bytes32 internal constant VOLATILE_SENDER_POSITION = keccak256("aragonOS.appStorage.volatile.sender"); */ bytes32 internal constant KERNEL_POSITION = 0x4172f0f7d2289153072b0a6ca36959e0cbe2efc3afe50fc81636caa96338137b; bytes32 internal constant APP_ID_POSITION = 0xd625496217aa6a3453eecb9c3489dc5a53e6c67b444329ea2b2cbc9ff547639b; - - bytes32 internal constant USED_NONCE_POSITION_BASE = keccak256("aragonOS.appStorage.usedNonce"); - bytes32 internal constant VOLATILE_SENDER_POSITION = keccak256("aragonOS.appStorage.volatile.sender"); + bytes32 internal constant LAST_NONCE_POSITION_BASE = 0x66c8c1e117f8d5835231a971a56ce0c7b70f9291340698a4263ada738d9269bd; + bytes32 internal constant VOLATILE_SENDER_POSITION = 0xd6486d5aa3dac4242db35dd7559408452252cf8050988dbc66956eaa315379ce; function kernel() public view returns (IKernel) { return IKernel(KERNEL_POSITION.getStorageAddress()); @@ -33,8 +35,8 @@ contract AppStorage { return VOLATILE_SENDER_POSITION.getStorageAddress(); } - function usedNonce(address _account, uint256 _nonce) public view returns (bool) { - return usedNoncePosition(_account, _nonce).getStorageBool(); + function lastNonce(address _account) public view returns (uint256) { + return lastNoncePosition(_account).getStorageUint256(); } function setKernel(IKernel _kernel) internal { @@ -49,11 +51,11 @@ contract AppStorage { VOLATILE_SENDER_POSITION.setStorageAddress(_sender); } - function setUsedNonce(address _account, uint256 _nonce, bool _used) internal { - return usedNoncePosition(_account, _nonce).setStorageBool(_used); + function setLastNonce(address _account, uint256 _lastNonce) internal { + return lastNoncePosition(_account).setStorageUint256(_lastNonce); } - function usedNoncePosition(address _account, uint256 _nonce) internal returns (bytes32) { - return keccak256(abi.encodePacked(USED_NONCE_POSITION_BASE, _account, _nonce)); + function lastNoncePosition(address _account) internal returns (bytes32) { + return keccak256(abi.encodePacked(LAST_NONCE_POSITION_BASE, _account)); } } diff --git a/contracts/relayer/RelayerAragonAppWithParameterizedSender.sol b/contracts/relayer/RelayerAragonAppWithParameterizedSender.sol index cf450863a..5a0243312 100644 --- a/contracts/relayer/RelayerAragonAppWithParameterizedSender.sol +++ b/contracts/relayer/RelayerAragonAppWithParameterizedSender.sol @@ -17,13 +17,13 @@ contract RelayerAragonAppWithParameterizedSender is BaseRelayer { function exec(address from, uint256 nonce, bytes calldata, bytes signature) external refundGas auth(OFF_CHAIN_RELAYER_SERVICE_ROLE) { assertValidTransaction(from, nonce, calldata, signature); - setUsedNonce(from, nonce, true); + setLastNonce(from, nonce); bool success = address(this).call(calldata); if (!success) revertForwardingError(); emit TransactionRelayed(from, address(this), nonce, calldata); } function isNonceUsed(address _account, uint256 _nonce) public view returns (bool) { - return usedNonce(_account, _nonce); + return lastNonce(_account) >= _nonce; } } diff --git a/contracts/relayer/RelayerAragonAppWithVolatileSender.sol b/contracts/relayer/RelayerAragonAppWithVolatileSender.sol index 6fac09731..b46fa0625 100644 --- a/contracts/relayer/RelayerAragonAppWithVolatileSender.sol +++ b/contracts/relayer/RelayerAragonAppWithVolatileSender.sol @@ -8,7 +8,7 @@ contract RelayerAragonAppWithVolatileSender is BaseRelayer { assertValidTransaction(from, nonce, calldata, signature); setVolatileStorageSender(from); - setUsedNonce(from, nonce, true); + setLastNonce(from, nonce); bool success = address(this).call(calldata); if (!success) revertForwardingError(); @@ -18,7 +18,7 @@ contract RelayerAragonAppWithVolatileSender is BaseRelayer { } function isNonceUsed(address _account, uint256 _nonce) public view returns (bool) { - return usedNonce(_account, _nonce); + return lastNonce(_account) >= _nonce; } function sender() internal view returns (address) { diff --git a/contracts/relayer/StandAloneRelayer.sol b/contracts/relayer/StandAloneRelayer.sol index 2ce9d0672..c14e7db72 100644 --- a/contracts/relayer/StandAloneRelayer.sol +++ b/contracts/relayer/StandAloneRelayer.sol @@ -4,18 +4,18 @@ import "./BaseRelayer.sol"; contract StandAloneRelayer is BaseRelayer { - mapping (address => mapping (uint256 => bool)) internal usedNonces; + mapping (address => uint256) internal lastUsedNonce; function relay(address from, address to, uint256 nonce, bytes calldata, bytes signature) external refundGas auth(OFF_CHAIN_RELAYER_SERVICE_ROLE) { assertValidTransaction(from, nonce, calldata, signature); - usedNonces[from][nonce] = true; + lastUsedNonce[from] = nonce; bool success = to.call(calldata); if (!success) revertForwardingError(); emit TransactionRelayed(from, to, nonce, calldata); } function isNonceUsed(address sender, uint256 nonce) public view returns (bool) { - return usedNonces[sender][nonce]; + return lastUsedNonce[sender] >= nonce; } } diff --git a/test/contracts/relayer/parameterized_relayed_app.js b/test/contracts/relayer/parameterized_relayed_app.js index 6320896f0..db18eda45 100644 --- a/test/contracts/relayer/parameterized_relayed_app.js +++ b/test/contracts/relayer/parameterized_relayed_app.js @@ -9,7 +9,7 @@ const SampleApp = artifacts.require('RelayedAragonAppWithParameterizedSenderMock const getEventArgument = (receipt, event, arg) => receipt.logs.filter(l => l.event === event)[0].args[arg] contract('ParameterizedRelayedApp', ([_, root, sender, vault, offChainRelayerService]) => { - let daoFactory, dao, acl, app, relayer, relayedTx, nonce = 0 + let daoFactory, dao, acl, app, relayer, relayedTx, nonce = 1 let kernelBase, aclBase, sampleAppBase, relayerBase let WRITING_ROLE, APP_MANAGER_ROLE, RELAYER_ROLE, OFF_CHAIN_RELAYER_SERVICE_ROLE @@ -67,7 +67,7 @@ contract('ParameterizedRelayedApp', ([_, root, sender, vault, offChainRelayerSer assert.equal((await app.read()).toString(), 10, 'app value does not match') }) - it('overloads a transaction with ~94k of gas', async () => { + it('overloads a transaction with ~78k of gas', async () => { const { receipt: { cumulativeGasUsed: relayedGasUsed } } = relayedTx const { receipt: { cumulativeGasUsed: nonRelayerGasUsed } } = await app.write(10, { from: sender }) @@ -76,6 +76,6 @@ contract('ParameterizedRelayedApp', ([_, root, sender, vault, offChainRelayerSer console.log('nonRelayerGasUsed:', nonRelayerGasUsed) console.log('gasOverload:', gasOverload) - assert.isBelow(gasOverload, 94000, 'relayed txs gas overload is higher than 94k') + assert.isBelow(gasOverload, 78000, 'relayed txs gas overload is higher than 78k') }) }) diff --git a/test/contracts/relayer/parameterized_relayer_app.js b/test/contracts/relayer/parameterized_relayer_app.js index 920945741..50cac8b38 100644 --- a/test/contracts/relayer/parameterized_relayer_app.js +++ b/test/contracts/relayer/parameterized_relayer_app.js @@ -8,7 +8,7 @@ const SampleApp = artifacts.require('RelayerAragonAppWithParameterizedSenderMock const getEventArgument = (receipt, event, arg) => receipt.logs.filter(l => l.event === event)[0].args[arg] contract('ParameterizedRelayerApp', ([_, root, sender, vault, offChainRelayerService]) => { - let daoFactory, dao, acl, app, relayedTx, nonce = 0 + let daoFactory, dao, acl, app, relayedTx, nonce = 1 let kernelBase, aclBase, sampleAppBase let WRITING_ROLE, APP_MANAGER_ROLE, OFF_CHAIN_RELAYER_SERVICE_ROLE diff --git a/test/contracts/relayer/volatile_relayed_app.js b/test/contracts/relayer/volatile_relayed_app.js index 2d66b3120..079e5c0ad 100644 --- a/test/contracts/relayer/volatile_relayed_app.js +++ b/test/contracts/relayer/volatile_relayed_app.js @@ -9,7 +9,7 @@ const SampleApp = artifacts.require('RelayedAragonAppWithVolatileSenderMock') const getEventArgument = (receipt, event, arg) => receipt.logs.filter(l => l.event === event)[0].args[arg] contract('VolatileRelayedApp', ([_, root, sender, vault, offChainRelayerService]) => { - let daoFactory, dao, acl, app, relayer, relayedTx, nonce = 0 + let daoFactory, dao, acl, app, relayer, relayedTx, nonce = 1 let kernelBase, aclBase, sampleAppBase, relayerBase let WRITING_ROLE, APP_MANAGER_ROLE, RELAYER_ROLE, OFF_CHAIN_RELAYER_SERVICE_ROLE @@ -68,7 +68,7 @@ contract('VolatileRelayedApp', ([_, root, sender, vault, offChainRelayerService] assert.equal((await app.read()).toString(), 10, 'app value does not match') }) - it('overloads a transaction with ~115k of gas', async () => { + it('overloads a transaction with ~99k of gas', async () => { const { receipt: { cumulativeGasUsed: relayedGasUsed } } = relayedTx const { receipt: { cumulativeGasUsed: nonRelayerGasUsed } } = await app.write(10, { from: sender }) @@ -77,6 +77,6 @@ contract('VolatileRelayedApp', ([_, root, sender, vault, offChainRelayerService] console.log('nonRelayerGasUsed:', nonRelayerGasUsed) console.log('gasOverload:', gasOverload) - assert.isBelow(gasOverload, 115000, 'relayed txs gas overload is higher than 115k') + assert.isBelow(gasOverload, 99000, 'relayed txs gas overload is higher than 99k') }) }) diff --git a/test/contracts/relayer/volatile_relayer_app.js b/test/contracts/relayer/volatile_relayer_app.js index fb6f34cac..3ab944bbb 100644 --- a/test/contracts/relayer/volatile_relayer_app.js +++ b/test/contracts/relayer/volatile_relayer_app.js @@ -8,7 +8,7 @@ const SampleApp = artifacts.require('RelayerAragonAppWithVolatileSenderMock') const getEventArgument = (receipt, event, arg) => receipt.logs.filter(l => l.event === event)[0].args[arg] contract('VolatileRelayerApp', ([_, root, sender, vault, offChainRelayerService]) => { - let daoFactory, dao, acl, app, relayedTx, nonce = 0 + let daoFactory, dao, acl, app, relayedTx, nonce = 1 let kernelBase, aclBase, sampleAppBase let WRITING_ROLE, APP_MANAGER_ROLE, OFF_CHAIN_RELAYER_SERVICE_ROLE @@ -56,7 +56,7 @@ contract('VolatileRelayerApp', ([_, root, sender, vault, offChainRelayerService] assert.equal((await app.read()).toString(), 10, 'app value does not match') }) - it('overloads a transaction with ~84k of gas', async () => { + it('overloads a transaction with ~83k of gas', async () => { const { receipt: { cumulativeGasUsed: relayedGasUsed } } = relayedTx const { receipt: { cumulativeGasUsed: nonRelayerGasUsed } } = await app.write(10, { from: sender }) @@ -65,6 +65,6 @@ contract('VolatileRelayerApp', ([_, root, sender, vault, offChainRelayerService] console.log('nonRelayerGasUsed:', nonRelayerGasUsed) console.log('gasOverload:', gasOverload) - assert.isBelow(gasOverload, 84000, 'relayed txs gas overload is higher than 84k') + assert.isBelow(gasOverload, 83000, 'relayed txs gas overload is higher than 83k') }) })