From c1c253a41ab14fe22c3d4c4e3aeddaa23d29d71e Mon Sep 17 00:00:00 2001 From: cbrzn Date: Mon, 28 Jun 2021 02:15:54 +0200 Subject: [PATCH 1/3] feat: initializable delay module --- contracts/DelayModule.sol | 18 ++++++++++++++---- contracts/test/DelayModuleMock.sol | 10 ++++++++++ test/DelayModule.spec.ts | 22 +++++++++++++--------- 3 files changed, 37 insertions(+), 13 deletions(-) create mode 100644 contracts/test/DelayModuleMock.sol diff --git a/contracts/DelayModule.sol b/contracts/DelayModule.sol index 39d63bd..d4fc05c 100644 --- a/contracts/DelayModule.sol +++ b/contracts/DelayModule.sol @@ -19,7 +19,7 @@ interface Executor { } contract DelayModule { - + event DelayModuleSetup(address indexed initiator, address indexed safe); event TransactionAdded( uint indexed queueNonce, bytes32 indexed txHash, @@ -32,7 +32,7 @@ contract DelayModule { event EnabledModule(address module); event DisabledModule(address module); - Executor public immutable executor; + Executor public executor; uint256 public txCooldown; uint256 public txExpiration; uint256 public txNonce; @@ -44,17 +44,27 @@ contract DelayModule { // Mapping of approved modules mapping(address => bool) public modules; + bool public isInitialized = false; + + constructor() { + isInitialized = true; + } + /// @param _executor Address of the executor (e.g. a Safe) /// @param cooldown Cooldown in seconds that should be required after a transaction is proposed /// @param expiration Duration that a proposed transaction is valid for after the cooldown, in seconds (or 0 if valid forever) /// @notice There need to be at least 60 seconds between end of cooldown and expiration - constructor(Executor _executor, uint256 cooldown, uint256 expiration) { + function setUp(Executor _executor, uint256 cooldown, uint256 expiration) external { + require(!isInitialized, "Module is already initialized"); require(cooldown > 0, "Cooldown must to be greater than 0"); require(expiration == 0 || expiration >= 60 , "Expiratition must be 0 or at least 60 seconds"); executor = _executor; txExpiration = expiration; txCooldown = cooldown; - } + isInitialized = true; + + emit DelayModuleSetup(msg.sender, address(_executor)); +} modifier executorOnly() { require(msg.sender == address(executor), "Not authorized"); diff --git a/contracts/test/DelayModuleMock.sol b/contracts/test/DelayModuleMock.sol new file mode 100644 index 0000000..6d02fdb --- /dev/null +++ b/contracts/test/DelayModuleMock.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: LGPL-3.0-only +pragma solidity >=0.8.0; + +import "../DelayModule.sol"; + +contract DelayModuleMock is DelayModule { + constructor() DelayModule() { + isInitialized = false; + } +} diff --git a/test/DelayModule.spec.ts b/test/DelayModule.spec.ts index 8c46409..fff51d5 100644 --- a/test/DelayModule.spec.ts +++ b/test/DelayModule.spec.ts @@ -17,31 +17,35 @@ describe("DelayModule", async () => { const setupTestWithTestExecutor = deployments.createFixture(async () => { const base = await baseSetup(); - const Module = await hre.ethers.getContractFactory("DelayModule"); - const module = await Module.deploy(base.executor.address, 42, "0x1337"); + const Module = await hre.ethers.getContractFactory("DelayModuleMock"); + const module = await Module.deploy(); + await module.setUp(base.executor.address, 42, "0x1337"); return { ...base, Module, module }; }) const [user1] = waffle.provider.getWallets(); - describe("constructor()", async () => { + describe("setUp()", async () => { it("throws if cooldown is 0", async () => { - const Module = await hre.ethers.getContractFactory("DelayModule") + const Module = await hre.ethers.getContractFactory("DelayModuleMock") + const module = await Module.deploy() await expect( - Module.deploy(user1.address, 0, 0) + module.setUp(user1.address, 0, 0) ).to.be.revertedWith("Cooldown must to be greater than 0") }) it("throws if not enough time between txCooldown and txExpiration", async () => { - const Module = await hre.ethers.getContractFactory("DelayModule") + const Module = await hre.ethers.getContractFactory("DelayModuleMock") + const module = await Module.deploy() await expect( - Module.deploy(user1.address, 1, 59) + module.setUp(user1.address, 1, 59) ).to.be.revertedWith("Expiratition must be 0 or at least 60 seconds") }) it("txExpiration can be 0", async () => { - const Module = await hre.ethers.getContractFactory("DelayModule") - await Module.deploy(user1.address, 1, 0) + const Module = await hre.ethers.getContractFactory("DelayModuleMock") + const module = await Module.deploy() + await module.setUp(user1.address, 1, 0) }) }) From 86ae35f1688e30a5bf6c299a307bb5a6841ac825 Mon Sep 17 00:00:00 2001 From: Auryn Macmillan Date: Thu, 1 Jul 2021 14:48:12 -0400 Subject: [PATCH 2/3] Removed minimum cooldown and DelayModuleMock --- contracts/DelayModule.sol | 405 +++++++++++++++-------------- contracts/test/DelayModuleMock.sol | 10 - test/DelayModule.spec.ts | 325 ++++++++++++----------- 3 files changed, 383 insertions(+), 357 deletions(-) delete mode 100644 contracts/test/DelayModuleMock.sol diff --git a/contracts/DelayModule.sol b/contracts/DelayModule.sol index d4fc05c..3a1a704 100644 --- a/contracts/DelayModule.sol +++ b/contracts/DelayModule.sol @@ -2,213 +2,218 @@ pragma solidity >=0.8.0; contract Enum { - enum Operation { - Call, DelegateCall - } + enum Operation {Call, DelegateCall} } interface Executor { - /// @dev Allows a Module to execute a transaction. - /// @param to Destination address of module transaction. - /// @param value Ether value of module transaction. - /// @param data Data payload of module transaction. - /// @param operation Operation type of module transaction. - function execTransactionFromModule(address to, uint256 value, bytes calldata data, Enum.Operation operation) - external - returns (bool success); + /// @dev Allows a Module to execute a transaction. + /// @param to Destination address of module transaction. + /// @param value Ether value of module transaction. + /// @param data Data payload of module transaction. + /// @param operation Operation type of module transaction. + function execTransactionFromModule( + address to, + uint256 value, + bytes calldata data, + Enum.Operation operation + ) external returns (bool success); } contract DelayModule { - event DelayModuleSetup(address indexed initiator, address indexed safe); - event TransactionAdded( - uint indexed queueNonce, - bytes32 indexed txHash, - address to, - uint256 value, - bytes data, - Enum.Operation operation - ); - - event EnabledModule(address module); - event DisabledModule(address module); - - Executor public executor; - uint256 public txCooldown; - uint256 public txExpiration; - uint256 public txNonce; - uint256 public queueNonce; - // Mapping of queue nonce to transaction hash. - mapping(uint256 => bytes32) public txHash; - // Mapping of queue nonce to creation timestamp. - mapping(uint256 => uint256) public txCreatedAt; - // Mapping of approved modules - mapping(address => bool) public modules; - - bool public isInitialized = false; - - constructor() { - isInitialized = true; - } - - /// @param _executor Address of the executor (e.g. a Safe) - /// @param cooldown Cooldown in seconds that should be required after a transaction is proposed - /// @param expiration Duration that a proposed transaction is valid for after the cooldown, in seconds (or 0 if valid forever) - /// @notice There need to be at least 60 seconds between end of cooldown and expiration - function setUp(Executor _executor, uint256 cooldown, uint256 expiration) external { - require(!isInitialized, "Module is already initialized"); - require(cooldown > 0, "Cooldown must to be greater than 0"); - require(expiration == 0 || expiration >= 60 , "Expiratition must be 0 or at least 60 seconds"); - executor = _executor; - txExpiration = expiration; - txCooldown = cooldown; - isInitialized = true; - - emit DelayModuleSetup(msg.sender, address(_executor)); -} + event DelayModuleSetup(address indexed initiator, address indexed safe); + event TransactionAdded( + uint256 indexed queueNonce, + bytes32 indexed txHash, + address to, + uint256 value, + bytes data, + Enum.Operation operation + ); + + event EnabledModule(address module); + event DisabledModule(address module); + + Executor public executor; + uint256 public txCooldown; + uint256 public txExpiration; + uint256 public txNonce; + uint256 public queueNonce; + // Mapping of queue nonce to transaction hash. + mapping(uint256 => bytes32) public txHash; + // Mapping of queue nonce to creation timestamp. + mapping(uint256 => uint256) public txCreatedAt; + // Mapping of approved modules + mapping(address => bool) public modules; + + bool public isInitialized = false; + + /// @param _executor Address of the executor (e.g. a Safe) + /// @param cooldown Cooldown in seconds that should be required after a transaction is proposed + /// @param expiration Duration that a proposed transaction is valid for after the cooldown, in seconds (or 0 if valid forever) + /// @notice There need to be at least 60 seconds between end of cooldown and expiration + function setUp( + Executor _executor, + uint256 cooldown, + uint256 expiration + ) external { + require(!isInitialized, "Module is already initialized"); + require( + expiration == 0 || expiration >= 60, + "Expiratition must be 0 or at least 60 seconds" + ); + executor = _executor; + txExpiration = expiration; + txCooldown = cooldown; + isInitialized = true; + + emit DelayModuleSetup(msg.sender, address(_executor)); + } + + modifier executorOnly() { + require(msg.sender == address(executor), "Not authorized"); + _; + } + + modifier moduleOnly() { + require(modules[msg.sender], "Module not authorized"); + _; + } + + /// @dev Disables a module that can add transactions to the queue + /// @param module Address of the module to be disabled + /// @notice This can only be called by the executor + function disableModule(address module) public executorOnly() { + require(modules[module] != false, "Module already disabled"); + modules[module] = false; + emit DisabledModule(module); + } + + /// @dev Enables a module that can add transactions to the queue + /// @param module Address of the module to be enabled + /// @notice This can only be called by the executor + function enableModule(address module) public executorOnly() { + require(modules[module] != true, "Module already enabled"); + modules[module] = true; + emit EnabledModule(module); + } + + /// @dev Sets the cooldown before a transaction can be executed. + /// @param cooldown Cooldown in seconds that should be required before the transaction can be executed + /// @notice This can only be called by the executor + function setTxCooldown(uint256 cooldown) public executorOnly() { + txCooldown = cooldown; + } + + /// @dev Sets the duration for which a transaction is valid. + /// @param expiration Duration that a transaction is valid in seconds (or 0 if valid forever) after the cooldown + /// @notice There need to be at least 60 seconds between end of cooldown and expiration + /// @notice This can only be called by the executor + function setTxExpiration(uint256 expiration) public executorOnly() { + require( + expiration == 0 || expiration >= 60, + "Expiratition must be 0 or at least 60 seconds" + ); + txExpiration = expiration; + } - modifier executorOnly() { - require(msg.sender == address(executor), "Not authorized"); - _; - } - - modifier moduleOnly() { - require(modules[msg.sender], "Module not authorized"); - _; - } - - /// @dev Disables a module that can add transactions to the queue - /// @param module Address of the module to be disabled - /// @notice This can only be called by the executor - function disableModule(address module) - public - executorOnly() - { - require(modules[module] != false, "Module already disabled"); - modules[module] = false; - emit DisabledModule(module); - } - - /// @dev Enables a module that can add transactions to the queue - /// @param module Address of the module to be enabled - /// @notice This can only be called by the executor - function enableModule(address module) - public - executorOnly() - { - require(modules[module] != true, "Module already enabled"); - modules[module] = true; - emit EnabledModule(module); - } - - /// @dev Sets the cooldown before a transaction can be executed. - /// @param cooldown Cooldown in seconds that should be required before the transaction can be executed - /// @notice This can only be called by the executor - function setTxCooldown(uint256 cooldown) - public - executorOnly() - { - txCooldown = cooldown; - } - - /// @dev Sets the duration for which a transaction is valid. - /// @param expiration Duration that a transaction is valid in seconds (or 0 if valid forever) after the cooldown - /// @notice There need to be at least 60 seconds between end of cooldown and expiration - /// @notice This can only be called by the executor - function setTxExpiration(uint256 expiration) - public - executorOnly() - { - require(expiration == 0 || expiration >= 60 , "Expiratition must be 0 or at least 60 seconds"); - txExpiration = expiration; - } - - /// @dev Sets transaction nonce. Used to invalidate or skip transactions in queue. - /// @param _nonce New transaction nonce - /// @notice This can only be called by the executor - function setTxNonce(uint256 _nonce) - public - executorOnly() - { - require(_nonce > txNonce, "New nonce must be higher than current txNonce"); - require(_nonce <= queueNonce, "Cannot be higher than queueNonce"); - txNonce = _nonce; - } - - /// @dev Adds a transaction to the queue (same as executor interface so that this can be placed between other modules and the safe). - /// @param to Destination address of module transaction - /// @param value Ether value of module transaction - /// @param data Data payload of module transaction - /// @param operation Operation type of module transaction - /// @notice Can only be called by enabled modules - function execTransactionFromModule(address to, uint256 value, bytes calldata data, Enum.Operation operation) - public - moduleOnly() - { - txHash[queueNonce] = getTransactionHash(to, value, data, operation); - txCreatedAt[queueNonce] = block.timestamp; - emit TransactionAdded(queueNonce, txHash[queueNonce], to, value, data, operation); - queueNonce++; - } - - /// @dev Executes the next transaction only if the cooldown has passed and the transaction has not expired - /// @param to Destination address of module transaction - /// @param value Ether value of module transaction - /// @param data Data payload of module transaction - /// @param operation Operation type of module transaction - /// @notice The txIndex used by this function is always 0 - function executeNextTx(address to, uint256 value, bytes calldata data, Enum.Operation operation) - public - { - require(txNonce < queueNonce, "Transaction queue is empty"); - require(block.timestamp - txCreatedAt[txNonce] >= txCooldown, "Transaction is still in cooldown"); - require(txCreatedAt[txNonce] + txCooldown + txExpiration > block.timestamp, "Transaction expired"); - require(txHash[txNonce] == getTransactionHash(to, value, data, operation), "Transaction hashes do not match"); - require(executor.execTransactionFromModule(to, value, data, operation), "Module transaction failed"); - txNonce ++; - } - - function skipExpired() - public - { - while ( - txExpiration != 0 && - txCreatedAt[txNonce] + txCooldown + txExpiration < block.timestamp && - txNonce <= queueNonce) - { - txNonce ++; + /// @dev Sets transaction nonce. Used to invalidate or skip transactions in queue. + /// @param _nonce New transaction nonce + /// @notice This can only be called by the executor + function setTxNonce(uint256 _nonce) public executorOnly() { + require( + _nonce > txNonce, + "New nonce must be higher than current txNonce" + ); + require(_nonce <= queueNonce, "Cannot be higher than queueNonce"); + txNonce = _nonce; } - } - - function getTransactionHash(address to, uint256 value, bytes memory data, Enum.Operation operation) - public - pure - returns(bytes32) - { - return keccak256(abi.encodePacked(to, value, data, operation)); - } - - function getTxHash(uint256 _nonce) - public - view - returns(bytes32) - { - return(txHash[_nonce]); - } - - function getTxCreatedAt(uint256 _nonce) - public - view - returns(uint256) - { - return(txCreatedAt[_nonce]); - } - - function getModuleStatus(address _module) - public - view - returns(bool) - { - return(modules[_module]); - } + /// @dev Adds a transaction to the queue (same as executor interface so that this can be placed between other modules and the safe). + /// @param to Destination address of module transaction + /// @param value Ether value of module transaction + /// @param data Data payload of module transaction + /// @param operation Operation type of module transaction + /// @notice Can only be called by enabled modules + function execTransactionFromModule( + address to, + uint256 value, + bytes calldata data, + Enum.Operation operation + ) public moduleOnly() { + txHash[queueNonce] = getTransactionHash(to, value, data, operation); + txCreatedAt[queueNonce] = block.timestamp; + emit TransactionAdded( + queueNonce, + txHash[queueNonce], + to, + value, + data, + operation + ); + queueNonce++; + } + + /// @dev Executes the next transaction only if the cooldown has passed and the transaction has not expired + /// @param to Destination address of module transaction + /// @param value Ether value of module transaction + /// @param data Data payload of module transaction + /// @param operation Operation type of module transaction + /// @notice The txIndex used by this function is always 0 + function executeNextTx( + address to, + uint256 value, + bytes calldata data, + Enum.Operation operation + ) public { + require(txNonce < queueNonce, "Transaction queue is empty"); + require( + block.timestamp - txCreatedAt[txNonce] >= txCooldown, + "Transaction is still in cooldown" + ); + require( + txCreatedAt[txNonce] + txCooldown + txExpiration > block.timestamp, + "Transaction expired" + ); + require( + txHash[txNonce] == getTransactionHash(to, value, data, operation), + "Transaction hashes do not match" + ); + require( + executor.execTransactionFromModule(to, value, data, operation), + "Module transaction failed" + ); + txNonce++; + } + + function skipExpired() public { + while ( + txExpiration != 0 && + txCreatedAt[txNonce] + txCooldown + txExpiration < + block.timestamp && + txNonce <= queueNonce + ) { + txNonce++; + } + } + + function getTransactionHash( + address to, + uint256 value, + bytes memory data, + Enum.Operation operation + ) public pure returns (bytes32) { + return keccak256(abi.encodePacked(to, value, data, operation)); + } + + function getTxHash(uint256 _nonce) public view returns (bytes32) { + return (txHash[_nonce]); + } + + function getTxCreatedAt(uint256 _nonce) public view returns (uint256) { + return (txCreatedAt[_nonce]); + } + + function getModuleStatus(address _module) public view returns (bool) { + return (modules[_module]); + } } diff --git a/contracts/test/DelayModuleMock.sol b/contracts/test/DelayModuleMock.sol deleted file mode 100644 index 6d02fdb..0000000 --- a/contracts/test/DelayModuleMock.sol +++ /dev/null @@ -1,10 +0,0 @@ -// SPDX-License-Identifier: LGPL-3.0-only -pragma solidity >=0.8.0; - -import "../DelayModule.sol"; - -contract DelayModuleMock is DelayModule { - constructor() DelayModule() { - isInitialized = false; - } -} diff --git a/test/DelayModule.spec.ts b/test/DelayModule.spec.ts index fff51d5..0ba4c96 100644 --- a/test/DelayModule.spec.ts +++ b/test/DelayModule.spec.ts @@ -2,10 +2,10 @@ import { expect } from "chai"; import hre, { deployments, ethers, waffle } from "hardhat"; import "@nomiclabs/hardhat-ethers"; -const ZERO_STATE = "0x0000000000000000000000000000000000000000000000000000000000000000"; +const ZERO_STATE = + "0x0000000000000000000000000000000000000000000000000000000000000000"; describe("DelayModule", async () => { - const baseSetup = deployments.createFixture(async () => { await deployments.fixture(); const Executor = await hre.ethers.getContractFactory("TestExecutor"); @@ -13,235 +13,244 @@ describe("DelayModule", async () => { const Mock = await hre.ethers.getContractFactory("MockContract"); const mock = await Mock.deploy(); return { Executor, executor, module, mock }; - }) + }); const setupTestWithTestExecutor = deployments.createFixture(async () => { const base = await baseSetup(); - const Module = await hre.ethers.getContractFactory("DelayModuleMock"); + const Module = await hre.ethers.getContractFactory("DelayModule"); const module = await Module.deploy(); - await module.setUp(base.executor.address, 42, "0x1337"); + await module.setUp(base.executor.address, 0, "0x1337"); return { ...base, Module, module }; - }) + }); const [user1] = waffle.provider.getWallets(); describe("setUp()", async () => { - it("throws if cooldown is 0", async () => { - const Module = await hre.ethers.getContractFactory("DelayModuleMock") - const module = await Module.deploy() - await expect( - module.setUp(user1.address, 0, 0) - ).to.be.revertedWith("Cooldown must to be greater than 0") - }) it("throws if not enough time between txCooldown and txExpiration", async () => { - const Module = await hre.ethers.getContractFactory("DelayModuleMock") - const module = await Module.deploy() - await expect( - module.setUp(user1.address, 1, 59) - ).to.be.revertedWith("Expiratition must be 0 or at least 60 seconds") - }) + const Module = await hre.ethers.getContractFactory("DelayModule"); + const module = await Module.deploy(); + await expect(module.setUp(user1.address, 1, 59)).to.be.revertedWith( + "Expiratition must be 0 or at least 60 seconds" + ); + }); it("txExpiration can be 0", async () => { - const Module = await hre.ethers.getContractFactory("DelayModuleMock") - const module = await Module.deploy() - await module.setUp(user1.address, 1, 0) - }) - }) - - describe('disableModule()', async () => { - it('throws if not authorized', async () => { + const Module = await hre.ethers.getContractFactory("DelayModule"); + const module = await Module.deploy(); + await module.setUp(user1.address, 1, 0); + }); + }); + + describe("disableModule()", async () => { + it("throws if not authorized", async () => { const { module } = await setupTestWithTestExecutor(); - await expect( - module.disableModule(user1.address) - ).to.be.revertedWith("Not authorized"); + await expect(module.disableModule(user1.address)).to.be.revertedWith( + "Not authorized" + ); }); - it('disables a module()', async () => { + it("disables a module()", async () => { const { executor, module } = await setupTestWithTestExecutor(); - const enable = await module.populateTransaction.enableModule(user1.address); - const disable = await module.populateTransaction.disableModule(user1.address); - - await executor.exec(module.address,0,enable.data); - await expect(await module.getModuleStatus(user1.address)).to.be.equals(true); - await executor.exec(module.address,0,disable.data); - await expect(await module.getModuleStatus(user1.address)).to.be.equals(false); + const enable = await module.populateTransaction.enableModule( + user1.address + ); + const disable = await module.populateTransaction.disableModule( + user1.address + ); + + await executor.exec(module.address, 0, enable.data); + await expect(await module.getModuleStatus(user1.address)).to.be.equals( + true + ); + await executor.exec(module.address, 0, disable.data); + await expect(await module.getModuleStatus(user1.address)).to.be.equals( + false + ); }); }); - describe('enableModule()', async () => { - it('throws if not authorized', async () => { + describe("enableModule()", async () => { + it("throws if not authorized", async () => { const { module } = await setupTestWithTestExecutor(); - await expect( - module.enableModule(user1.address) - ).to.be.revertedWith("Not authorized"); + await expect(module.enableModule(user1.address)).to.be.revertedWith( + "Not authorized" + ); }); - it('enables a module', async () => { + it("enables a module", async () => { const { executor, module } = await setupTestWithTestExecutor(); - const enable = await module.populateTransaction.enableModule(user1.address); - - await executor.exec(module.address,0,enable.data); - await expect(await module.getModuleStatus(user1.address)).to.be.equals(true); + const enable = await module.populateTransaction.enableModule( + user1.address + ); + + await executor.exec(module.address, 0, enable.data); + await expect(await module.getModuleStatus(user1.address)).to.be.equals( + true + ); }); }); - describe('setTxCooldown()', async () => { - it('throws if not authorized', async () => { + describe("setTxCooldown()", async () => { + it("throws if not authorized", async () => { const { module } = await setupTestWithTestExecutor(); - await expect( - module.setTxCooldown(42) - ).to.be.revertedWith("Not authorized"); + await expect(module.setTxCooldown(42)).to.be.revertedWith( + "Not authorized" + ); }); - it('sets cooldown', async () => { + it("sets cooldown", async () => { const { executor, module } = await setupTestWithTestExecutor(); const tx = await module.populateTransaction.setTxCooldown(43); let cooldown = await module.txCooldown(); - await expect(cooldown._hex).to.be.equals("0x2a"); - await executor.exec(module.address,0,tx.data) + await expect(cooldown._hex).to.be.equals("0x00"); + await executor.exec(module.address, 0, tx.data); cooldown = await module.txCooldown(); await expect(cooldown._hex).to.be.equals("0x2b"); }); }); - describe('setTxExpiration()', async () => { - it('throws if not authorized', async () => { + describe("setTxExpiration()", async () => { + it("throws if not authorized", async () => { const { module } = await setupTestWithTestExecutor(); - await expect( - module.setTxExpiration(42) - ).to.be.revertedWith("Not authorized"); + await expect(module.setTxExpiration(42)).to.be.revertedWith( + "Not authorized" + ); }); - it('thows if expiration is less than 60 seconds.', async () => { + it("thows if expiration is less than 60 seconds.", async () => { const { executor, module } = await setupTestWithTestExecutor(); const tx = await module.populateTransaction.setTxExpiration(59); await expect( - executor.exec(module.address,0,tx.data) + executor.exec(module.address, 0, tx.data) ).to.be.revertedWith("Expiratition must be 0 or at least 60 seconds"); }); - it('sets expiration', async () => { + it("sets expiration", async () => { const { executor, module } = await setupTestWithTestExecutor(); const tx = await module.populateTransaction.setTxExpiration("0x031337"); let expiration = await module.txExpiration(); await expect(expiration._hex).to.be.equals("0x1337"); - await executor.exec(module.address,0,tx.data) + await executor.exec(module.address, 0, tx.data); expiration = await module.txExpiration(); await expect(expiration._hex).to.be.equals("0x031337"); }); - }); - describe('setTxNonce()', async () => { - it('throws if not authorized', async () => { + describe("setTxNonce()", async () => { + it("throws if not authorized", async () => { const { module } = await setupTestWithTestExecutor(); - await expect( - module.setTxNonce(42) - ).to.be.revertedWith("Not authorized"); + await expect(module.setTxNonce(42)).to.be.revertedWith("Not authorized"); }); - it('thows if nonce is less than current nonce.', async () => { + it("thows if nonce is less than current nonce.", async () => { const { executor, module } = await setupTestWithTestExecutor(); const tx = await module.populateTransaction.setTxExpiration(60); const tx2 = await module.populateTransaction.setTxNonce(0); - await expect(executor.exec(module.address,0,tx.data)); + await expect(executor.exec(module.address, 0, tx.data)); await expect( - executor.exec(module.address,0,tx2.data) + executor.exec(module.address, 0, tx2.data) ).to.be.revertedWith("New nonce must be higher than current txNonce"); }); - - it('thows if nonce is more than queueNonce + 1.', async () => { + it("thows if nonce is more than queueNonce + 1.", async () => { const { executor, module } = await setupTestWithTestExecutor(); const tx = await module.populateTransaction.enableModule(user1.address); const tx2 = await module.populateTransaction.setTxNonce(42); - await expect(executor.exec(module.address,0,tx.data)); - await module.execTransactionFromModule(user1.address,0,"0x",0); + await expect(executor.exec(module.address, 0, tx.data)); + await module.execTransactionFromModule(user1.address, 0, "0x", 0); await expect( - executor.exec(module.address,0,tx2.data) + executor.exec(module.address, 0, tx2.data) ).to.be.revertedWith("Cannot be higher than queueNonce"); }); - it('sets nonce', async () => { + it("sets nonce", async () => { const { executor, module } = await setupTestWithTestExecutor(); const tx = await module.populateTransaction.enableModule(user1.address); const tx2 = await module.populateTransaction.setTxNonce(1); let txNonce = await module.txNonce(); await expect(txNonce._hex).to.be.equals("0x00"); - await executor.exec(module.address,0,tx.data); - await module.execTransactionFromModule(user1.address,0,"0x",0); - await expect(executor.exec(module.address,0,tx2.data)); + await executor.exec(module.address, 0, tx.data); + await module.execTransactionFromModule(user1.address, 0, "0x", 0); + await expect(executor.exec(module.address, 0, tx2.data)); txNonce = await module.txNonce(); await expect(txNonce._hex).to.be.equals("0x01"); }); }); - describe('execTransactionFromModule()', async () => { - it('throws if not authorized', async () => { + describe("execTransactionFromModule()", async () => { + it("throws if not authorized", async () => { const { module } = await setupTestWithTestExecutor(); await expect( - module.execTransactionFromModule(user1.address,0,"0x",0) + module.execTransactionFromModule(user1.address, 0, "0x", 0) ).to.be.revertedWith("Module not authorized"); }); - it('increments queueNonce', async () => { + it("increments queueNonce", async () => { const { executor, module } = await setupTestWithTestExecutor(); const tx = await module.populateTransaction.enableModule(user1.address); - await executor.exec(module.address,0,tx.data); + await executor.exec(module.address, 0, tx.data); let queueNonce = await module.queueNonce(); await expect(queueNonce._hex).to.be.equals("0x00"); - await module.execTransactionFromModule(user1.address,0,"0x",0) + await module.execTransactionFromModule(user1.address, 0, "0x", 0); queueNonce = await module.queueNonce(); await expect(queueNonce._hex).to.be.equals("0x01"); }); - it('sets txHash', async () => { + it("sets txHash", async () => { const { executor, module } = await setupTestWithTestExecutor(); const tx = await module.populateTransaction.enableModule(user1.address); - await executor.exec(module.address,0,tx.data); + await executor.exec(module.address, 0, tx.data); - let txHash = await module.getTransactionHash(user1.address,0,"0x",0); + let txHash = await module.getTransactionHash(user1.address, 0, "0x", 0); await expect(await module.getTxHash(0)).to.be.equals(ZERO_STATE); - await module.execTransactionFromModule(user1.address,0,"0x",0) + await module.execTransactionFromModule(user1.address, 0, "0x", 0); await expect(await module.getTxHash(0)).to.be.equals(txHash); }); - it('sets txCreatedAt', async () => { + it("sets txCreatedAt", async () => { const { executor, module } = await setupTestWithTestExecutor(); const tx = await module.populateTransaction.enableModule(user1.address); let expectedTimestamp = await module.getTxCreatedAt(0); - await executor.exec(module.address,0,tx.data); + await executor.exec(module.address, 0, tx.data); await expect(expectedTimestamp._hex).to.be.equals("0x00"); - let receipt = await module.execTransactionFromModule(user1.address,0,"0x",0); + let receipt = await module.execTransactionFromModule( + user1.address, + 0, + "0x", + 0 + ); let blockNumber = receipt.blockNumber; - let block = await hre.network.provider.send("eth_getBlockByNumber", ["latest", false]); + let block = await hre.network.provider.send("eth_getBlockByNumber", [ + "latest", + false, + ]); expectedTimestamp = await module.getTxCreatedAt(0); await expect(block.timestamp).to.be.equals(expectedTimestamp._hex); }); - it('emits transaction details', async () => { + it("emits transaction details", async () => { const { executor, module } = await setupTestWithTestExecutor(); const tx = await module.populateTransaction.enableModule(user1.address); - await executor.exec(module.address,0,tx.data); + await executor.exec(module.address, 0, tx.data); const expectedQueueNonce = await module.queueNonce; - await expect(module.execTransactionFromModule(user1.address,42,"0x",0)) - .to.emit(module, 'TransactionAdded') + await expect(module.execTransactionFromModule(user1.address, 42, "0x", 0)) + .to.emit(module, "TransactionAdded") .withArgs( expectedQueueNonce, - await module.getTransactionHash(user1.address,42,"0x",0), + await module.getTransactionHash(user1.address, 42, "0x", 0), user1.address, 42, "0x", @@ -250,106 +259,128 @@ describe("DelayModule", async () => { }); }); - describe('executeNextTx()', async () => { - it('throws if there is nothing in queue', async () => { + describe("executeNextTx()", async () => { + it("throws if there is nothing in queue", async () => { const { executor, module } = await setupTestWithTestExecutor(); const tx = await module.populateTransaction.enableModule(user1.address); - await executor.exec(module.address,0,tx.data); + await executor.exec(module.address, 0, tx.data); - await expect(module.executeNextTx(user1.address,42,"0x",0)) - .to.be.revertedWith("Transaction queue is empty"); + await expect( + module.executeNextTx(user1.address, 42, "0x", 0) + ).to.be.revertedWith("Transaction queue is empty"); }); - it('throws if cooldown has not passed', async () => { + it("throws if cooldown has not passed", async () => { const { executor, module } = await setupTestWithTestExecutor(); - const tx = await module.populateTransaction.enableModule(user1.address); - await executor.exec(module.address,0,tx.data); + let tx = await module.populateTransaction.setTxCooldown(42); + await executor.exec(module.address, 0, tx.data); + + tx = await module.populateTransaction.enableModule(user1.address); + await executor.exec(module.address, 0, tx.data); - await module.execTransactionFromModule(user1.address,42,"0x",0); - await expect(module.executeNextTx(user1.address,42,"0x",0)) - .to.be.revertedWith("Transaction is still in cooldown"); + await module.execTransactionFromModule(user1.address, 42, "0x", 0); + await expect( + module.executeNextTx(user1.address, 42, "0x", 0) + ).to.be.revertedWith("Transaction is still in cooldown"); }); - it('throws if transaction has expired', async () => { + it("throws if transaction has expired", async () => { const { executor, module } = await setupTestWithTestExecutor(); const tx = await module.populateTransaction.enableModule(user1.address); - await executor.exec(module.address,0,tx.data); + await executor.exec(module.address, 0, tx.data); await executor.setModule(module.address); - await module.execTransactionFromModule(user1.address,0,"0x",0); + await module.execTransactionFromModule(user1.address, 0, "0x", 0); let expiry = await module.txCreatedAt(0); - await hre.network.provider.send("evm_setNextBlockTimestamp", [4242424242]); - await expect(module.executeNextTx(user1.address,0,"0x",0)) - .to.be.revertedWith("Transaction expired"); + await hre.network.provider.send("evm_setNextBlockTimestamp", [ + 4242424242, + ]); + await expect( + module.executeNextTx(user1.address, 0, "0x", 0) + ).to.be.revertedWith("Transaction expired"); }); - it('throws if transaction hashes do not match', async () => { + it("throws if transaction hashes do not match", async () => { const { executor, module } = await setupTestWithTestExecutor(); const tx = await module.populateTransaction.enableModule(user1.address); - await executor.exec(module.address,0,tx.data); + await executor.exec(module.address, 0, tx.data); await executor.setModule(module.address); - await module.execTransactionFromModule(user1.address,0,"0x",0); - let block = await hre.network.provider.send("eth_getBlockByNumber", ["latest", false]); + await module.execTransactionFromModule(user1.address, 0, "0x", 0); + let block = await hre.network.provider.send("eth_getBlockByNumber", [ + "latest", + false, + ]); let timestamp = parseInt(block.timestamp) + 100; await hre.network.provider.send("evm_setNextBlockTimestamp", [timestamp]); - await expect(module.executeNextTx(user1.address,1,"0x",0)) - .to.be.revertedWith("Transaction hashes do not match"); + await expect( + module.executeNextTx(user1.address, 1, "0x", 0) + ).to.be.revertedWith("Transaction hashes do not match"); }); - it('throws if transaction module transaction throws', async () => { + it("throws if transaction module transaction throws", async () => { const { executor, module } = await setupTestWithTestExecutor(); const tx = await module.populateTransaction.enableModule(user1.address); - await executor.exec(module.address,0,tx.data); + await executor.exec(module.address, 0, tx.data); await executor.setModule(module.address); - await module.execTransactionFromModule(user1.address,1,"0x",0); - let block = await hre.network.provider.send("eth_getBlockByNumber", ["latest", false]); + await module.execTransactionFromModule(user1.address, 1, "0x", 0); + let block = await hre.network.provider.send("eth_getBlockByNumber", [ + "latest", + false, + ]); let timestamp = parseInt(block.timestamp) + 100; await hre.network.provider.send("evm_setNextBlockTimestamp", [timestamp]); - await expect(module.executeNextTx(user1.address,1,"0x",0)) - .to.be.revertedWith("Module transaction failed"); + await expect( + module.executeNextTx(user1.address, 1, "0x", 0) + ).to.be.revertedWith("Module transaction failed"); }); - it('executes transaction', async () => { + it("executes transaction", async () => { const { executor, module } = await setupTestWithTestExecutor(); const tx = await module.populateTransaction.enableModule(user1.address); - await executor.exec(module.address,0,tx.data); + await executor.exec(module.address, 0, tx.data); await executor.setModule(module.address); - await module.execTransactionFromModule(user1.address,0,"0x",0); - let block = await hre.network.provider.send("eth_getBlockByNumber", ["latest", false]); + await module.execTransactionFromModule(user1.address, 0, "0x", 0); + let block = await hre.network.provider.send("eth_getBlockByNumber", [ + "latest", + false, + ]); let timestamp = parseInt(block.timestamp) + 100; await hre.network.provider.send("evm_setNextBlockTimestamp", [timestamp]); - await expect(module.executeNextTx(user1.address,0,"0x",0)); + await expect(module.executeNextTx(user1.address, 0, "0x", 0)); }); }); - describe('skipExpired()', async () => { - it('should skip to the next nonce that has not yet expired', async () => { + describe("skipExpired()", async () => { + it("should skip to the next nonce that has not yet expired", async () => { const { executor, module } = await setupTestWithTestExecutor(); const tx = await module.populateTransaction.enableModule(user1.address); - await executor.exec(module.address,0,tx.data); + await executor.exec(module.address, 0, tx.data); await executor.setModule(module.address); for (let i = 0; i < 3; i++) { - await module.execTransactionFromModule(user1.address,0,"0x",0); + await module.execTransactionFromModule(user1.address, 0, "0x", 0); } - let block = await hre.network.provider.send("eth_getBlockByNumber", ["latest", false]); + let block = await hre.network.provider.send("eth_getBlockByNumber", [ + "latest", + false, + ]); let timestamp = parseInt(block.timestamp) + 424242; await hre.network.provider.send("evm_setNextBlockTimestamp", [timestamp]); - await expect(module.executeNextTx(user1.address,0,"0x",0)) - .to.be.revertedWith("Transaction expired"); + await expect( + module.executeNextTx(user1.address, 0, "0x", 0) + ).to.be.revertedWith("Transaction expired"); for (let i = 0; i < 2; i++) { - await module.execTransactionFromModule(user1.address,0,"0x",0); + await module.execTransactionFromModule(user1.address, 0, "0x", 0); } await expect(module.skipExpired()); let txNonce = await module.txNonce(); let queueNonce = await module.queueNonce(); await expect(parseInt(txNonce._hex)).to.be.equals(3); await expect(parseInt(queueNonce._hex)).to.be.equals(5); - await expect(module.executeNextTx(user1.address,0,"0x",0)); + await expect(module.executeNextTx(user1.address, 0, "0x", 0)); }); }); - -}) +}); From 97d2cee039cfef9dddd3888c71f5060d2ed91de8 Mon Sep 17 00:00:00 2001 From: Auryn Macmillan Date: Thu, 1 Jul 2021 14:48:41 -0400 Subject: [PATCH 3/3] Formatting --- test/DelayModule.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/DelayModule.spec.ts b/test/DelayModule.spec.ts index 0ba4c96..c436054 100644 --- a/test/DelayModule.spec.ts +++ b/test/DelayModule.spec.ts @@ -26,7 +26,6 @@ describe("DelayModule", async () => { const [user1] = waffle.provider.getWallets(); describe("setUp()", async () => { - it("throws if not enough time between txCooldown and txExpiration", async () => { const Module = await hre.ethers.getContractFactory("DelayModule"); const module = await Module.deploy();