Merge pull request #337 from consenlabs/test/add-gas-snapshot

add gas measurement to tests and refactor some test structures

.gitmodules

@@ -1,8 +1,6 @@
 [submodule "lib/openzeppelin-contracts"]
 	path = lib/openzeppelin-contracts
 	url = https://github.com/OpenZeppelin/openzeppelin-contracts
-	branch = v5.0.2
 [submodule "lib/forge-std"]
 	path = lib/forge-std
 	url = https://github.com/foundry-rs/forge-std
-	branch = v1.3.0

snapshots/AdminManagement.json

@@ -0,0 +1,4 @@
+{
+  "approveTokens(): testApproveTokens": "136967",
+  "rescueTokens(): testRescueTokens": "86851"
+}

snapshots/AllowanceTarget.json

@@ -0,0 +1,8 @@
+{
+  "pause(): testSpendFromUserToAfterUnpause": "27667",
+  "spendFromUserTo(): testSpendFromUserTo": "63533",
+  "spendFromUserTo(): testSpendFromUserToAfterUnpause": "63533",
+  "spendFromUserTo(): testSpendFromUserToWithDeflationaryToken": "91576",
+  "spendFromUserTo(): testSpendFromUserToWithNoReturnValueToken": "68943",
+  "unpause(): testSpendFromUserToAfterUnpause": "27645"
+}

snapshots/Asset.json

@@ -0,0 +1,11 @@
+{
+  "getBalance(): testGetBalance": "6107",
+  "getBalance(): testGetBalance(ETH_ADDRESS)": "764",
+  "getBalance(): testGetBalance(ZERO_ADDRESS)": "781",
+  "isETH(): testIsETH(ETH_ADDRESS)": "441",
+  "isETH(): testIsETH2(ZERO_ADDRESS)": "458",
+  "transferTo(): testDoNothingIfTransferToSelf": "22431",
+  "transferTo(): testDoNothingIfTransferWithZeroAmount": "22419",
+  "transferTo(): testTransferETH": "57012",
+  "transferTo(): testTransferToken": "51501"
+}

snapshots/CoordinatedTaker.json

@@ -0,0 +1,6 @@
+{
+  "approveTokens(): testApproveTokens": "54307",
+  "setCoordinator(): testSetCoordinator": "30043",
+  "submitLimitOrderFill(): testFillWithETH": "192971",
+  "submitLimitOrderFill(): testFillWithPermission": "256473"
+}

snapshots/EIP712.json

@@ -0,0 +1,5 @@
+{
+  "EIP712_DOMAIN_SEPARATOR(): testDomainSeparatorOnChain": "276",
+  "EIP712_DOMAIN_SEPARATOR(): testDomainSeparatorOnDifferentChain": "626",
+  "getEIP712Hash(): testGetEIP712Hash": "828"
+}

snapshots/GenericSwap.json

@@ -0,0 +1,9 @@
+{
+  "executeSwap(): testGenericSwapWithUniswap": "251909",
+  "executeSwap(): testLeaveOneWeiWithMultipleUsers(the first deposit)": "251909",
+  "executeSwap(): testLeaveOneWeiWithMultipleUsers(the second deposit)": "208263",
+  "executeSwap(): testSwapWithETHInput": "100382",
+  "executeSwap(): testSwapWithETHOutput": "130241",
+  "executeSwap(): testSwapWithLessOutputButWithinTolerance": "160688",
+  "executeSwapWithSig(): testGenericSwapRelayed": "283756"
+}

snapshots/LimitOrderSwap.json

@@ -0,0 +1,20 @@
+{
+  "cancelOrder(): testCancelOrder": "52753",
+  "fillLimitOrder(): testFillLimitOrderWithETH": "158029",
+  "fillLimitOrder(): testFillWithBetterTakingAmount": "215471",
+  "fillLimitOrder(): testFillWithBetterTakingAmountButGetAdjusted": "215705",
+  "fillLimitOrder(): testFillWithETHRefund": "165308",
+  "fillLimitOrder(): testFillWithLargerVolumeAndSettleAsManyAsPossible": "215693",
+  "fillLimitOrder(): testFillWithoutMakerSigForVerifiedOrder": "215471",
+  "fillLimitOrder(): testFillWithoutMakerSigForVerifiedOrder(without makerSig)": "120983",
+  "fillLimitOrder(): testFullyFillLimitOrder": "215471",
+  "fillLimitOrder(): testFullyFillLimitOrderUsingAMM": "235046",
+  "fillLimitOrder(): testPartiallyFillLimitOrder": "215471",
+  "fillLimitOrderFullOrKill(): testFillWithFOK": "215449",
+  "fillLimitOrderGroup(): testGroupFillRingTrade": "291749",
+  "fillLimitOrderGroup(): testGroupFillWithPartialWETHUnwrap": "277818",
+  "fillLimitOrderGroup(): testGroupFillWithTakerPrefundETH": "198117",
+  "fillLimitOrderGroup(): testGroupFillWithWETHUnwrap": "198117",
+  "fillLimitOrderGroup(): testPartialFillLargeOrderWithSmallOrders": "265910",
+  "testGroupFillWithProfit: fillLimitOrderGroup()": "225225"
+}

snapshots/Ownable.json

@@ -0,0 +1,5 @@
+{
+  "acceptOwnership(): testAcceptOwnership": "28390",
+  "nominateNewOwner(): testNominateNewOwner": "47132",
+  "renounceOwnership(): testRenounceOwnership": "25351"
+}

snapshots/RFQ.json

@@ -0,0 +1,15 @@
+{
+  "cancelRFQOffer(): testCancelRFQOffer": "49848",
+  "fillRFQ(): testFillRFQ": "219666",
+  "fillRFQ(): testFillRFQTakerGetRawETH": "238797",
+  "fillRFQ(): testFillRFQWithRawETH": "159563",
+  "fillRFQ(): testFillRFQWithRawETHAndReceiveWETH": "174630",
+  "fillRFQ(): testFillRFQWithTakerApproveAllowanceTarget": "187631",
+  "fillRFQ(): testFillRFQWithWETH": "222131",
+  "fillRFQ(): testFillRFQWithWETHAndReceiveWETH": "208373",
+  "fillRFQ(): testFillRFQWithZeroFee": "192740",
+  "fillRFQ(): testFillWithContract": "223381",
+  "fillRFQ(): testPartialFill": "219826",
+  "fillRFQWithSig(): testFillRFQByTakerSig": "228592",
+  "setFeeCollector(): testSetFeeCollector": "30099"
+}

snapshots/SmartOrderStrategy.json

@@ -0,0 +1,11 @@
+{
+  "executeStrategy(): testMultipleAMMs": "610173",
+  "executeStrategy(): testUniswapV2WithWETHUnwrap": "142340",
+  "executeStrategy(): testUniswapV3WithAmountReplace": "161811",
+  "executeStrategy(): testUniswapV3WithMaxAmountReplace": "161607",
+  "executeStrategy(): testUniswapV3WithoutAmountReplace": "155017",
+  "executeStrategy(): testV6LOIntegration": "180661",
+  "executeStrategy(): testV6RFQIntegration": "183394",
+  "executeStrategy(): testV6RFQIntegrationWhenMakerTokenIsETH": "149739",
+  "executeStrategy(): testV6RFQIntegrationWhenTakerTokenIsETH": "204956"
+}

snapshots/TokenCollector.json

@@ -0,0 +1,7 @@
+{
+  "collect(): testCollectByAllowanceTarget": "66338",
+  "collect(): testCollectByPermit2AllowanceTransfer": "102281",
+  "collect(): testCollectByPermit2SignatureTransfer": "94388",
+  "collect(): testCollectByTokenApproval": "58597",
+  "collect(): testCollectByTokenPermit": "93729"
+}

test/AllowanceTarget.t.sol

@@ -32,7 +32,6 @@ contract AllowanceTargetTest is BalanceUtil {
     MockNoRevertERC20 noRevertERC20 = new MockNoRevertERC20();
     IERC20[] tokens = [IERC20(mockERC20), IERC20(address(deflationaryERC20)), IERC20(address(noReturnERC20)), IERC20(address(noRevertERC20))];
 
-    // effectively a "beforeEach" block
     function setUp() public {
         // Deploy
         allowanceTarget = new AllowanceTarget(allowanceTargetOwner, trusted);
@@ -58,16 +57,20 @@ contract AllowanceTargetTest is BalanceUtil {
 
     function testCannotSpendFromUserInsufficientBalanceWithNoReturnValueToken() public {
         uint256 userBalance = noReturnERC20.balanceOf(user);
+
+        vm.startPrank(authorized);
         vm.expectRevert("ERC20: transfer amount exceeds balance");
-        vm.prank(authorized);
         allowanceTarget.spendFromUserTo(user, address(noReturnERC20), recipient, userBalance + 1);
+        vm.stopPrank();
     }
 
     function testCannotSpendFromUserInsufficientBalanceWithReturnFalseToken() public {
         uint256 userBalance = noRevertERC20.balanceOf(user);
+
+        vm.startPrank(authorized);
         vm.expectRevert(abi.encodeWithSelector(SafeERC20.SafeERC20FailedOperation.selector, address(noRevertERC20)));
-        vm.prank(authorized);
         allowanceTarget.spendFromUserTo(user, address(noRevertERC20), recipient, userBalance + 1);
+        vm.stopPrank();
     }
 
     function testCannotPauseIfNotOwner() public {
@@ -76,16 +79,18 @@ contract AllowanceTargetTest is BalanceUtil {
     }
 
     function testCannotUnpauseIfNotOwner() public {
-        vm.prank(allowanceTargetOwner, allowanceTargetOwner);
+        vm.startPrank(allowanceTargetOwner);
         allowanceTarget.pause();
+        vm.stopPrank();
 
         vm.expectRevert(Ownable.NotOwner.selector);
         allowanceTarget.unpause();
     }
 
     function testCannotSpendIfPaused() public {
-        vm.prank(allowanceTargetOwner, allowanceTargetOwner);
+        vm.startPrank(allowanceTargetOwner);
         allowanceTarget.pause();
+        vm.stopPrank();
 
         vm.expectRevert(Pausable.EnforcedPause.selector);
         allowanceTarget.spendFromUserTo(user, address(mockERC20), recipient, 1234);
@@ -96,8 +101,11 @@ contract AllowanceTargetTest is BalanceUtil {
         Snapshot memory toBalance = BalanceSnapshot.take({ owner: recipient, token: address(mockERC20) });
 
         uint256 amount = 100;
-        vm.prank(authorized);
+
+        vm.startPrank(authorized);
         allowanceTarget.spendFromUserTo(user, address(mockERC20), recipient, amount);
+        vm.stopPrank();
+        vm.snapshotGasLastCall("AllowanceTarget", "spendFromUserTo(): testSpendFromUserTo");
 
         fromBalance.assertChange(-int256(amount));
         toBalance.assertChange(int256(amount));
@@ -111,11 +119,15 @@ contract AllowanceTargetTest is BalanceUtil {
 
         vm.startPrank(allowanceTargetOwner);
         allowanceTarget.pause();
+        vm.snapshotGasLastCall("AllowanceTarget", "pause(): testSpendFromUserToAfterUnpause");
         allowanceTarget.unpause();
+        vm.snapshotGasLastCall("AllowanceTarget", "unpause(): testSpendFromUserToAfterUnpause");
         vm.stopPrank();
 
-        vm.prank(authorized);
+        vm.startPrank(authorized);
         allowanceTarget.spendFromUserTo(user, address(mockERC20), recipient, amount);
+        vm.stopPrank();
+        vm.snapshotGasLastCall("AllowanceTarget", "spendFromUserTo(): testSpendFromUserToAfterUnpause");
 
         fromBalance.assertChange(-int256(amount));
         toBalance.assertChange(int256(amount));
@@ -126,8 +138,10 @@ contract AllowanceTargetTest is BalanceUtil {
         Snapshot memory toBalance = BalanceSnapshot.take({ owner: recipient, token: address(noReturnERC20) });
 
         uint256 amount = 100;
-        vm.prank(authorized);
+        vm.startPrank(authorized);
         allowanceTarget.spendFromUserTo(user, address(noReturnERC20), recipient, amount);
+        vm.stopPrank();
+        vm.snapshotGasLastCall("AllowanceTarget", "spendFromUserTo(): testSpendFromUserToWithNoReturnValueToken");
 
         fromBalance.assertChange(-int256(amount));
         toBalance.assertChange(int256(amount));
@@ -138,8 +152,10 @@ contract AllowanceTargetTest is BalanceUtil {
         Snapshot memory toBalance = BalanceSnapshot.take({ owner: recipient, token: address(deflationaryERC20) });
 
         uint256 amount = 100;
-        vm.prank(authorized);
+        vm.startPrank(authorized);
         allowanceTarget.spendFromUserTo(user, address(deflationaryERC20), recipient, 100);
+        vm.stopPrank();
+        vm.snapshotGasLastCall("AllowanceTarget", "spendFromUserTo(): testSpendFromUserToWithDeflationaryToken");
 
         uint256 expectedReceive = 99; // MockDeflationaryERC20 will burn 1% during each transfer
         fromBalance.assertChange(-int256(amount));

test/AdminManagement.t.sol → test/abstracts/AdminManagement.t.sol

@@ -31,17 +31,19 @@ contract AdminManagementTest is BalanceUtil {
     }
 
     function testApproveTokens() public {
-        for (uint256 i = 0; i < tokens.length; ++i) {
-            for (uint256 j = 0; j < spenders.length; ++j) {
+        for (uint256 i; i < tokens.length; ++i) {
+            for (uint256 j; j < spenders.length; ++j) {
                 assertEq(IERC20(tokens[i]).allowance(address(contractWithAdmin), spenders[j]), 0);
             }
         }
 
-        vm.prank(owner);
+        vm.startPrank(owner);
         contractWithAdmin.approveTokens(tokens, spenders);
+        vm.stopPrank();
+        vm.snapshotGasLastCall("AdminManagement", "approveTokens(): testApproveTokens");
 
-        for (uint256 i = 0; i < tokens.length; ++i) {
-            for (uint256 j = 0; j < spenders.length; ++j) {
+        for (uint256 i; i < tokens.length; ++i) {
+            for (uint256 j; j < spenders.length; ++j) {
                 assertEq(IERC20(tokens[i]).allowance(address(contractWithAdmin), spenders[j]), type(uint256).max);
             }
         }
@@ -63,8 +65,10 @@ contract AdminManagementTest is BalanceUtil {
         assertEq(IERC20(tokens[1]).balanceOf(address(contractWithAdmin)), amount2);
         assertEq(IERC20(tokens[1]).balanceOf(rescueTarget), 0);
 
-        vm.prank(owner);
+        vm.startPrank(owner);
         contractWithAdmin.rescueTokens(tokens, rescueTarget);
+        vm.stopPrank();
+        vm.snapshotGasLastCall("AdminManagement", "rescueTokens(): testRescueTokens");
 
         assertEq(IERC20(tokens[0]).balanceOf(address(contractWithAdmin)), 0);
         assertEq(IERC20(tokens[0]).balanceOf(rescueTarget), amount1);

test/abstracts/EIP712.t.sol

@@ -5,48 +5,55 @@ import { Test } from "forge-std/Test.sol";
 import { EIP712 } from "contracts/abstracts/EIP712.sol";
 
 contract EIP712Test is Test {
-    EIP712TestContract eip712;
+    EIP712Harness eip712Harness;
 
     // Dummy struct hash for testing
     bytes32 public constant DUMMY_STRUCT_HASH = keccak256("DummyStruct(string message)");
 
     function setUp() public {
-        eip712 = new EIP712TestContract();
+        eip712Harness = new EIP712Harness();
     }
 
     function testOriginalChainId() public {
         uint256 chainId = block.chainid;
-        assertEq(eip712.originalChainId(), chainId);
+        assertEq(eip712Harness.originalChainId(), chainId);
     }
 
     function testOriginalDomainSeparator() public {
-        bytes32 expectedDomainSeparator = eip712.calculateDomainSeparator();
-        assertEq(eip712.originalEIP712DomainSeparator(), expectedDomainSeparator);
+        bytes32 expectedDomainSeparator = eip712Harness.calculateDomainSeparator();
+        assertEq(eip712Harness.originalEIP712DomainSeparator(), expectedDomainSeparator);
     }
 
     function testGetEIP712Hash() public {
         bytes32 structHash = DUMMY_STRUCT_HASH;
-        bytes32 domainSeparator = eip712.calculateDomainSeparator();
-        bytes32 expectedEIP712Hash = keccak256(abi.encodePacked(eip712.EIP191_HEADER(), domainSeparator, structHash));
+        bytes32 domainSeparator = eip712Harness.calculateDomainSeparator();
+        bytes32 expectedEIP712Hash = keccak256(abi.encodePacked(eip712Harness.EIP191_HEADER(), domainSeparator, structHash));
 
-        assertEq(eip712.getEIP712HashWrap(structHash), expectedEIP712Hash);
+        assertEq(eip712Harness.exposedGetEIP712Hash(structHash), expectedEIP712Hash);
+        vm.snapshotGasLastCall("EIP712", "getEIP712Hash(): testGetEIP712Hash");
     }
 
     function testDomainSeparatorOnDifferentChain() public {
         uint256 chainId = block.chainid + 1234;
         vm.chainId(chainId);
 
-        bytes32 newDomainSeparator = eip712.calculateDomainSeparator();
-        assertEq(eip712.EIP712_DOMAIN_SEPARATOR(), newDomainSeparator, "Domain separator should match the expected value on a different chain");
+        bytes32 newDomainSeparator = eip712Harness.calculateDomainSeparator();
+        assertEq(eip712Harness.EIP712_DOMAIN_SEPARATOR(), newDomainSeparator, "Domain separator should match the expected value on a different chain");
+        vm.snapshotGasLastCall("EIP712", "EIP712_DOMAIN_SEPARATOR(): testDomainSeparatorOnDifferentChain");
+    }
+
+    function testDomainSeparatorOnChain() public {
+        eip712Harness.EIP712_DOMAIN_SEPARATOR();
+        vm.snapshotGasLastCall("EIP712", "EIP712_DOMAIN_SEPARATOR(): testDomainSeparatorOnChain");
     }
 }
 
-contract EIP712TestContract is EIP712 {
+contract EIP712Harness is EIP712 {
     function calculateDomainSeparator() external view returns (bytes32) {
         return keccak256(abi.encode(EIP712_TYPE_HASH, keccak256(bytes(EIP712_NAME)), keccak256(bytes(EIP712_VERSION)), block.chainid, address(this)));
     }
 
-    function getEIP712HashWrap(bytes32 structHash) public view returns (bytes32) {
-        return super.getEIP712Hash(structHash);
+    function exposedGetEIP712Hash(bytes32 structHash) public view returns (bytes32) {
+        return getEIP712Hash(structHash);
     }
 }