From 855dcf72883a38a2367089d39b885f1b34022bab Mon Sep 17 00:00:00 2001 From: Sam MacPherson Date: Sat, 30 Mar 2024 19:20:07 +0900 Subject: [PATCH] Add an overload which can set more settings (#13) * add an overload which can set more settings * set base fee instead of margin * fix CI; update forge-std * remove goerli as its deprecated * more overloads * remove hard coded arbitrum * remove goerli * temoprarily disable zkevm tests until its fixed * filter out domain testing dir from CI coverage * remove coverage testing * re-add coverage; fix exclusion of domain testing --- .github/workflows/ci.yml | 92 ++++++++++++++++++++++++++++++++++ .github/workflows/test.yml | 34 ------------- lib/forge-std | 2 +- src/XChainForwarders.sol | 27 ++++++---- test/ArbitrumIntegration.t.sol | 16 +++--- test/GnosisIntegration.t.sol | 6 --- test/IntegrationBase.t.sol | 2 - test/OptimismIntegration.t.sol | 8 --- test/ZkEVMIntegration.t.sol | 3 +- 9 files changed, 120 insertions(+), 70 deletions(-) create mode 100644 .github/workflows/ci.yml delete mode 100644 .github/workflows/test.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..283f602 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,92 @@ +name: CI + +on: + workflow_dispatch: + pull_request: + push: + branches: + - master + +env: + FOUNDRY_PROFILE: ci + +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + + - name: Install Foundry + uses: foundry-rs/foundry-toolchain@v1 + + - name: Build contracts + run: | + forge --version + forge build --sizes + + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + + - name: Install Foundry + uses: foundry-rs/foundry-toolchain@v1 + + - name: Run tests + env: + MAINNET_RPC_URL: ${{secrets.MAINNET_RPC_URL}} + OPTIMISM_RPC_URL: ${{secrets.OPTIMISM_RPC_URL}} + ARBITRUM_ONE_RPC_URL: ${{secrets.ARBITRUM_ONE_RPC_URL}} + ARBITRUM_NOVA_RPC_URL: ${{secrets.ARBITRUM_NOVA_RPC_URL}} + GNOSIS_CHAIN_RPC_URL: ${{secrets.GNOSIS_CHAIN_RPC_URL}} + BASE_RPC_URL: ${{secrets.BASE_RPC_URL}} + run: FOUNDRY_PROFILE=ci forge test + + coverage: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + + - name: Install Foundry + uses: foundry-rs/foundry-toolchain@v1 + + - name: Run coverage + env: + MAINNET_RPC_URL: ${{secrets.MAINNET_RPC_URL}} + OPTIMISM_RPC_URL: ${{secrets.OPTIMISM_RPC_URL}} + ARBITRUM_ONE_RPC_URL: ${{secrets.ARBITRUM_ONE_RPC_URL}} + ARBITRUM_NOVA_RPC_URL: ${{secrets.ARBITRUM_NOVA_RPC_URL}} + GNOSIS_CHAIN_RPC_URL: ${{secrets.GNOSIS_CHAIN_RPC_URL}} + BASE_RPC_URL: ${{secrets.BASE_RPC_URL}} + run: forge coverage --report summary --report lcov + + # To ignore coverage for certain directories modify the paths in this step as needed. The + # below default ignores coverage results for the test and script directories. Alternatively, + # to include coverage in all directories, comment out this step. Note that because this + # filtering applies to the lcov file, the summary table generated in the previous step will + # still include all files and directories. + # The `--rc lcov_branch_coverage=1` part keeps branch info in the filtered report, since lcov + # defaults to removing branch info. + - name: Filter directories + run: | + sudo apt update && sudo apt install -y lcov + lcov --remove lcov.info 'test/*' 'script/*' 'src/testing/*' --output-file lcov.info --rc lcov_branch_coverage=1 + + # This step posts a detailed coverage report as a comment and deletes previous comments on + # each push. The below step is used to fail coverage if the specified coverage threshold is + # not met. The below step can post a comment (when it's `github-token` is specified) but it's + # not as useful, and this action cannot fail CI based on a minimum coverage threshold, which + # is why we use both in this way. + - name: Post coverage report + if: github.event_name == 'pull_request' # This action fails when ran outside of a pull request. + uses: romeovs/lcov-reporter-action@v0.3.1 + with: + delete-old-comments: true + lcov-file: ./lcov.info + github-token: ${{ secrets.GITHUB_TOKEN }} # Adds a coverage summary comment to the PR. + + - name: Verify minimum coverage + uses: zgosalvez/github-actions-report-lcov@v2 + with: + coverage-files: ./lcov.info + minimum-coverage: 90 # Set coverage threshold. diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml deleted file mode 100644 index 09880b1..0000000 --- a/.github/workflows/test.yml +++ /dev/null @@ -1,34 +0,0 @@ -name: test - -on: workflow_dispatch - -env: - FOUNDRY_PROFILE: ci - -jobs: - check: - strategy: - fail-fast: true - - name: Foundry project - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v3 - with: - submodules: recursive - - - name: Install Foundry - uses: foundry-rs/foundry-toolchain@v1 - with: - version: nightly - - - name: Run Forge build - run: | - forge --version - forge build --sizes - id: build - - - name: Run Forge tests - run: | - forge test -vvv - id: test diff --git a/lib/forge-std b/lib/forge-std index dcb0d52..e4aef94 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit dcb0d52bc4399d37a6545848e3b8f9d03c77b98d +Subproject commit e4aef94c1768803a16fe19f7ce8b65defd027cfd diff --git a/src/XChainForwarders.sol b/src/XChainForwarders.sol index b60b0b3..d0c60b2 100644 --- a/src/XChainForwarders.sol +++ b/src/XChainForwarders.sol @@ -86,14 +86,11 @@ library XChainForwarders { address l1CrossDomain, address target, bytes memory message, - uint256 gasLimit + uint256 gasLimit, + uint256 maxFeePerGas, + uint256 baseFee ) internal { - // These constants are reasonable estimates based on current market conditions - // They can be updated as needed - uint256 maxFeePerGas = 1 gwei; - uint256 baseFeeMargin = 10 gwei; - - uint256 maxSubmission = ICrossDomainArbitrum(l1CrossDomain).calculateRetryableSubmissionFee(message.length, block.basefee + baseFeeMargin); + uint256 maxSubmission = ICrossDomainArbitrum(l1CrossDomain).calculateRetryableSubmissionFee(message.length, baseFee); uint256 maxRedemption = gasLimit * maxFeePerGas; ICrossDomainArbitrum(l1CrossDomain).createRetryableTicket{value: maxSubmission + maxRedemption}( target, @@ -110,26 +107,34 @@ library XChainForwarders { function sendMessageArbitrumOne( address target, bytes memory message, - uint256 gasLimit + uint256 gasLimit, + uint256 maxFeePerGas, + uint256 baseFee ) internal { sendMessageArbitrum( 0x4Dbd4fc535Ac27206064B68FfCf827b0A60BAB3f, target, message, - gasLimit + gasLimit, + maxFeePerGas, + baseFee ); } function sendMessageArbitrumNova( address target, bytes memory message, - uint256 gasLimit + uint256 gasLimit, + uint256 maxFeePerGas, + uint256 baseFee ) internal { sendMessageArbitrum( 0xc4448b71118c9071Bcb9734A0EAc55D18A153949, target, message, - gasLimit + gasLimit, + maxFeePerGas, + baseFee ); } diff --git a/test/ArbitrumIntegration.t.sol b/test/ArbitrumIntegration.t.sol index ce95b6c..441e879 100644 --- a/test/ArbitrumIntegration.t.sol +++ b/test/ArbitrumIntegration.t.sol @@ -23,10 +23,6 @@ contract ArbitrumIntegrationTest is IntegrationBaseTest { checkArbitrumStyle(new ArbitrumDomain(getChain("arbitrum_one"), mainnet)); } - function test_arbitrumOneGoerli() public { - checkArbitrumStyle(new ArbitrumDomain(getChain("arbitrum_one_goerli"), goerli)); - } - function test_arbitrumNova() public { checkArbitrumStyle(new ArbitrumDomain(getChain("arbitrum_nova"), mainnet)); } @@ -66,13 +62,17 @@ contract ArbitrumIntegrationTest is IntegrationBaseTest { address(arbitrum.INBOX()), address(moArbitrum), abi.encodeWithSelector(MessageOrdering.push.selector, 1), - 100000 + 100000, + 1 gwei, + block.basefee + 10 gwei ); XChainForwarders.sendMessageArbitrum( address(arbitrum.INBOX()), address(moArbitrum), abi.encodeWithSelector(MessageOrdering.push.selector, 2), - 100000 + 100000, + 1 gwei, + block.basefee + 10 gwei ); vm.stopPrank(); @@ -96,7 +96,9 @@ contract ArbitrumIntegrationTest is IntegrationBaseTest { address(arbitrum.INBOX()), address(moArbitrum), abi.encodeWithSelector(MessageOrdering.push.selector, 999), - 100000 + 100000, + 1 gwei, + block.basefee + 10 gwei ); vm.stopPrank(); diff --git a/test/GnosisIntegration.t.sol b/test/GnosisIntegration.t.sol index 8b2c13f..0828082 100644 --- a/test/GnosisIntegration.t.sol +++ b/test/GnosisIntegration.t.sol @@ -23,12 +23,6 @@ contract GnosisIntegrationTest is IntegrationBaseTest { checkGnosisStyle(new GnosisDomain(getChain('gnosis_chain'), mainnet), 0x75Df5AF045d91108662D8080fD1FEFAd6aA0bb59); } - function test_chiado() public { - setChain("chiado", ChainData("Chiado", 10200, "https://rpc.chiadochain.net")); - - checkGnosisStyle(new GnosisDomain(getChain('chiado'), goerli), 0x99Ca51a3534785ED619f46A79C7Ad65Fa8d85e7a); - } - function checkGnosisStyle(GnosisDomain gnosis, address _l2CrossDomain) public { Domain host = gnosis.hostDomain(); diff --git a/test/IntegrationBase.t.sol b/test/IntegrationBase.t.sol index b37ea19..b9df3e7 100644 --- a/test/IntegrationBase.t.sol +++ b/test/IntegrationBase.t.sol @@ -24,14 +24,12 @@ contract MessageOrdering { abstract contract IntegrationBaseTest is Test { Domain mainnet; - Domain goerli; address l1Authority = makeAddr("l1Authority"); address notL1Authority = makeAddr("notL1Authority"); function setUp() public { mainnet = new Domain(getChain("mainnet")); - goerli = new Domain(getChain("goerli")); } } diff --git a/test/OptimismIntegration.t.sol b/test/OptimismIntegration.t.sol index 56a7886..69e6793 100644 --- a/test/OptimismIntegration.t.sol +++ b/test/OptimismIntegration.t.sol @@ -25,18 +25,10 @@ contract OptimismIntegrationTest is IntegrationBaseTest { checkOptimismStyle(new OptimismDomain(getChain("optimism"), mainnet)); } - function test_optimismGoerli() public { - checkOptimismStyle(new OptimismDomain(getChain("optimism_goerli"), goerli)); - } - function test_base() public { checkOptimismStyle(new OptimismDomain(getChain("base"), mainnet)); } - function test_baseGoerli() public { - checkOptimismStyle(new OptimismDomain(getChain("base_goerli"), goerli)); - } - function checkOptimismStyle(OptimismDomain optimism) public { Domain host = optimism.hostDomain(); diff --git a/test/ZkEVMIntegration.t.sol b/test/ZkEVMIntegration.t.sol index bd1483e..85f968b 100644 --- a/test/ZkEVMIntegration.t.sol +++ b/test/ZkEVMIntegration.t.sol @@ -18,7 +18,8 @@ contract ZkevmMessageOrdering is MessageOrdering, IBridgeMessageReceiver { } -contract ZkEVMIntegrationTest is IntegrationBaseTest { +// FIXME: zkEVM bridging is broken, marking as abstract to temporarily disable until it's fixed +abstract contract ZkEVMIntegrationTest is IntegrationBaseTest { function test_zkevm() public { setChain("zkevm", ChainData("ZkEVM", 1101, "https://zkevm-rpc.com"));