Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add gas measurement to tests and refactor some test structures #337

Merged
merged 1 commit into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
[submodule "lib/openzeppelin-contracts"]
path = lib/openzeppelin-contracts
url = https://github.com/OpenZeppelin/openzeppelin-contracts
branch = v5.0.2
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The branch entry is an optional setting. However, its inclusion causes forge update to fail when updating dependencies.

Reference: foundry-rs/foundry#3720 (comment)

[submodule "lib/forge-std"]
path = lib/forge-std
url = https://github.com/foundry-rs/forge-std
branch = v1.3.0
4 changes: 4 additions & 0 deletions snapshots/AdminManagement.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"approveTokens(): testApproveTokens": "136967",
"rescueTokens(): testRescueTokens": "86851"
}
8 changes: 8 additions & 0 deletions snapshots/AllowanceTarget.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"pause(): testSpendFromUserToAfterUnpause": "27667",
"spendFromUserTo(): testSpendFromUserTo": "63533",
"spendFromUserTo(): testSpendFromUserToAfterUnpause": "63533",
"spendFromUserTo(): testSpendFromUserToWithDeflationaryToken": "91576",
"spendFromUserTo(): testSpendFromUserToWithNoReturnValueToken": "68943",
"unpause(): testSpendFromUserToAfterUnpause": "27645"
}
11 changes: 11 additions & 0 deletions snapshots/Asset.json
Original file line number Diff line number Diff line change
@@ -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"
}
6 changes: 6 additions & 0 deletions snapshots/CoordinatedTaker.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"approveTokens(): testApproveTokens": "54307",
"setCoordinator(): testSetCoordinator": "30043",
"submitLimitOrderFill(): testFillWithETH": "192971",
"submitLimitOrderFill(): testFillWithPermission": "256473"
}
5 changes: 5 additions & 0 deletions snapshots/EIP712.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"EIP712_DOMAIN_SEPARATOR(): testDomainSeparatorOnChain": "276",
"EIP712_DOMAIN_SEPARATOR(): testDomainSeparatorOnDifferentChain": "626",
"getEIP712Hash(): testGetEIP712Hash": "828"
}
9 changes: 9 additions & 0 deletions snapshots/GenericSwap.json
Original file line number Diff line number Diff line change
@@ -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"
}
20 changes: 20 additions & 0 deletions snapshots/LimitOrderSwap.json
Original file line number Diff line number Diff line change
@@ -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"
}
5 changes: 5 additions & 0 deletions snapshots/Ownable.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"acceptOwnership(): testAcceptOwnership": "28390",
"nominateNewOwner(): testNominateNewOwner": "47132",
"renounceOwnership(): testRenounceOwnership": "25351"
}
15 changes: 15 additions & 0 deletions snapshots/RFQ.json
Original file line number Diff line number Diff line change
@@ -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"
}
11 changes: 11 additions & 0 deletions snapshots/SmartOrderStrategy.json
Original file line number Diff line number Diff line change
@@ -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"
}
7 changes: 7 additions & 0 deletions snapshots/TokenCollector.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"collect(): testCollectByAllowanceTarget": "66338",
"collect(): testCollectByPermit2AllowanceTransfer": "102281",
"collect(): testCollectByPermit2SignatureTransfer": "94388",
"collect(): testCollectByTokenApproval": "58597",
"collect(): testCollectByTokenPermit": "93729"
}
34 changes: 25 additions & 9 deletions test/AllowanceTarget.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 {
Expand All @@ -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);
Expand All @@ -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));
Expand All @@ -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));
Expand All @@ -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));
Expand All @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand All @@ -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);
Expand Down
33 changes: 20 additions & 13 deletions test/abstracts/EIP712.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
}
}
Loading
Loading