tests: add solver delegate coverage#3
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive test suite for the Solver7702Delegate contract, including unit, integration, and fork tests, along with necessary mocks and gas snapshots. It also adds documentation to the fallback function regarding the expected calldata format. Feedback focuses on correcting the invalid Solidity pragma version 0.8.34 to a stable release and improving fuzz test coverage by allowing empty payloads instead of substituting them with 0x00.
There was a problem hiding this comment.
Pull request overview
Adds a comprehensive Foundry test suite around Solver7702Delegate’s EIP-7702 delegated fallback forwarding behavior, including unit, integration (mock GPv2), and fork-based regression tests.
Changes:
- Added unit/integration/fork tests plus supporting mocks and utilities for constructing settlement-like calldata and revert/return-data scenarios.
- Added gas snapshot JSONs for the new test suites.
- Updated tooling/config (test solhint overrides, coverage exclusions to ignore
lib/, Foundry remappings,.env.example, and OpenZeppelin submodule pin).
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/utils/SettlementUtils.sol | Utility library to build mock/real GPv2 settlement calldata for integration/fork tests. |
| test/utils/MathUtils.sol | Small test-only math helper library. |
| test/Solver7702Delegate.t.sol | Unit tests covering constructor behavior and fallback forwarding/revert bubbling. |
| test/Solver7702Delegate.integration.t.sol | Integration tests using mocked GPv2 settlement/authenticator. |
| test/Solver7702Delegate.fork.t.sol | Fork tests against real GPv2 settlement across networks plus historical calldata replay. |
| test/mocks/targets/RevertingTarget.sol | Target contract producing standard Solidity revert shapes (custom error/string/panic/empty). |
| test/mocks/targets/RawRevertTarget.sol | Target contract that reverts with arbitrary raw bytes. |
| test/mocks/targets/PayableFallbackTarget.sol | Payable fallback/receive target used to assert exact forwarding semantics. |
| test/mocks/targets/NonpayableTarget.sol | Nonpayable target used to assert value-forwarding failure behavior. |
| test/mocks/MockSettlement.sol | Simple payable settlement-like target for delegate integration tests. |
| test/mocks/MockGPv2Settlement.sol | Minimal GPv2 settlement mock that checks solver allowlist and executes interactions. |
| test/mocks/MockGPv2Authenticator.sol | Minimal solver allowlist mock used by integration tests. |
| test/dependencies/settlement/IGPv2Settlement.sol | Test-local GPv2 settlement interface/types used by mocks and calldata builders. |
| test/dependencies/settlement/IGPv2Authenticator.sol | Test-local GPv2 authenticator interface used by mocks. |
| test/BaseTest.t.sol | Shared test harness: approved callers, packed calldata helper, delegation attachment helper. |
| test/.solhint.json | Test-folder solhint overrides to accommodate patterns used in the new tests. |
| src/Solver7702Delegate.sol | Added calldata-format documentation to the fallback function. |
| snapshots/Solver7702DelegateTest.json | Gas snapshot baselines for unit tests. |
| snapshots/Solver7702DelegateIntegrationTest.json | Gas snapshot baselines for integration tests. |
| snapshots/Solver7702DelegateForkTest.json | Gas snapshot baselines for fork tests. |
| Justfile | Adjusted test/coverage commands (including excluding lib/ from coverage). |
| foundry.toml | Added OpenZeppelin remapping and formatting adjustment. |
| foundry.lock | Added OpenZeppelin submodule pin. |
| .gitmodules | Added OpenZeppelin contracts submodule. |
| .env.example | Added RPC URL environment variables for fork testing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| test: | ||
| {{FORGE}} test -vvv --show-progress --gas-snapshot-check true | ||
| test *args: | ||
| {{FORGE}} test -vvv --show-progress {{args}} | ||
|
|
||
| # Print coverage summary | ||
| coverage-summary: | ||
| {{FORGE}} coverage --no-match-coverage "^(test|script)/" --report summary | ||
| {{FORGE}} coverage --no-match-coverage "^(test|script|lib)/" --report summary | ||
|
|
||
| # Generate lcov coverage report | ||
| coverage-lcov: | ||
| {{FORGE}} coverage --no-match-coverage "^(test|script)/" --report lcov | ||
| {{FORGE}} coverage --no-match-coverage "^(test|script|lib)/" --report lcov |
There was a problem hiding this comment.
I've added these modifications to the contracts-template repo as well
Basically:
- don't check snapshots locally, only in the CI, otherwise you can't generate them locally with
just test libwas getting caught in coverage so I removed it
There was a problem hiding this comment.
Added for mocks and possibly other utilities for testing. Isn't used in src/
| env: | ||
| ETH_MAINNET_RPC_URL: ${{ secrets.ETH_MAINNET_RPC_URL }} | ||
| GNOSIS_RPC_URL: ${{ secrets.GNOSIS_RPC_URL }} | ||
| ARBITRUM_ONE_RPC_URL: ${{ secrets.ARBITRUM_ONE_RPC_URL }} | ||
| BASE_RPC_URL: ${{ secrets.BASE_RPC_URL }} | ||
| OPTIMISM_RPC_URL: ${{ secrets.OPTIMISM_RPC_URL }} | ||
| POLYGON_RPC_URL: ${{ secrets.POLYGON_RPC_URL }} | ||
| BNB_MAINNET_RPC_URL: ${{ secrets.BNB_MAINNET_RPC_URL }} | ||
| PLASMA_RPC_URL: ${{ secrets.PLASMA_RPC_URL }} | ||
| AVALANCHE_RPC_URL: ${{ secrets.AVALANCHE_RPC_URL }} | ||
| INK_RPC_URL: ${{ secrets.INK_RPC_URL }} | ||
| LINEA_RPC_URL: ${{ secrets.LINEA_RPC_URL }} |
There was a problem hiding this comment.
Supported sample tests for all networks just to be sure ERC-7702 is present on all of them and generally as a sanity check.
| - uses: ./.github/actions/setup | ||
| - name: Test | ||
| run: just test | ||
| run: just test --gas-snapshot-check true |
There was a problem hiding this comment.
See comment below on Justfile
Summary
This PR adds test coverage for
Solver7702Delegate.What changed
Test plan
Run: