refactor(rpc): drop the unreachable pre-Jade witness gate - #178
Conversation
The gate rejected witness requests whose parent state predates Jade, but no request can reach that far: the retained window spans days while Jade activated months ago. It could only fire if an operator widened the window across the fork, and it would then report a hardfork error where the honest answer is that such a configuration was never supported. In exchange it cost an extra header lookup on the runtime, a `chain_spec` dependency on the RPC handler, and a rejection path that no end-to-end test can exercise, since the e2e genesis activates Jade at timestamp 0. The reasoning is kept as a module comment so the next reader does not have to re-derive why the check is absent. Also comment the two places where `morph-proofs` deliberately diverges from the vendored Base source (`proof.rs` conditioning the forced root node, `provider.rs` sorting canonical output). Those are the only divergences in a crate whose NOTICE.md pins it to Base `b2673bbd`, so an uncommented conflict during a future sync would very likely be resolved towards upstream, silently restoring a half-canonical witness: the mode parameter reachable, but only half of its semantics applied. Both comments say to keep the local side. `parent_block_id` gains the reasoning for addressing the parent by hash rather than by height, which is what makes an abandoned-branch request fail instead of silently replaying against a sibling branch's state.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Follow-up review cleanup on
d8ff161.Drop the pre-Jade gate
ensure_mpt_parentrejected requests whose parent state predates Jade. It cannot fire in practice: the retained proof window spans days, Jade activated months ago, so no in-window parent is ever pre-Jade. The only way to reach it is widening--proofs-history.windowacross the fork, and it would then answer with a hardfork error when the honest answer is that such a configuration was never supported.What it cost:
sealed_header_by_hashlookup on the runtime for every request, only to read a timestampchain_specfield and constructor argument on the RPC handlercrates/node/tests/assets/test-genesis.jsonsetsjadeForkTime: 0— only the pureensure_mpt_parent(bool)unit test covered itThe reasoning survives as a module comment, so the absence of the check reads as a decision rather than an omission.
Comment the two vendored divergences
crates/proofs/NOTICE.mdpins that crate to Baseb2673bbd, and these are the only two places where it deliberately departs from upstream:proof.rs— force-includes the root node for the legacy shape only (Base does it unconditionally)provider.rs— sorts canonical output (Base never sorts)Both were uncommented. A future sync from Base would put a conflict in front of whoever is resolving it with no indication that the local side is intentional, and resolving towards upstream would silently restore a half-canonical witness:
modereachable on the wire, but only part of its semantics applied — worse than either consistent state. Both comments now name the divergence, point at reth'sHistoricalStateProviderRef::witnessas the reference, and say to keep the local side.Style
parent_block_iddocuments why the parent is addressed by hash rather than byblock_number - 1:executionWitnessByBlockHashaccepts any block still in the database, so a height would resolve to the canonical block at that height and replay the requested block against a sibling branch's state, returning a successful but meaningless witness. A hash reaches the canonical check inMorphProofStateProviderFactoryand fails the request instead.The
morph_chainspecsubmodule-path import goes away with the gate, so the remaining imports are consistent with the rest of the crate.Testing
cargo nextest run -p morph-rpc -p morph-proofs— 251 passedcargo nextest run -p morph-node --features test-utils -E 'binary(it)'— 117 passedcargo clippy --all --all-targets -- -D warningsand themorph-node --features test-utilsvariant — cleancargo fmt --all -- --check— cleanNo behaviour change for any reachable request: the removed gate was unreachable, and the added comments are comments.