fix(precompiles): remove incomplete PrecompileProvider impl (Cantina #19 / BOP-606)

[rayyan224]

Summary

• Removes impl PrecompileProvider for BasePrecompiles from crates/common/precompiles/src/provider.rs. BasePrecompiles::new_with_spec builds an inner EthPrecompiles holding only the static per-hardfork set; the dynamic fork-installed entries (B20Factory, BerylLookup, PolicyRegistryPrecompile, ActivationRegistry at Beryl+, plus TxContext and NonceManager at Cobalt+) are only injected by install_with_observer. The former trait impl silently omitted them.
• Retargets the struct doc to name BasePrecompiles a builder that must be finalized via .install() / .install_with_observer() → PrecompilesMap (which already impl PrecompileProvider upstream in alloy_evm).
• Adds a Cobalt boundary regression test cobalt_install_exposes_every_fork_installed_precompile that asserts all six fork-installed addresses are reachable via .install().

First-party builders (api/builder.rs::precompiles_for_node, zk/utils/precompiles/mod.rs::installed_precompiles) already go through .install_with_observer(...), so there is no runtime behavioral change — this is a Cantina-flagged Informational integration hazard.

Cantina finding #19 (severity Informational, reporter slowfi) — link. Linear: BOP-606.

Base: stacked on top of #4748 (BOP-605); will auto-retarget to main once that merges. My change is independent — rebase onto main is trivial if reviewers prefer.

Test plan

• cargo check -p base-common-precompiles -p base-common-evm -p base-proof-zk-utils --all-targets — clean.
• cargo test -p base-common-precompiles — 662 tests pass, including the new cobalt_install_exposes_every_fork_installed_precompile.
• cargo test -p base-common-evm — 122 tests pass, including build_base_installs_dynamic_beryl_precompiles (end-to-end coverage that the builder still produces a complete provider).
• cargo clippy -p base-common-precompiles -p base-common-evm --all-targets -- -D warnings — clean.

🤖 Generated with Claude Code

[linear]

BOP-606

[depot-code-access]

✅ All benchmarks green — 14 within ±2% (deterministic instruction counts). View run

Benchmark details (14)

Benchmark	Base (target)	Head (this PR)	Δ instructions
batch_queue/drain/drain_cached_span_batches	242,027	242,027	+0.0%
batch_transaction/encode_in_place/encode_in_place	4,199,759	4,199,759	+0.0%
batch_transaction/temporary_frame_buffers/temporary_frame_buffers	8,408,350	8,408,350	+0.0%
flashblock_decode/decode/brotli	3,296,484	3,296,484	+0.0%
flashblock_decode/decode/plain_json	2,280,194	2,280,194	+0.0%
flz/compress_len/real_contract_call	43,148	43,148	+0.0%
flz/compress_len/synthetic_0	38,205	38,205	+0.0%
flz/compress_len/synthetic_1	54,682	54,682	+0.0%
flz/compress_len/synthetic_2	147,976	147,976	+0.0%
flz/data_gas	43,059	43,059	+0.0%
flz/tx_estimated_size	43,056	43,056	+0.0%
frame_parse/decode/single_4kib	1,031	1,031	+0.0%
frame_parse/parse_frames/few_large	1,053,062	1,053,062	+0.0%
frame_parse/parse_frames/many_small	154,763	154,763	+0.0%

[github-actions]

Review Summary

No issues found. This is a clean, well-scoped fix for Cantina #19 (BOP-606).

What changed

The incomplete impl PrecompileProvider<CTX> for BasePrecompiles<S> is removed. That impl delegated to self.inner (an EthPrecompiles), which only holds the static per-hardfork precompile set and silently omitted all six dynamic fork-installed entries (B20Factory, BerylLookup, PolicyRegistryPrecompile, ActivationRegistry at Beryl+, plus TxContext and NonceManager at Cobalt+).

Correctness verification

• All production callers already go through the install() / install_with_observer() path:
  • api/builder.rs::precompiles_for_node → .install_with_observer(BerylPrecompileMetricsObserver)
  • zk/utils/precompiles/mod.rs::installed_precompiles → .install_with_observer(...)
• The BaseEvm<DB, I, P = BasePrecompiles> default type parameter is always overridden at construction time — all builder methods return BaseEvm<..., PrecompilesMap>, never BaseEvm<..., BasePrecompiles>.
• No external crate in the workspace was using BasePrecompiles as a PrecompileProvider directly.

Block-production sensitivity

This PR touches precompile code, which is block-production-sensitive. However, the change removes a dead/incorrect code path rather than altering a live one. No block-production risk.

Test coverage

The new cobalt_install_exposes_every_fork_installed_precompile test asserts all six fork-installed addresses are present after .install() at the Cobalt boundary. BerylLookup is correctly excluded from direct address checks — it's a dynamic routing mechanism without a fixed address, and its effect is covered via the B20Variant::Asset.compute_address(...) token address check.

[depot-code-access]

Base Std historical fork tests

Fork	Result	Passed	Failed	Skipped	base/base	base-anvil	base-std
Beryl	pass	616	0	13	23d3eafb	cecddfa5	4658f1b7
Cobalt	pass	721	0	14	23d3eafb	fb00db40	e30b3421

View run

[github-actions]

Review Summary

PR: fix(precompiles): remove incomplete PrecompileProvider impl (Cantina #19 / BOP-606)

Block-production sensitivity: This PR touches precompile code, which is block-production-sensitive. However, the change is purely defensive — removing a trait impl that was never used in production paths. All first-party builders already go through .install() / .install_with_observer(), and no external code references BasePrecompiles as a PrecompileProvider. Confirmed via grep: zero callers depend on the removed impl.

Findings: None.

The removed impl PrecompileProvider for BasePrecompiles correctly identified the hazard: it delegated only to self.inner (EthPrecompiles), silently omitting the six dynamic fork-installed precompiles. The new regression test cobalt_install_exposes_every_fork_installed_precompile provides good coverage — it asserts all five fixed-address precompiles and exercises the BerylLookup dynamic lookup path via B20Variant::Asset.compute_address.

No correctness, safety, or performance issues found.

[robriks]

nice, the fix is correct and the reasoning holds up.

The deleted impl routed contains, run, and warm_addresses to self.inner (EthPrecompiles, holding only the static per-hardfork set) so a caller using BasePrecompiles as a provider would have had B20 token calls execute as calls to empty accounts rather than precompiles. Removing it is the right call.

I also checked for a second instance of the same shape and there isn't one: BaseZkvmPrecompiles (zk/utils/src/precompiles/mod.rs:172-207) looks similar but delegates to PrecompilesMap, so the dynamic lookup is included 👍

Claude pointed out a nit that can be scoped for a follow-up since it's not strictly in this PR's scope:

crates/common/evm/src/evm.rs:52 still declares:
pub struct BaseEvm<DB: RevmDatabase, I, P = BasePrecompiles> {

Every functional impl on BaseEvm requires P: PrecompileProvider<BaseContext<DB>, Output = InterpreterResult>. After this PR BasePrecompiles satisfies none of [EvmTr, ExecuteEvm, Evm, etc], so BaseEvm<DB, I> becomes an inert type: constructible, but no EVM trait applies.

That's a similar "looks usable, silently isn't" hazard like the one removed by this PR, relocated one layer up. It compiles today because nothing uses the bare form but is worth addressing in a follow-up. Retargeting to P = PrecompilesMap and fixing those two doc lines would close the finding at both layers.

[robriks]

I might be off the mark here but are we sure this test will fail as stated?

I had Claude run the test after reinstating the deleted impl verbatim and it passed which seems to imply that adding/adjusting impls in future changes won't cause this test to fail:

test provider::tests::cobalt_install_exposes_every_fork_installed_precompile ... ok
test result: ok. 662 passed; 0 failed
test result: ok. 39 passed; 0 failed

Claude tells me this is because the test never touches PrecompileProvider, only .install(), so a reintroduced impl is invisible to it