test(precompiles): add golden tests for factory V1 (BOP-424)#4015
test(precompiles): add golden tests for factory V1 (BOP-424)#4015stephancill wants to merge 1 commit into
Conversation
✅ Heimdall Review Status
|
7318c75 to
b67edca
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| #[test] | ||
| fn golden_create_reverts_malformed_params() { | ||
| let mut s = fresh(); | ||
| let (rev, _bytes) = call_factory( | ||
| &mut s, | ||
| CREATOR, | ||
| create_call( | ||
| IB20Factory::B20Variant::ASSET, | ||
| SALT, | ||
| Bytes::from(vec![0xaa_u8, 0xbb, 0xcc]), | ||
| vec![], | ||
| ), | ||
| ); | ||
| assert!(rev); | ||
| } |
There was a problem hiding this comment.
nit: For a golden test that pins exact behavior, discarding the revert bytes (_bytes) means the specific revert selector/payload isn't pinned. If the factory changes which error it returns for malformed params, this test would still pass. Same applies to golden_create_reverts_when_not_activated below. Consider asserting the exact revert bytes (or at minimum the error selector) to match the thoroughness of the other revert tests.
Pins Factory V1 behavior of the B-20 precompile: token creation flows (createB20 asset + stablecoin, with/without initCalls, zero-admin), deterministic address derivation (getB20Address), prefix/initialized reads (isB20, isB20Initialized), and every validation guard (TokenAlreadyExists, UnsupportedVersion, InvalidDecimals, MissingRequiredField, InvalidCurrency, InitCallFailed, typed-revert propagation, malformed params, not-activated, NonPayable). Each case asserts returned bytes/typed reverts, the created token's state + code, emitted events (B20Created + init events), and a per-case keccak storage-hash snapshot scoped to the factory + created token; plus per-op gas footprints and a compile-time op-coverage checklist. Authored and blessed against the shipped v1.1.1 (pre-versioned) factory, then confirmed identical pins pass on the versioned (resolver-gated) structure - proving the SOLID/versioned migration (BOP-418) is behavior-preserving. ~96% line coverage of b20_factory/logic/v1.rs (remainder are defensive error edges). Test-only: new integration test + one Cargo.toml [[test]] stanza; no source/logic changes. Co-authored-by: OpenCode <opencode-noreply@coinbase.com>
b67edca to
bd96a39
Compare
| StorageCtx::enter(&mut s, |ctx| { | ||
| B20FactoryStorage::new(ctx).dispatch(ctx, &calldata, BaseUpgrade::Beryl) | ||
| }) | ||
| .expect("gas-footprint op must succeed"); | ||
| (s.counter_sload(), s.counter_sstore(), s.counter_keccak256()) |
There was a problem hiding this comment.
The gas() helper unwraps the outer Result but doesn't assert that the dispatch actually succeeded (i.e. !output.is_revert()). If a future refactor introduces a bug that makes one of these calls revert, this test would silently measure the revert-path gas footprint rather than failing.
| StorageCtx::enter(&mut s, |ctx| { | |
| B20FactoryStorage::new(ctx).dispatch(ctx, &calldata, BaseUpgrade::Beryl) | |
| }) | |
| .expect("gas-footprint op must succeed"); | |
| (s.counter_sload(), s.counter_sstore(), s.counter_keccak256()) | |
| let out = StorageCtx::enter(&mut s, |ctx| { | |
| B20FactoryStorage::new(ctx).dispatch(ctx, &calldata, BaseUpgrade::Beryl) | |
| }) | |
| .expect("gas-footprint op must not fatally error"); | |
| assert!(!out.is_revert(), "gas-footprint op must not revert"); |
Review SummaryWell-structured golden test suite with thorough coverage of Factory V1 operations. The compile-time op-coverage checklist is a clever pattern for ensuring new ABI ops get test coverage. Two items noted: Findings
|
❌ base-std fork tests did not runThe build or setup step failed before any tests could execute. Check the workflow logs for details.
|
3 participants
What
Adds a golden/snapshot suite pinning Factory V1 behavior of the B-20 precompile (
crates/common/precompiles/tests/b20_factory_v1_golden.rs), driven through the realB20FactoryStoragedispatch entry. Covers all 4 ops and every validation guard:createB20— asset + stablecoin success (created token state/code +B20Createdevent), withinitCalls(asset & stablecoin), zero-admin (skips role grant), and guards:TokenAlreadyExists,UnsupportedVersion,InvalidDecimals,MissingRequiredField,InvalidCurrency,InitCallFailed, typed-revert propagation from an init call, malformed params, not-activated, andNonPayable.getB20Address— deterministic address derivation (asset + stablecoin).isB20/isB20Initialized— prefix + factory-initialized reads.Each case asserts exact returned bytes (or typed revert), the created token's resulting state, emitted events, and a per-case keccak storage-hash snapshot scoped to the factory + created token (excludes activation scaffolding); plus per-op storage-access gas footprints and a compile-time op-coverage checklist over
IB20FactoryCalls.Why
Baseline for the frozen-manifest check (BOP-422/BOP-424). The pins were authored and blessed against the shipped v1.1.1 (pre-versioned) factory implementation, then confirmed to pass unchanged on the versioned (resolver-gated) structure — proving the SOLID/versioned migration (BOP-418) is behavior-preserving.
Testing
~96% line coverage of
b20_factory/logic/v1.rs(remainder are defensive?error edges + the unused non-observercreate_b20alt-API). Test-only: new integration test + oneCargo.toml[[test]]stanza; no source/logic changes.