diff --git a/contracts/SecretDelay.sol b/contracts/SecretDelay.sol index ac5f400..8c7056a 100644 --- a/contracts/SecretDelay.sol +++ b/contracts/SecretDelay.sol @@ -18,11 +18,14 @@ contract SecretDelay is Modifier { bytes data, Enum.Operation operation ); + event TransactionPromoted(uint256 indexed nonce); uint256 public txCooldown; uint256 public txExpiration; uint256 public txNonce; uint256 public queueNonce; + uint256 public approved; + // Mapping of queue nonce to transaction hash. mapping(uint256 => bytes32) public txHash; // Mapping of queue nonce to creation timestamp. @@ -106,9 +109,26 @@ contract SecretDelay is Modifier { function setTxNonce(uint256 _nonce) public onlyOwner { require(_nonce > txNonce, "New nonce must be higher than current txNonce"); require(_nonce <= queueNonce, "Cannot be higher than queueNonce"); + + adjustApprovals(_nonce); txNonce = _nonce; } + function setTxNonceAndApprove(uint256 _nonce, uint256 _transactions) + public + onlyOwner + { + require(_nonce < queueNonce, "Cannot skip all transactions"); + if (_nonce > txNonce) setTxNonce(_nonce); + approveNext(_transactions); + } + + function approveNext(uint256 transactions) public onlyOwner { + require(transactions > 0, "Must approve at least one tx"); + require(queueNonce - txNonce >= transactions, "Cannot approve unknown tx"); + approved = transactions; + } + /// @dev Adds a transaction to the queue (same as avatar interface so that this can be placed between other modules and the avatar). /// @param to Destination address of module transaction /// @param value Ether value of module transaction @@ -149,7 +169,7 @@ contract SecretDelay is Modifier { ) public { require(txNonce < queueNonce, "Transaction queue is empty"); require( - block.timestamp - txCreatedAt[txNonce] >= txCooldown, + (block.timestamp - txCreatedAt[txNonce] >= txCooldown) || approved > 0, "Transaction is still in cooldown" ); if (txExpiration != 0) { @@ -163,6 +183,7 @@ contract SecretDelay is Modifier { "Transaction hashes do not match" ); txNonce++; + if (approved > 0) approved--; require(exec(to, value, data, operation), "Module transaction failed"); } @@ -192,4 +213,14 @@ contract SecretDelay is Modifier { function getTxCreatedAt(uint256 _nonce) public view returns (uint256) { return (txCreatedAt[_nonce]); } + + function adjustApprovals(uint256 _nonce) internal { + uint256 delta = _nonce - txNonce; + + if (delta > approved) { + if (approved != 0) approved = 0; + } else { + approved -= delta; + } + } } diff --git a/contracts/test/TestContract.sol b/contracts/test/TestContract.sol new file mode 100644 index 0000000..2d977d9 --- /dev/null +++ b/contracts/test/TestContract.sol @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +pragma solidity >=0.8.0; + +import "@openzeppelin/contracts/access/Ownable.sol"; + +contract TestContract is Ownable { + event ButtonPushed(address pusher); + + function pushButton() public onlyOwner { + emit ButtonPushed(msg.sender); + } +} diff --git a/test/SecretDelay.spec.ts b/test/SecretDelay.spec.ts index 080dbae..e915b11 100644 --- a/test/SecretDelay.spec.ts +++ b/test/SecretDelay.spec.ts @@ -1,6 +1,7 @@ import { expect } from "chai"; -import hre, { deployments, waffle } from "hardhat"; +import hre, { deployments, waffle, ethers } from "hardhat"; import "@nomiclabs/hardhat-ethers"; +import { Contract, PopulatedTransaction, Transaction } from "ethers"; const ZeroState = "0x0000000000000000000000000000000000000000000000000000000000000000"; @@ -30,6 +31,14 @@ describe("SecretDelay", async () => { return { ...base, Modifier, modifier }; }); + const setupTestContract = async (address: string) => { + const TestContract = await ethers.getContractFactory("TestContract"); + const testContract = await TestContract.deploy(); + await testContract.transferOwnership(address); + + return testContract; + }; + const [user1] = waffle.provider.getWallets(); describe("setUp()", async () => { @@ -262,10 +271,81 @@ describe("SecretDelay", async () => { await expect(txNonce._hex).to.be.equals("0x00"); await avatar.exec(modifier.address, 0, tx.data); await modifier.execTransactionFromModule(user1.address, 0, "0x", 0); - await expect(avatar.exec(modifier.address, 0, tx2.data)); + await avatar.exec(modifier.address, 0, tx2.data); txNonce = await modifier.txNonce(); + await expect(txNonce._hex).to.be.equals("0x01"); }); + + context("with two enqueued txs", () => { + let testContract: Contract, modifier: Contract, avatar: Contract; + + beforeEach("enqueue and approve two transactions", async () => { + ({ avatar, modifier } = await setupTestWithTestAvatar()); + testContract = await setupTestContract(avatar.address); + + let tx = await modifier.populateTransaction.setTxCooldown(42); + await avatar.exec(modifier.address, 0, tx.data); + + tx = await modifier.populateTransaction.enableModule(user1.address); + await avatar.exec(modifier.address, 0, tx.data); + + await avatar.setModule(modifier.address); + + const tx2 = await testContract.populateTransaction.pushButton(); + await modifier.execTransactionFromModule( + testContract.address, + 0, + tx2.data, + 0 + ); + await modifier.execTransactionFromModule( + testContract.address, + 0, + tx2.data, + 0 + ); + }); + + context("with no approved tx", () => { + it("sets txNonce", async () => { + const tx = await modifier.populateTransaction.setTxNonce(1); + await avatar.exec(modifier.address, 0, tx.data); + + expect(await modifier.txNonce()).to.equal(1); + }); + }); + + context("with two approved tx", () => { + it("decrements number of approved transactions", async () => { + let tx3 = await modifier.populateTransaction.setTxNonceAndApprove( + 0, + 2 + ); + await avatar.exec(modifier.address, 0, tx3.data); + + const tx = await modifier.populateTransaction.setTxNonce(1); + await avatar.exec(modifier.address, 0, tx.data); + + expect(await modifier.approved()).to.equal(1); + }); + }); + + context("with two approved tx and jump to end of queue", () => { + it("sets number of approved to zero", async () => { + let tx3 = await modifier.populateTransaction.setTxNonceAndApprove( + 0, + 1 + ); + await avatar.exec(modifier.address, 0, tx3.data); + + const tx = await modifier.populateTransaction.setTxNonce(2); + await avatar.exec(modifier.address, 0, tx.data); + + expect(await modifier.approved()).to.equal(0); + }); + }); + }); }); describe("execTransactionFromModule()", async () => { @@ -370,6 +450,92 @@ describe("SecretDelay", async () => { ).to.be.revertedWith("Transaction is still in cooldown"); }); + context("when transaction is approved", () => { + let testContract: Contract, modifier: Contract, avatar: Contract; + let pushButtonTx: PopulatedTransaction; + + beforeEach("enqueue two tx", async () => { + ({ avatar, modifier } = await setupTestWithTestAvatar()); + testContract = await setupTestContract(avatar.address); + + let tx = await modifier.populateTransaction.setTxCooldown(42); + await avatar.exec(modifier.address, 0, tx.data); + + tx = await modifier.populateTransaction.enableModule(user1.address); + await avatar.exec(modifier.address, 0, tx.data); + + await avatar.setModule(modifier.address); + + pushButtonTx = await testContract.populateTransaction.pushButton(); + await modifier.execTransactionFromModule( + testContract.address, + 0, + pushButtonTx.data, + 0 + ); + await modifier.execTransactionFromModule( + testContract.address, + 0, + pushButtonTx.data, + 0 + ); + + let tx3 = await modifier.populateTransaction.approveNext(1); + await avatar.exec(modifier.address, 0, tx3.data); + }); + + it("executes", async () => { + await expect( + modifier.executeNextTx(testContract.address, 0, pushButtonTx.data, 0) + ).to.emit(testContract, "ButtonPushed"); + }); + + it("decrements approved transactions", async () => { + await modifier.executeNextTx( + testContract.address, + 0, + pushButtonTx.data, + 0 + ); + + expect(await modifier.approved()).to.equal(0); + }); + }); + + // it("executes during cooldown if transaction had been approved", async () => { + // const { avatar, modifier } = await setupTestWithTestAvatar(); + + // const TestContract = await ethers.getContractFactory("TestContract"); + // const testContract = await TestContract.deploy(); + // await testContract.transferOwnership(avatar.address); + + // let tx = await modifier.populateTransaction.setTxCooldown(42); + // await avatar.exec(modifier.address, 0, tx.data); + + // tx = await modifier.populateTransaction.enableModule(user1.address); + // await avatar.exec(modifier.address, 0, tx.data); + + // const tx2 = await testContract.populateTransaction.pushButton(); + // await modifier.execTransactionFromModule( + // testContract.address, + // 0, + // tx2.data, + // 0 + // ); + // await expect( + // modifier.executeNextTx(user1.address, 42, "0x", 0) + // ).to.be.revertedWith("Transaction is still in cooldown"); + + // let tx3 = await modifier.populateTransaction.setTxNonceAndApprove(0); + // await avatar.exec(modifier.address, 0, tx3.data); + + // await avatar.setModule(modifier.address); + + // await expect( + // modifier.executeNextTx(testContract.address, 0, tx2.data, 0) + // ).to.emit(testContract, "ButtonPushed"); + // }); + it("throws if transaction has expired", async () => { const { avatar, modifier } = await setupTestWithTestAvatar(); const tx = await modifier.populateTransaction.enableModule(user1.address); @@ -469,4 +635,165 @@ describe("SecretDelay", async () => { await expect(modifier.executeNextTx(user1.address, 0, "0x", 0)); }); }); + + describe("setTxNonceAndApprove()", async () => { + it("throws if not authorized", async () => { + const { modifier } = await setupTestWithTestAvatar(); + await expect(modifier.setTxNonceAndApprove(42, 1)).to.be.revertedWith( + "Ownable: caller is not the owner" + ); + }); + + it("reverts if attempting to skip all transactions", async () => { + const { avatar, modifier } = await setupTestWithTestAvatar(); + const tx = await modifier.populateTransaction.enableModule(user1.address); + const tx2 = await modifier.populateTransaction.setTxNonceAndApprove(1, 1); + + expect(await modifier.approved()).to.equal(0); + + await avatar.exec(modifier.address, 0, tx.data); + // await avatar.exec(modifier.address, 0, tx2.data); + + await expect( + avatar.exec(modifier.address, 0, tx2.data) + ).to.be.revertedWith("Cannot skip all transactions"); + }); + + it("increases the txNonce and resets approved when executed", async () => { + const { avatar, modifier } = await setupTestWithTestAvatar(); + const TestContract = await ethers.getContractFactory("TestContract"); + const testContract = await TestContract.deploy(); + await testContract.transferOwnership(avatar.address); + + let tx = await modifier.populateTransaction.setTxCooldown(42); + await avatar.exec(modifier.address, 0, tx.data); + + tx = await modifier.populateTransaction.enableModule(user1.address); + await avatar.exec(modifier.address, 0, tx.data); + + const tx2 = await testContract.populateTransaction.pushButton(); + await modifier.execTransactionFromModule( + testContract.address, + 0, + tx2.data, + 0 + ); + + let tx3 = await modifier.populateTransaction.setTxNonceAndApprove(0, 1); + await avatar.exec(modifier.address, 0, tx3.data); + + await avatar.setModule(modifier.address); + + await expect( + modifier.executeNextTx(testContract.address, 0, tx2.data, 0) + ).to.emit(testContract, "ButtonPushed"); + + expect(await modifier.txNonce()).to.be.equal(await modifier.queueNonce()); + expect(await modifier.approved()).to.equal(0); + }); + + context("with two enqueued transactions", () => { + let testContract: Contract, modifier: Contract, avatar: Contract; + + let pushButtonTx: PopulatedTransaction; + + beforeEach("enqueue two tx and approve second tx", async () => { + ({ avatar, modifier } = await setupTestWithTestAvatar()); + testContract = await setupTestContract(avatar.address); + + let tx = await modifier.populateTransaction.setTxCooldown(42); + await avatar.exec(modifier.address, 0, tx.data); + + tx = await modifier.populateTransaction.enableModule(user1.address); + await avatar.exec(modifier.address, 0, tx.data); + + await avatar.setModule(modifier.address); + + pushButtonTx = await testContract.populateTransaction.pushButton(); + await modifier.execTransactionFromModule( + testContract.address, + 0, + pushButtonTx.data, + 0 + ); + await modifier.execTransactionFromModule( + testContract.address, + 0, + pushButtonTx.data, + 0 + ); + + let tx3 = await modifier.populateTransaction.setTxNonceAndApprove(1, 1); + await avatar.exec(modifier.address, 0, tx3.data); + }); + + it("executes the second tx", async () => { + await expect( + modifier.executeNextTx(testContract.address, 0, pushButtonTx.data, 0) + ).to.emit(testContract, "ButtonPushed"); + expect(await modifier.queueNonce()).to.equal(2); + expect(await modifier.txNonce()).to.equal(2); + }); + }); + }); + + describe("approveNext()", async () => { + let testContract: Contract, modifier: Contract, avatar: Contract; + + let pushButtonTx: PopulatedTransaction; + + beforeEach("enqueue two tx", async () => { + ({ avatar, modifier } = await setupTestWithTestAvatar()); + testContract = await setupTestContract(avatar.address); + + let tx = await modifier.populateTransaction.setTxCooldown(42); + await avatar.exec(modifier.address, 0, tx.data); + + tx = await modifier.populateTransaction.enableModule(user1.address); + await avatar.exec(modifier.address, 0, tx.data); + + await avatar.setModule(modifier.address); + + pushButtonTx = await testContract.populateTransaction.pushButton(); + await modifier.execTransactionFromModule( + testContract.address, + 0, + pushButtonTx.data, + 0 + ); + await modifier.execTransactionFromModule( + testContract.address, + 0, + pushButtonTx.data, + 0 + ); + }); + + context("with parameter is zero", () => { + it("reverts 'Must approve at least one tx'", async () => { + let tx = await modifier.populateTransaction.approveNext(0); + + await expect( + avatar.exec(modifier.address, 0, tx.data) + ).to.be.revertedWith("Must approve at least one tx"); + }); + }); + + context("when approving more tx than enqueued", () => { + it("reverts 'Cannot approve unknown tx'", async () => { + let tx = await modifier.populateTransaction.approveNext(3); + + await expect( + avatar.exec(modifier.address, 0, tx.data) + ).to.be.revertedWith("Cannot approve unknown tx"); + }); + }); + + it("sets approved", async () => { + let tx = await modifier.populateTransaction.approveNext(2); + + await avatar.exec(modifier.address, 0, tx.data); + expect(await modifier.approved()).to.equal(2); + }); + }); });