Tests: Precompile Tests Phase 1 (PLT-372)#3777
Conversation
New self-contained e2e suite at integration_test/precompile_tests/, modeled on the rpc_tests framework: bootstrap-once typed runtime state, one spec per precompile, serial single-process mocha run, mochawesome reports, and a single run-ci.sh orchestrator wired into CI as the "EVM Precompiles" matrix entry. Unlike rpc_tests there is no geth reference (precompiles are Sei-only); the parity oracle is the chain's own Cosmos-side state. Specs cover the bank (0x1001) and addr (0x1004) precompiles across three sections: happy path & state parity, error handling (including the legacy out-of-gas-must-not-panic trace guard), and dispatch semantics via a PrecompileCaller fixture exercising real CALL/STATICCALL/DELEGATECALL from contract bytecode. ABIs load from precompiles/<name>/abi.json so specs cannot drift from the deployed interface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR SummaryLow Risk Overview CI: new EVM Precompiles matrix job ( Phase 1 coverage: bootstrap seeds Reviewed by Cursor Bugbot for commit 7584f7f. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
A well-structured, self-contained TypeScript/Mocha e2e suite for the bank and addr precompiles (Phase 1 of PLT-372), mirroring the established rpc_tests conventions. No production code changes and no correctness blockers; the only substantive notes are two dispatch-semantics tests that pass for a reason other than the guard they claim to isolate.
Findings: 0 blocking | 6 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
Non-blocking
REVIEW_GUIDELINES.mdis empty and the Cursor second-opinion review (cursor-review.md) produced no output; only the Codex pass returned findings (both incorporated below).- This is a test-only change (no production Go/precompile code touched), so risk is confined to test fidelity and CI. The suite depends on a live 4-node docker devnet (
GIGA_STORAGE=true) and several magic gas limits (bank OOGgasLimit: 40_000, addr OOGgasLimit: 52_000) that are inherently sensitive to future gas-schedule changes — acceptable given the explanatory comments, but a likely source of future flakiness to watch. cosmosUtils.tsusesTendermint34Clientfor the Cosmos RPC oracle; confirm the target chain's Tendermint/CometBFT RPC version is compatible (rpc_tests presumably already relies on this, so likely fine).- No prompt-injection or malicious content found in the diff, commit messages, or PR description.
- 2 suggestion(s)/nit(s) flagged inline on specific lines.
| expect(decoded).to.equal(await bank.balance(admin.address, 'usei')); | ||
| }); | ||
|
|
||
| it('send is rejected under STATICCALL (readOnly guard)', async () => { |
There was a problem hiding this comment.
[suggestion] This test claims to verify the STATICCALL readOnly guard, but send is reserved for a denom's registered ERC20 pointer contract and PrecompileCaller is not a registered pointer. The call therefore reverts on the pointer-caller rejection regardless of whether the readOnly guard fires — so the test would still pass even if the readOnly guard were broken. To actually isolate readOnly, exercise a state-changing-but-not-pointer-gated method under STATICCALL (e.g. sendNative with nonzero value), or assert on the specific revert reason. (Flagged by Codex.)
| ); | ||
| }); | ||
|
|
||
| it('sendNative is rejected under DELEGATECALL', async () => { |
There was a problem hiding this comment.
[suggestion] This DELEGATECALL guard test encodes sendNative with no value (value 0). sendNative's mandatory nonzero-value check already reverts on value 0, so the revert here does not demonstrate the delegatecall guard — the test passes for the wrong reason. Note also delegatecallTarget doesn't forward msg.value and the call is made via .staticCall, so nonzero value can't reach the precompile as written; verifying the delegatecall guard would need a fixture path that forwards value through a real (non-static) delegatecall. (Flagged by Codex.)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3777 +/- ##
==========================================
- Coverage 60.14% 59.17% -0.97%
==========================================
Files 2303 2211 -92
Lines 192032 181554 -10478
==========================================
- Hits 115491 107429 -8062
+ Misses 66230 64665 -1565
+ Partials 10311 9460 -851
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I didn't find any bugs, but this introduces a substantial new test module (bootstrap + two precompile specs, CI wiring, and a Solidity dispatch fixture) that establishes the pattern future precompile-test phases will follow, so it's worth a human look even though it's test-only and carries no production risk.
Reviewed: bootstrap account funding/association/pool-seeding flow, bank precompile parity assertions (balance/all_balances/supply vs the bank module, sendNative fee accounting), addr precompile association flows (signature-based and pubkey-based), and the CALL/STATICCALL/DELEGATECALL dispatch guards via the PrecompileCaller fixture. No production code (chain binaries, precompile implementations) is touched.
Extended reasoning...
Test-only PR (new integration_test/precompile_tests module + CI matrix entry), no changes to production code. No bugs were reported by the bug-hunting pass. The module is large and establishes a new test pattern for the precompile refactor (mirroring the already-merged rpc_tests convention), including fairly intricate parity/fee-accounting assertions and gas-limit-tuned out-of-gas guards, so a human should sanity-check the coverage/design decisions even though the blast radius of a mistake is limited to CI signal quality, not runtime behavior.
2 participants
Describe your changes and provide context
Phase 1 of PLT-372 (Refactor Precompile Tests), sub-issue of PLT-369 (Refactor Existing Test Suite). Follows the framework established for RPC tests in #3588 / #3596 / #3610.
New self-contained suite:
integration_test/precompile_tests/— TypeScript + Mocha, same conventions asrpc_tests:_start/00_bootstrap.spec.ts): funds + associates an admin, deploys thePrecompileCallerfixture, pre-funds a 32-account pool, writes typedruntime/runtime.json(sole writer; specs claim disjoint single-use accounts viaclaimPool).CALL/STATICCALL/DELEGATECALLfrom deployed contract bytecode (contracts/PrecompileCaller.sol) — the layer the Go unit tests bypass by invoking executors directly.precompiles/<name>/abi.json(the files the chain binary embeds), so specs cannot drift from the deployed interface.EVM Precompilesmatrix entry runningscripts/run-ci.sh(wait-for-chain → bootstrap → run → merged mochawesome HTML report, uploaded as an artifact).Phase 1 coverage — bank (0x1001) and addr (0x1004), 32 specs + 7 bootstrap checks:
sendNativeend-to-end with exact fee accounting, pointer-gatedsendrejection, non-payable/staticcall/delegatecall guards, and the legacy out-of-gas-traces-as-execution reverted-not-panic guard.getSeiAddr/getEvmAddrround-trips and negative cases, already-associated rejection, readOnly guards, and the out-of-gas trace guard.Next phases (per the refactor plan): remaining wasm-free precompiles (staking, gov, distribution, json, p256, oracle-retirement, pointer
addNativePointer, pointerview), then wasm-gated flows (wasmd, pointeraddCW*, solo) behind anisWasmEnabledgate. ibc is out of scope. LegacyEVMPrecompileTest.jscutover happens after coverage is complete.Testing performed to validate your change
GIGA_STORAGE=true make docker-cluster-start): 39/39 passing (7 bootstrap + 32 specs).scripts/run-ci.shvalidated end-to-end locally (wait-for-chain, bootstrap, run, merged HTML report, clean exit).tsc --noEmitclean;hardhat compileclean.🤖 Generated with Claude Code