[WIP] Fuzz and Invariant Tests

The test directory has been restructured for easier discovery of different tests based on types. Minor gas optimisations have been made in the `L2Comptroller` contract.

.gitmodules

@@ -6,9 +6,6 @@
 	path = lib/openzeppelin-contracts-upgradeable
 	url = https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable
 	branch = v4.8.2
-[submodule "lib/foundry-upgrades"]
-	path = lib/foundry-upgrades
-	url = https://github.com/odyslam/foundry-upgrades
 [submodule "lib/openzeppelin-contracts"]
 	path = lib/openzeppelin-contracts
 	url = https://github.com/OpenZeppelin/openzeppelin-contracts

foundry.toml

@@ -2,6 +2,11 @@
 src = 'src'
 out = 'out'
 libs = ['lib']
+verbosity = 2
+
+[fuzz]
+runs = 1_000
+fail_on_revert = true
 
 [rpc_endpoints]
 ethereum = "${ETHEREUM_RPC_URL}"

package.json

@@ -8,7 +8,9 @@
     "test": "test"
   },
   "scripts": {
-    "test": "echo \"Error: no test specified\" && exit 1"
+    "tests:integration": "forge t --no-match-contract '(?:Fuzz|Invariant)'",
+    "tests:fuzz": "forge t --match-contract '(?:Fuzz)'",
+    "tests:invariant": "forge t --match-contract '(?:Invariant)'"
   },
   "repository": {
     "type": "git",

remappings.txt

@@ -1,7 +1,4 @@
 ds-test/=lib/forge-std/lib/ds-test/src/
 forge-std/=lib/forge-std/src/
-foundry-upgrades/=lib/foundry-upgrades/src/
 openzeppelin-contracts-upgradeable/=lib/openzeppelin-contracts-upgradeable/
 openzeppelin-contracts/=lib/openzeppelin-contracts/
-openzeppelin/=lib/foundry-upgrades/lib/openzeppelin-contracts/contracts/
-solmate/=lib/foundry-upgrades/lib/solmate/src/

src/L2Comptroller.sol

@@ -20,7 +20,10 @@ contract L2Comptroller is OwnableUpgradeable, PausableUpgradeable {
     event ModifiedMaxTokenPriceDrop(uint256 newMaxTokenPriceDrop);
     event EmergencyWithdrawal(address indexed token, uint256 amount);
     event RequireErrorDuringBuyBack(address indexed depositor, string reason);
-    event AssertionErrorDuringBuyBack(address indexed depositor, uint256 errorCode);
+    event AssertionErrorDuringBuyBack(
+        address indexed depositor,
+        uint256 errorCode
+    );
     event LowLevelErrorDuringBuyBack(address indexed depositor, bytes reason);
     event TokensClaimed(
         address indexed depositor,
@@ -38,10 +41,6 @@ contract L2Comptroller is OwnableUpgradeable, PausableUpgradeable {
         uint256 minAcceptablePrice,
         uint256 actualPrice
     );
-    error BuyTokenAlreadyClaimed(
-        address l1Depositor,
-        uint256 totalAmountClaimed
-    );
     error ExceedingClaimableAmount(
         address depositor,
         uint256 maxClaimableAmount,
@@ -193,12 +192,16 @@ contract L2Comptroller is OwnableUpgradeable, PausableUpgradeable {
 
         // The cumulative token amount burnt and claimed against on L2 should never be less than
         // what's been burnt on L1. This indicates some serious issues.
-        assert(totalAmountClaimed < totalAmountBurntOnL1);
+        assert(totalAmountClaimed <= totalAmountBurntOnL1);
 
         // The difference of both these variables tell us the claimable token amount in `tokenToBurn`
         // denomination.
         uint256 burnTokenAmount = totalAmountBurntOnL1 - totalAmountClaimed;
 
+        if (burnTokenAmount == 0) {
+            revert ExceedingClaimableAmount(l1Depositor, 0, 0);
+        }
+
         // Store the new total amount of tokens burnt on L1 and claimed against on L2.
         l1BurntAmountOf[l1Depositor] = totalAmountBurntOnL1;
 
@@ -253,21 +256,21 @@ contract L2Comptroller is OwnableUpgradeable, PausableUpgradeable {
         uint256 totalAmountClaimed = claimedAmountOf[msg.sender];
         uint256 totalAmountBurntOnL1 = l1BurntAmountOf[msg.sender];
 
-        // If the tokens have been claimed already then, revert the transaction.
-        // TODO: Remove this check as `burnTokenAmount` calculation already handles this.
-        if (totalAmountClaimed == totalAmountBurntOnL1)
-            revert BuyTokenAlreadyClaimed(msg.sender, totalAmountBurntOnL1);
-
         // The cumulative token amount burnt and claimed against on L2 should never be less than
         // what's been burnt on L1. This indicates some serious issues.
-        assert(totalAmountClaimed < totalAmountBurntOnL1);
+        assert(totalAmountClaimed <= totalAmountBurntOnL1);
 
         // The difference of both these variables tells us the remaining claimable token amount in `tokenToBurn`
         // denomination.
         uint256 remainingBurnTokenAmount = totalAmountBurntOnL1 -
             totalAmountClaimed;
 
-        if (burnTokenAmount > remainingBurnTokenAmount)
+        // Will revert in case there are no tokens remaining to be claimed by the user or the amount they
+        // asked for exceeds their claimable amount.
+        if (
+            burnTokenAmount > remainingBurnTokenAmount ||
+            remainingBurnTokenAmount == 0
+        )
             revert ExceedingClaimableAmount(
                 msg.sender,
                 remainingBurnTokenAmount,

test/L1Comptroller/fuzz-tests/BuyBackFuzz.t.sol

@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity 0.8.18;
+
+import {Setup} from "../../helpers/Setup.sol";
+import {SafeERC20Upgradeable} from "openzeppelin-contracts-upgradeable/contracts/token/ERC20/utils/SafeERC20Upgradeable.sol";
+import {IERC20Upgradeable} from "openzeppelin-contracts-upgradeable/contracts/interfaces/IERC20Upgradeable.sol";
+import {L1Comptroller} from "../../../src/L1Comptroller.sol";
+import "forge-std/Test.sol";
+import "forge-std/console.sol";
+
+contract BuyBackFuzz is Setup {
+    using SafeERC20Upgradeable for IERC20Upgradeable;
+    using stdStorage for StdStorage;
+
+    function setUp() public override {
+        super.setUp();
+        vm.selectFork(l1ForkId);
+    } 
+
+    function testFuzz_ShouldBurnCorrectAmountOfTokens_WhenReceiverIsSender(uint256 tokenToBurnAmount) public {
+        uint256 tokenSupplyBefore = tokenToBurnL1.totalSupply();
+
+        // Make sure the fuzzer gives amount less than the token supply.
+        tokenToBurnAmount = bound(tokenToBurnAmount, 1, tokenSupplyBefore);
+
+        deal(address(tokenToBurnL1), alice, tokenToBurnAmount);
+
+        uint256 aliceBalanceBefore = tokenToBurnL1.balanceOf(alice);
+
+        vm.startPrank(alice);
+
+        // Approve the MTA tokens to the L1Comptroller for burn.
+        IERC20Upgradeable(tokenToBurnL1).safeIncreaseAllowance(
+            address(L1ComptrollerProxy),
+            tokenToBurnAmount
+        );
+
+        // Expecting a call to be made to the Optimism's cross domain messenger contract on L1
+        // with the relevant data.
+        vm.expectCall(
+            address(L1DomainMessenger),
+            abi.encodeCall(
+                L1DomainMessenger.sendMessage,
+                (
+                    address(L2ComptrollerProxy),
+                    abi.encodeWithSignature(
+                        "buyBackFromL1(address,address,uint256)",
+                        alice,
+                        alice,
+                        tokenToBurnAmount
+                    ),
+                    1_920_000
+                )
+            )
+        );
+
+        L1ComptrollerProxy.buyBack(alice, tokenToBurnAmount);
+
+        assertEq(
+            tokenToBurnL1.balanceOf(alice),
+            aliceBalanceBefore - tokenToBurnAmount,
+            "Wrong Alice's balance after burn"
+        );
+        assertEq(
+            tokenToBurnL1.totalSupply(),
+            tokenSupplyBefore - tokenToBurnAmount,
+            "Wrong total supply"
+        );
+        assertEq(
+            L1ComptrollerProxy.burntAmountOf(alice),
+            tokenToBurnAmount,
+            "Burnt amount not updated"
+        );
+    }
+
+    function testFuzz_ShouldBurnCorrectAmountOfTokens_WhenReceiverIsNotSender(uint256 tokenToBurnAmount) public {
+        uint256 tokenSupplyBefore = tokenToBurnL1.totalSupply();
+
+        // Make sure the fuzzer gives amount less than the token supply.
+        tokenToBurnAmount = bound(tokenToBurnAmount, 1, tokenSupplyBefore);
+
+        deal(address(tokenToBurnL1), alice, tokenToBurnAmount);
+
+        uint256 aliceBalanceBefore = tokenToBurnL1.balanceOf(alice);
+        address dummyReceiver = makeAddr("dummyReceiver");
+
+        vm.startPrank(alice);
+
+        // Approve the MTA tokens to the L1Comptroller for burn.
+        IERC20Upgradeable(tokenToBurnL1).safeIncreaseAllowance(
+            address(L1ComptrollerProxy),
+            tokenToBurnAmount
+        );
+
+        // Expecting a call to be made to the Optimism's cross domain messenger contract on L1
+        // with the relevant data.
+        vm.expectCall(
+            address(L1DomainMessenger),
+            abi.encodeCall(
+                L1DomainMessenger.sendMessage,
+                (
+                    address(L2ComptrollerProxy),
+                    abi.encodeWithSignature(
+                        "buyBackFromL1(address,address,uint256)",
+                        alice,
+                        dummyReceiver,
+                        tokenToBurnAmount
+                    ),
+                    1_920_000
+                )
+            )
+        );
+
+        L1ComptrollerProxy.buyBack(dummyReceiver, tokenToBurnAmount);
+
+        assertEq(
+            tokenToBurnL1.balanceOf(alice),
+            aliceBalanceBefore - tokenToBurnAmount,
+            "Wrong Alice's balance after burn"
+        );
+        assertEq(
+            tokenToBurnL1.totalSupply(),
+            tokenSupplyBefore - tokenToBurnAmount,
+            "Wrong total supply"
+        );
+        assertEq(
+            L1ComptrollerProxy.burntAmountOf(alice),
+            tokenToBurnAmount,
+            "Burnt amount not updated"
+        );
+    }
+}

test/L1Comptroller/BuyBack.t.sol → test/L1Comptroller/integration-tests/BuyBack.t.sol

@@ -1,14 +1,14 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity 0.8.18;
 
-import {Setup} from "../helpers/Setup.sol";
+import {Setup} from "../../helpers/Setup.sol";
 import {SafeERC20Upgradeable} from "openzeppelin-contracts-upgradeable/contracts/token/ERC20/utils/SafeERC20Upgradeable.sol";
 import {IERC20Upgradeable} from "openzeppelin-contracts-upgradeable/contracts/interfaces/IERC20Upgradeable.sol";
-import {L1Comptroller} from "../../src/L1Comptroller.sol";
+import {L1Comptroller} from "../../../src/L1Comptroller.sol";
 import "forge-std/Test.sol";
 import "forge-std/console.sol";
 
-contract BuyBackOnL2 is Setup {
+contract BuyBack is Setup {
     using SafeERC20Upgradeable for IERC20Upgradeable;
     using stdStorage for StdStorage;
 

test/L1Comptroller/EmergencyWithdraw.t.sol → test/L1Comptroller/integration-tests/EmergencyWithdraw.t.sol

@@ -1,11 +1,11 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity 0.8.18;
 
-import {Setup} from "../helpers/Setup.sol";
+import {Setup} from "../../helpers/Setup.sol";
 import {ERC20} from "openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
 import {SafeERC20Upgradeable} from "openzeppelin-contracts-upgradeable/contracts/token/ERC20/utils/SafeERC20Upgradeable.sol";
 import {IERC20Upgradeable} from "openzeppelin-contracts-upgradeable/contracts/interfaces/IERC20Upgradeable.sol";
-import {L1Comptroller} from "../../src/L1Comptroller.sol";
+import {L1Comptroller} from "../../../src/L1Comptroller.sol";
 import "forge-std/Test.sol";
 import "forge-std/console.sol";
 

test/L1Comptroller/Pausable.t.sol → test/L1Comptroller/integration-tests/Pausable.t.sol

@@ -1,10 +1,10 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity 0.8.18;
 
-import {Setup} from "../helpers/Setup.sol";
+import {Setup} from "../../helpers/Setup.sol";
 import {SafeERC20Upgradeable} from "openzeppelin-contracts-upgradeable/contracts/token/ERC20/utils/SafeERC20Upgradeable.sol";
 import {IERC20Upgradeable} from "openzeppelin-contracts-upgradeable/contracts/interfaces/IERC20Upgradeable.sol";
-import {L1Comptroller} from "../../src/L1Comptroller.sol";
+import {L1Comptroller} from "../../../src/L1Comptroller.sol";
 import "forge-std/Test.sol";
 import "forge-std/console.sol";
 

test/L1Comptroller/SetCrossChainGasLimit.t.sol → test/L1Comptroller/integration-tests/SetCrossChainGasLimit.t.sol

@@ -1,10 +1,10 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity 0.8.18;
 
-import {Setup} from "../helpers/Setup.sol";
+import {Setup} from "../../helpers/Setup.sol";
 import {SafeERC20Upgradeable} from "openzeppelin-contracts-upgradeable/contracts/token/ERC20/utils/SafeERC20Upgradeable.sol";
 import {IERC20Upgradeable} from "openzeppelin-contracts-upgradeable/contracts/interfaces/IERC20Upgradeable.sol";
-import {L1Comptroller} from "../../src/L1Comptroller.sol";
+import {L1Comptroller} from "../../../src/L1Comptroller.sol";
 import "forge-std/Test.sol";
 import "forge-std/console.sol";
 

test/L1Comptroller/SetL2Comptroller.t.sol → test/L1Comptroller/integration-tests/SetL2Comptroller.t.sol

@@ -1,10 +1,10 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity 0.8.18;
 
-import {Setup} from "../helpers/Setup.sol";
+import {Setup} from "../../helpers/Setup.sol";
 import {SafeERC20Upgradeable} from "openzeppelin-contracts-upgradeable/contracts/token/ERC20/utils/SafeERC20Upgradeable.sol";
 import {IERC20Upgradeable} from "openzeppelin-contracts-upgradeable/contracts/interfaces/IERC20Upgradeable.sol";
-import {L1Comptroller} from "../../src/L1Comptroller.sol";
+import {L1Comptroller} from "../../../src/L1Comptroller.sol";
 import "forge-std/Test.sol";
 import "forge-std/console.sol";