From 29ee9c285e28991e6f2c9d88a7db564564247995 Mon Sep 17 00:00:00 2001 From: wildmolasses Date: Fri, 29 Sep 2023 17:20:58 -0400 Subject: [PATCH] L1 gas golfing --- src/FakeERC20.sol | 3 ++- src/L1VotePool.sol | 16 ++++++++-------- src/WormholeL1ERC20Bridge.sol | 13 ++++++------- src/WormholeL2ERC20.sol | 5 ++--- src/WormholeL2GovernorMetadata.sol | 2 +- src/WormholeReceiver.sol | 2 +- test/WormholeL1ERC20Bridge.t.sol | 2 +- test/WormholeL1VotePool.t.sol | 18 +++++++++--------- test/WormholeL2ERC20.t.sol | 8 ++++---- test/WormholeL2VoteAggregator.t.sol | 3 --- test/WormholeReceiver.t.sol | 2 +- test/harness/L1VotePoolHarness.sol | 8 ++++---- 12 files changed, 39 insertions(+), 43 deletions(-) diff --git a/src/FakeERC20.sol b/src/FakeERC20.sol index 126068a3..1b3b57e3 100644 --- a/src/FakeERC20.sol +++ b/src/FakeERC20.sol @@ -2,12 +2,13 @@ pragma solidity ^0.8.20; import {ERC20VotesComp} from "openzeppelin/token/ERC20/extensions/ERC20VotesComp.sol"; +import {ERC20Votes} from "openzeppelin/token/ERC20/extensions/ERC20Votes.sol"; import {ERC20Permit} from "openzeppelin/token/ERC20/extensions/ERC20Permit.sol"; import {ERC20} from "openzeppelin/token/ERC20/ERC20.sol"; import {IERC20Mint} from "src/interfaces/IERC20Mint.sol"; /// @notice An ERC20Votes token to help test the L2 voting system -contract FakeERC20 is ERC20VotesComp, IERC20Mint { +contract FakeERC20 is ERC20Votes, IERC20Mint { constructor(string memory _name, string memory _symbol) ERC20(_name, _symbol) ERC20Permit(_name) {} /// @dev Mints tokens to an address to help test bridging and voting. diff --git a/src/L1VotePool.sol b/src/L1VotePool.sol index 2f0ab12f..9d045525 100644 --- a/src/L1VotePool.sol +++ b/src/L1VotePool.sol @@ -5,7 +5,11 @@ import {IGovernor} from "openzeppelin/governance/Governor.sol"; abstract contract L1VotePool { /// @notice The address of the L1 Governor contract. - IGovernor public governor; + IGovernor public immutable GOVERNOR; + + // This param is ignored by the governor when voting with fractional + // weights. It makes no difference what vote type this is. + uint8 constant UNUSED_SUPPORT_PARAM = uint8(1); event VoteCast( address indexed voter, @@ -33,7 +37,7 @@ abstract contract L1VotePool { /// @param _governor The address of the L1 Governor contract. constructor(address _governor) { - governor = IGovernor(_governor); + GOVERNOR = IGovernor(_governor); } /// @notice Casts vote to the L1 Governor. @@ -41,13 +45,9 @@ abstract contract L1VotePool { function _castVote(uint256 proposalId, ProposalVote memory vote) internal { bytes memory votes = abi.encodePacked(vote.againstVotes, vote.forVotes, vote.abstainVotes); - // This param is ignored by the governor when voting with fractional - // weights. It makes no difference what vote type this is. - uint8 unusedSupportParam = uint8(1); - // TODO: string should probably mention chain - governor.castVoteWithReasonAndParams( - proposalId, unusedSupportParam, "rolled-up vote from governance L2 token holders", votes + GOVERNOR.castVoteWithReasonAndParams( + proposalId, UNUSED_SUPPORT_PARAM, "rolled-up vote from governance L2 token holders", votes ); emit VoteCast(msg.sender, proposalId, vote.againstVotes, vote.forVotes, vote.abstainVotes); diff --git a/src/WormholeL1ERC20Bridge.sol b/src/WormholeL1ERC20Bridge.sol index 14771099..d0f7dfb9 100644 --- a/src/WormholeL1ERC20Bridge.sol +++ b/src/WormholeL1ERC20Bridge.sol @@ -17,15 +17,15 @@ contract WormholeL1ERC20Bridge is WormholeL1VotePool, WormholeSender, WormholeRe /// @notice Token address which is minted on L2. address public L2_TOKEN_ADDRESS; - /// @notice A unique number used to send messages. - uint32 public nonce; - /// @notice Used to indicate whether the contract has been initialized with the L2 token address. bool public INITIALIZED = false; /// @dev Contract is already initialized with an L2 token. error AlreadyInitialized(); + /// @dev The value sent to the relayer must match the cost of the message. + error CostValueMismatch(); + event TokenBridged( address indexed sender, address indexed targetAddress, @@ -72,11 +72,10 @@ contract WormholeL1ERC20Bridge is WormholeL1VotePool, WormholeSender, WormholeRe function deposit(address account, uint256 amount) public payable returns (uint256 sequence) { L1_TOKEN.safeTransferFrom(msg.sender, address(this), amount); - // TODO optimize with encodePacked - bytes memory mintCalldata = abi.encode(account, amount); + bytes memory mintCalldata = abi.encodePacked(account, amount); uint256 cost = quoteDeliveryCost(TARGET_CHAIN); - require(cost == msg.value, "Cost should be msg.Value"); + if (cost != msg.value) revert CostValueMismatch(); emit TokenBridged(msg.sender, account, TARGET_CHAIN, amount, L2_TOKEN_ADDRESS); @@ -92,7 +91,7 @@ contract WormholeL1ERC20Bridge is WormholeL1VotePool, WormholeSender, WormholeRe } function receiveWormholeMessages( - bytes memory payload, + bytes calldata payload, bytes[] memory additionalVaas, bytes32 sourceAddress, uint16 sourceChain, diff --git a/src/WormholeL2ERC20.sol b/src/WormholeL2ERC20.sol index f920664b..057bc5b7 100644 --- a/src/WormholeL2ERC20.sol +++ b/src/WormholeL2ERC20.sol @@ -67,7 +67,7 @@ contract WormholeL2ERC20 is ERC20Votes, WormholeReceiver, WormholeSender { /// @notice Receives a message from L1 and mints L2 tokens. /// @param payload The payload that was sent to in the delivery request. function receiveWormholeMessages( - bytes memory payload, + bytes calldata payload, bytes[] memory, bytes32 sourceAddress, uint16 sourceChain, @@ -80,8 +80,7 @@ contract WormholeL2ERC20 is ERC20Votes, WormholeReceiver, WormholeSender { isRegisteredSender(sourceChain, sourceAddress) replayProtect(deliveryHash) { - (address account, uint256 amount) = abi.decode(payload, (address, uint256)); - _mint(account, amount); + _mint(address(bytes20(payload[:20])), uint224(bytes28(payload[20:48]))); } /// @dev Clock used for flagging checkpoints. diff --git a/src/WormholeL2GovernorMetadata.sol b/src/WormholeL2GovernorMetadata.sol index c0bab0df..bfa4e421 100644 --- a/src/WormholeL2GovernorMetadata.sol +++ b/src/WormholeL2GovernorMetadata.sol @@ -17,7 +17,7 @@ contract WormholeL2GovernorMetadata is L2GovernorMetadata, WormholeReceiver { /// @notice Receives a message from L1 and saves the proposal metadata. /// @param payload The payload that was sent to in the delivery request. function receiveWormholeMessages( - bytes memory payload, + bytes calldata payload, bytes[] memory, bytes32 sourceAddress, uint16 sourceChain, diff --git a/src/WormholeReceiver.sol b/src/WormholeReceiver.sol index d4c980b0..b7de8f6a 100644 --- a/src/WormholeReceiver.sol +++ b/src/WormholeReceiver.sol @@ -38,7 +38,7 @@ abstract contract WormholeReceiver is Ownable, WormholeBase { /// @param sourceChain Chain that the delivery was requested from. /// @param deliveryHash Unique identifier of this delivery request. function receiveWormholeMessages( - bytes memory payload, + bytes calldata payload, bytes[] memory additionalVaas, bytes32 sourceAddress, uint16 sourceChain, diff --git a/test/WormholeL1ERC20Bridge.t.sol b/test/WormholeL1ERC20Bridge.t.sol index 94fba183..265a85ea 100644 --- a/test/WormholeL1ERC20Bridge.t.sol +++ b/test/WormholeL1ERC20Bridge.t.sol @@ -161,7 +161,7 @@ contract _ReceiveWithdrawalWormholeMessages is Test, TestConstants { contract _Withdraw is Test, TestConstants { event Withdraw(address indexed account, uint256 amount); - function testFork_CorrectlyWithdrawTokens(address _account, uint96 _amount, address l2Erc20) + function testFork_CorrectlyWithdrawTokens(address _account, uint224 _amount, address l2Erc20) public { vm.assume(_account != address(0)); diff --git a/test/WormholeL1VotePool.t.sol b/test/WormholeL1VotePool.t.sol index d17d986d..f74b6e73 100644 --- a/test/WormholeL1VotePool.t.sol +++ b/test/WormholeL1VotePool.t.sol @@ -27,14 +27,14 @@ contract L1VotePoolHarness is WormholeL1VotePool, WormholeReceiver, Test { {} function _jumpToActiveProposal(uint256 proposalId) public { - uint256 _deadline = governor.proposalDeadline(proposalId); + uint256 _deadline = GOVERNOR.proposalDeadline(proposalId); vm.roll(_deadline - 1); } /// @dev We need this function because when we call `performDelivery` the proposal is not active, /// and it does not seem configurable in the wormhole sdk utilities. function receiveWormholeMessages( - bytes memory payload, + bytes calldata payload, bytes[] memory additionalVaas, bytes32 sourceAddress, uint16 sourceChain, @@ -63,7 +63,7 @@ contract L1VotePoolHarness is WormholeL1VotePool, WormholeReceiver, Test { } function cancel(address l1Erc20) public returns (uint256) { - bytes memory proposalCalldata = abi.encode(FakeERC20.mint.selector, address(governor), 100_000); + bytes memory proposalCalldata = abi.encode(FakeERC20.mint.selector, address(GOVERNOR), 100_000); address[] memory targets = new address[](1); bytes[] memory calldatas = new bytes[](1); @@ -74,11 +74,11 @@ contract L1VotePoolHarness is WormholeL1VotePool, WormholeReceiver, Test { values[0] = 0; return - governor.cancel(targets, values, calldatas, keccak256(bytes("Proposal: To inflate token"))); + GOVERNOR.cancel(targets, values, calldatas, keccak256(bytes("Proposal: To inflate token"))); } function _createExampleProposal(address l1Erc20) internal returns (uint256) { - bytes memory proposalCalldata = abi.encode(FakeERC20.mint.selector, address(governor), 100_000); + bytes memory proposalCalldata = abi.encode(FakeERC20.mint.selector, address(GOVERNOR), 100_000); address[] memory targets = new address[](1); bytes[] memory calldatas = new bytes[](1); @@ -88,7 +88,7 @@ contract L1VotePoolHarness is WormholeL1VotePool, WormholeReceiver, Test { calldatas[0] = proposalCalldata; values[0] = 0; - return governor.propose(targets, values, calldatas, "Proposal: To inflate token"); + return GOVERNOR.propose(targets, values, calldatas, "Proposal: To inflate token"); } function createProposalVote(address l1Erc20) public returns (uint256) { @@ -113,12 +113,12 @@ contract L1VotePoolHarness is WormholeL1VotePool, WormholeReceiver, Test { } function _jumpToProposalEnd(uint256 proposalId) external { - uint256 _deadline = governor.proposalDeadline(proposalId); + uint256 _deadline = GOVERNOR.proposalDeadline(proposalId); vm.roll(_deadline); } function _jumpToProposalEnd(uint256 proposalId, uint32 additionalBlocks) external { - uint256 _deadline = governor.proposalDeadline(proposalId); + uint256 _deadline = GOVERNOR.proposalDeadline(proposalId); vm.roll(_deadline + additionalBlocks); } } @@ -196,7 +196,7 @@ contract Constructor is L1VotePoolTest { new GovernorFlexibleVotingMock("Testington Dao", ERC20VotesComp(address(l1Erc20))); l1VotePool = new L1VotePoolHarness(L1_CHAIN.wormholeRelayer, address(gov)); - assertEq(address(l1VotePool.governor()), address(gov), "Governor is not set correctly"); + assertEq(address(l1VotePool.GOVERNOR()), address(gov), "Governor is not set correctly"); } } diff --git a/test/WormholeL2ERC20.t.sol b/test/WormholeL2ERC20.t.sol index 982eb732..b3725288 100644 --- a/test/WormholeL2ERC20.t.sol +++ b/test/WormholeL2ERC20.t.sol @@ -86,7 +86,7 @@ contract ReceiveWormholeMessages is L2ERC20Test { vm.prank(L2_CHAIN.wormholeRelayer); l2Erc20.receiveWormholeMessages( - abi.encode(account, l1Amount), + abi.encodePacked(account, l1Amount), new bytes[](0), MOCK_WORMHOLE_SERIALIZED_ADDRESS, L1_CHAIN.wormholeChainId, @@ -96,7 +96,7 @@ contract ReceiveWormholeMessages is L2ERC20Test { assertEq(l2Amount, l1Amount, "Amount after receive is incorrect"); } - function testFuzz_RevertIf_NotCalledByRelayer(address account, uint256 amount, address caller) + function testFuzz_RevertIf_NotCalledByRelayer(address account, uint224 amount, address caller) public { vm.assume(caller != L2_CHAIN.wormholeRelayer); @@ -149,7 +149,7 @@ contract CLOCK_MODE is L2ERC20Test { } contract L1Unlock is L2ERC20Test { - function testForkFuzz_CorrectlyWithdrawToken(address account, uint96 amount) public { + function testForkFuzz_CorrectlyWithdrawToken(address account, uint224 amount) public { vm.assume(account != address(0)); vm.selectFork(targetFork); @@ -161,7 +161,7 @@ contract L1Unlock is L2ERC20Test { vm.recordLogs(); vm.prank(L2_CHAIN.wormholeRelayer); l2Erc20.receiveWormholeMessages( - abi.encode(account, amount), + abi.encodePacked(account, amount), new bytes[](0), MOCK_WORMHOLE_SERIALIZED_ADDRESS, L1_CHAIN.wormholeChainId, diff --git a/test/WormholeL2VoteAggregator.t.sol b/test/WormholeL2VoteAggregator.t.sol index 283e7cf8..df7f3960 100644 --- a/test/WormholeL2VoteAggregator.t.sol +++ b/test/WormholeL2VoteAggregator.t.sol @@ -10,12 +10,9 @@ import {L1Block} from "src/L1Block.sol"; import {L2GovernorMetadata} from "src/L2GovernorMetadata.sol"; import {WormholeL2GovernorMetadata} from "src/WormholeL2GovernorMetadata.sol"; import {FakeERC20} from "src/FakeERC20.sol"; -import {WormholeL1VotePool} from "src/WormholeL1VotePool.sol"; import {L2VoteAggregator} from "src/L2VoteAggregator.sol"; import {WormholeL2VoteAggregator} from "src/WormholeL2VoteAggregator.sol"; import {L2GovernorMetadata} from "src/WormholeL2GovernorMetadata.sol"; -import {WormholeBase} from "src/WormholeBase.sol"; -import {WormholeReceiver} from "src/WormholeReceiver.sol"; import {TestConstants} from "test/Constants.sol"; import {GovernorMetadataMock} from "test/mock/GovernorMetadataMock.sol"; import {GovernorFlexibleVotingMock} from "test/mock/GovernorMock.sol"; diff --git a/test/WormholeReceiver.t.sol b/test/WormholeReceiver.t.sol index 61aca20b..774a18ee 100644 --- a/test/WormholeReceiver.t.sol +++ b/test/WormholeReceiver.t.sol @@ -10,7 +10,7 @@ import {TestConstants} from "test/Constants.sol"; contract WormholeReceiverTestHarness is WormholeReceiver { constructor(address _relayer, address _owner) WormholeBase(_relayer) WormholeReceiver(_owner) {} function receiveWormholeMessages( - bytes memory payload, + bytes calldata payload, bytes[] memory additionalVaas, bytes32 sourceAddress, uint16 sourceChain, diff --git a/test/harness/L1VotePoolHarness.sol b/test/harness/L1VotePoolHarness.sol index 16eb2981..96f9d354 100644 --- a/test/harness/L1VotePoolHarness.sol +++ b/test/harness/L1VotePoolHarness.sol @@ -16,7 +16,7 @@ contract L1VotePoolHarness is WormholeL1VotePool, WormholeReceiver, Test { {} function receiveWormholeMessages( - bytes memory payload, + bytes calldata payload, bytes[] memory additionalVaas, bytes32 sourceAddress, uint16 sourceChain, @@ -36,7 +36,7 @@ contract L1VotePoolHarness is WormholeL1VotePool, WormholeReceiver, Test { } function _createExampleProposal(address l1Erc20) internal returns (uint256) { - bytes memory proposalCalldata = abi.encode(FakeERC20.mint.selector, address(governor), 100_000); + bytes memory proposalCalldata = abi.encode(FakeERC20.mint.selector, address(GOVERNOR), 100_000); address[] memory targets = new address[](1); bytes[] memory calldatas = new bytes[](1); @@ -46,7 +46,7 @@ contract L1VotePoolHarness is WormholeL1VotePool, WormholeReceiver, Test { calldatas[0] = proposalCalldata; values[0] = 0; - return governor.propose(targets, values, calldatas, "Proposal: To inflate token"); + return GOVERNOR.propose(targets, values, calldatas, "Proposal: To inflate token"); } function createProposalVote(address l1Erc20) public returns (uint256) { @@ -71,7 +71,7 @@ contract L1VotePoolHarness is WormholeL1VotePool, WormholeReceiver, Test { } function _jumpToActiveProposal(uint256 proposalId) internal { - uint256 _deadline = governor.proposalDeadline(proposalId); + uint256 _deadline = GOVERNOR.proposalDeadline(proposalId); vm.roll(_deadline - 1); } }