improve tests with better descriptions and more state assertions

Signed-off-by: AlienTornadosaurusHex <>

.gitignore

@@ -1,2 +1,3 @@
 out
 cache
+FORUM_POST.md

README.md

@@ -5,24 +5,18 @@
 
 ## Installation
 
-```
+```bash
-git clone --recurse-submodules https://git.tornado.ws/Theo/proposal-21-test.git
+git clone --recurse-submodules https://git.tornado.ws/AlienTornadosaurusHex/proposal-21-test.git
 ```
 
 ## Testing
 
+```bash
+yarn # or npm
+# To test slots
+yarn test:slots # or npm
+# To test execution of the proposal and balance changes
+yarn test:execution
+# To test both
+yarn test
 ```
-npm run test
-```
-
-## Test logic
-
-Hacker added 1 200 000 TORN tokens to staking directly, 10 000 to 100 accounts and 200 000 to one account.
-He then withdrew 483,000 TORN from the Governance Vault, however, 717,000 remained.
-
-We need to check, that:
-
-1. All attacker staked TORNs after proposal 21 will be nullified;
-2. Governance Vault will be replenished from Governance for 483 000 TORN - the entire amount that he withdrew, to set the ratio of funds locked in staking contract to real tokens in Vault 1: 1
-
-If all tests passed, all good.

package.json

@@ -9,12 +9,12 @@
     "test": "npm run test:slots & npm run test:execution",
     "build": "forge build --optimize",
     "test:slots": "forge test -vv --match-contract StorageBased",
-    "test:execution": "forge test -vvv --fork-url https://rpc.mevblocker.io --block-number 17315182 --match-contract ExecutionBased"
+    "test:execution": "forge test -vv --fork-url https://rpc.mevblocker.io --block-number 17315182 --match-contract ExecutionBased"
   },
   "repository": {
     "type": "git",
     "url": "https://git.tornado.ws/Theo/proposal-21-test"
   },
-  "author": "Theo",
+  "author": "Theo & AlienTornadosaurusHex",
   "license": "MIT"
 }

src/interfaces/IGovernance.sol

@@ -21,6 +21,20 @@ struct Proposal {
 }
 
 interface IGovernance {
+    function initialized() external view returns (bool);
+    function initializing() external view returns (bool);
+
+    function EXECUTION_DELAY() external view returns (uint256);
+    function EXECUTION_EXPIRATION() external view returns (uint256);
+    function QUORUM_VOTES() external view returns (uint256);
+    function PROPOSAL_THRESHOLD() external view returns (uint256);
+    function VOTING_DELAY() external view returns (uint256);
+    function VOTING_PERIOD() external view returns (uint256);
+    function CLOSING_PERIOD() external view returns (uint256);
+    function VOTE_EXTEND_TIME() external view returns (uint256);
+
+    function torn() external view returns (address);
+
     function proposals(uint256 index) external view returns (Proposal memory);
 
     function lockedBalance(address account) external view returns (uint256);

test/ExecutionBased.sol

@@ -24,6 +24,7 @@ contract ExecutionBased is Test, Parameters, Mock {
     }
 
     function testExistentHackerProposal() public conditionStateChecks {
+        storeStateBefore();
         waitUntilExecutable(_attackerProposalId);
         governance.execute(_attackerProposalId);
     }
@@ -37,26 +38,87 @@ contract ExecutionBased is Test, Parameters, Mock {
 
     // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ PREDICATES ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
+    function intro() internal {
+        console2.log("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ EXECUTION TESTS ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
+        console2.log("");
+        console2.log(
+            "It is expected that the attackers balances are nullified and that the Vault receives 483,000 TORN,"
+        );
+        console2.log("and that the Governance TORN balance does not otherwise change, or any other state.");
+        console2.log("");
+    }
+
+    function outro() internal {
+        console2.log("");
+        console2.log("All state other state checks completed successfully.");
+        console2.log("");
+    }
+
     function checkCurrentState() internal {
-        console2.log("Current attacker locked balance: %s TORN", getAttackerLockedBalance() / 10 ** 18);
+        intro();
-        console2.log("Current governance Vault balance: %s TORN", getGovernanceVaultBalance() / 10 ** 18);
+        console2.log("Governance balance before execution: %s TORN", getGovernanceBalance() / 10 ** 18);
+        console2.log("Governance Vault balance before execution: %s TORN", getGovernanceVaultBalance() / 10 ** 18);
+        console2.log("Attacker locked balance before execution: %s TORN", getAttackerLockedBalance() / 10 ** 18);
     }
 
     function checkResults() internal {
-        uint256 attackerBalanceAfterProposalExecution = getAttackerLockedBalance();
+        uint256 governanceBalanceAfterProposalExecution = getGovernanceBalance();
         uint256 governanceVaultBalanceAfterProposalExecution = getGovernanceVaultBalance();
+        uint256 attackerBalanceAfterProposalExecution = getAttackerLockedBalance();
 
         console2.log(
-            "Attacker locked balance after proposal 21 execution: %s TORN",
+            "Governance balance after the execution of Proposal #21: %s TORN",
-            attackerBalanceAfterProposalExecution / 10 ** 18
+            governanceBalanceAfterProposalExecution / 10 ** 18
         );
         console2.log(
-            "Governance Vault balance after proposal 21 execution: %s TORN",
+            "Governance Vault balance after the execution of Proposal #21: %s TORN",
             governanceVaultBalanceAfterProposalExecution / 10 ** 18
         );
+        console2.log(
+            "Attacker locked balance after the execution of Proposal #21: %s TORN",
+            attackerBalanceAfterProposalExecution / 10 ** 18
+        );
 
-        require(attackerBalanceAfterProposalExecution == 0 ether);
+        require(uint8(uint256(bytes32(vm.load(_governanceAddress, bytes32(0))))) == 1);
+
+        require(_EXECUTION_DELAY == governance.EXECUTION_DELAY());
+        require(_EXECUTION_EXPIRATION == governance.EXECUTION_EXPIRATION());
+        require(_QUORUM_VOTES == governance.QUORUM_VOTES());
+        require(_PROPOSAL_THRESHOLD == governance.PROPOSAL_THRESHOLD());
+        require(_VOTING_DELAY == governance.VOTING_DELAY());
+        require(_VOTING_PERIOD == governance.VOTING_PERIOD());
+        require(_CLOSING_PERIOD == governance.CLOSING_PERIOD());
+        require(_VOTE_EXTEND_TIME == governance.VOTE_EXTEND_TIME());
+
+        require(governanceBalanceAfterProposalExecution == _governanceBalanceBefore - attackerWithdrawnAmount);
         require(governanceVaultBalanceAfterProposalExecution == currentGovernanceVaultBalance + attackerWithdrawnAmount);
+        require(attackerBalanceAfterProposalExecution == 0 ether);
+
+        require(address(governance.torn()) == _tokenAddress);
+
+        outro();
+    }
+
+    uint256 internal _governanceBalanceBefore;
+    uint256 internal _EXECUTION_DELAY;
+    uint256 internal _EXECUTION_EXPIRATION;
+    uint256 internal _QUORUM_VOTES;
+    uint256 internal _PROPOSAL_THRESHOLD;
+    uint256 internal _VOTING_DELAY;
+    uint256 internal _VOTING_PERIOD;
+    uint256 internal _CLOSING_PERIOD;
+    uint256 internal _VOTE_EXTEND_TIME;
+
+    function storeStateBefore() internal {
+        _governanceBalanceBefore = getGovernanceBalance();
+        _EXECUTION_DELAY = governance.EXECUTION_DELAY();
+        _EXECUTION_EXPIRATION = governance.EXECUTION_EXPIRATION();
+        _QUORUM_VOTES = governance.QUORUM_VOTES();
+        _PROPOSAL_THRESHOLD = governance.PROPOSAL_THRESHOLD();
+        _VOTING_DELAY = governance.VOTING_DELAY();
+        _VOTING_PERIOD = governance.VOTING_PERIOD();
+        _CLOSING_PERIOD = governance.CLOSING_PERIOD();
+        _VOTE_EXTEND_TIME = governance.VOTE_EXTEND_TIME();
     }
 
     // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ HELPERS ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -133,6 +195,10 @@ contract ExecutionBased is Test, Parameters, Mock {
         return attackerLockedBalance;
     }
 
+    function getGovernanceBalance() internal returns (uint256) {
+        return IERC20(_tokenAddress).balanceOf(_governanceAddress);
+    }
+
     function getGovernanceVaultBalance() internal returns (uint256) {
         return IERC20(_tokenAddress).balanceOf(_governanceVaultAddress);
     }

test/StorageBased.sol

@@ -13,6 +13,8 @@ contract StorageBased is Test, Parameters {
     // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ TESTS ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
     function testSlotsMustBeEqualToAddresses() public delimit {
+        console2.log("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ SLOT TO ADDRESS EQUALITY TEST ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
+        console2.log("");
         console2.log(
             "Follow this link to find the storage slots modified during the attack (out of order) => https://etherscan.io/tx/0x3274b6090685b842aca80b304a4dcee0f61ef8b6afee10b7c7533c32fb75486d#statechange\n"
         );
@@ -34,14 +36,29 @@ contract StorageBased is Test, Parameters {
             console2.log("Should equal storage address =>");
             console2.logBytes32(expectedStorageAddress);
             console2.log("Is it? => ", storageAddress == expectedStorageAddress);
+
+            require(storageAddress == expectedStorageAddress);
         }
+
+        console2.log("");
+        console2.log("All 101 slots have been determined to belong to an address being nullified.");
+        console2.log("");
     }
 
     function testComputeProposalExecutedSlot() public delimit {
+        console2.log("~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ PROPOSAL EXECUTED SLOT TEST ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~");
+        console2.log("");
         console2.log("The former test does not account for 1 more storage slot modified in governance.\n");
         console2.log("This is storage slot 0xece66cfdbd22e3f37d348a3d8e19074452862cd65fd4b9a11f0336d1ac6d1e69.");
         console2.log("Computed:");
         console2.logBytes32(bytes32(uint256(keccak256(abi.encode(20 | 61))) + 166));
+        require(
+            (bytes32(uint256(keccak256(abi.encode(20 | 61))) + 166))
+                == bytes32(0xece66cfdbd22e3f37d348a3d8e19074452862cd65fd4b9a11f0336d1ac6d1e69)
+        );
+        console2.log("");
+        console2.log("The slot belongs to proposal.executed = true, this is a successful test.");
+        console2.log("");
     }
 
     // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ GETTERS ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~