Skip to content

chore(storage): validate pending blocks before storing them - #657

Open
Aliemeka wants to merge 7 commits into
lambdaclass:mainfrom
Aliemeka:fix/verify-blocks-before-storing-as-pending
Open

Aliemeka wants to merge 7 commits into
lambdaclass:mainfrom
Aliemeka:fix/verify-blocks-before-storing-as-pending

Conversation

@Aliemeka

@Aliemeka Aliemeka commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

🗒️ Description / Motivation

When a block arrives and its parent state is missing, the BlockChain actor writes it to the DB as pending (header, body, and proof, with no LiveChain entry) so the cascade can import it once the parent lands. The P2P actor serves anything in those tables over BlocksByRoot.

Before this PR, the only checks ahead of that write were that the slot is above finalized and not too far in the future. That had three consequences:

  1. Any peer could get an unverified block persisted and re-served to other peers by our node.
  2. discard_pending_subtree only cleared the in-memory maps, so discarded pending blocks stayed on disk and kept being served.
  3. A block that failed its state transition left its waiting children stuck forever, and each new child triggered a refetch and re-verification of the same invalid parent.

This PR validates a block before storing it as pending, deletes the rows of pending blocks that are discarded, and rejects children of blocks that failed the state transition.

What Changed

  • crates/blockchain/src/store.rs
    • Added validate_pending_block, which runs every check that needs no parent state, cheapest first:
      1. The parent root is not the zero root or the block's own root.
      2. No duplicate AttestationData, and at most MAX_ATTESTATIONS_DATA distinct entries.
      3. proposer_index is the slot's proposer.
      4. Every attester index is inside the validator registry.
      5. If the parent's header is stored (the parent is itself pending): the block's slot is after the parent's, and the parent is not on a fork that conflicts with the finalized block.
      6. Last, verify_block_signatures, so a forged block never reaches the SNARK verifier.
    • The registry is read from the head state. It is fixed at genesis, so every state gives the same answer, and the head state is almost always in the state cache.
    • Extracted validate_block_attestations and validate_validator_indices, now shared by on_block_core, verify_block_signatures, and the pending path.
    • New StoreError variants: InvalidParentRoot, ParentSlotNotBefore, ParentConflictsWithFinalized.
  • crates/blockchain/src/lib.rs
    • process_or_pend_block validates before insert_pending_block. A rejected block is logged and its waiting subtree is discarded.
    • New invalid_blocks: HashMap<H256, u64> field holding roots that failed the state transition. A block whose parent is in it is rejected before validation. Entries at or below the finalized slot are pruned after each block cascade.
    • New on_import_failure: on a StateTransitionFailed error it records the root and discards the block and its waiting children. Other failures are ignored.
    • discard_pending_subtree now deletes the stored rows of every block it discards.
  • crates/blockchain/src/spec_test_runner.rs
    • Maps the three new variants to no spec rejection reason, since the spec has no pending step.
  • crates/storage/src/store.rs
    • Added Store::delete_pending_block, which removes the BlockHeaders, BlockBodies, and BlockProof rows of a block that was never imported.
  • docs/architecture.md, docs/data_storage.md
    • Describe pending validation and the deletion of discarded pending rows.

Correctness / Behavior Guarantees

  • Import path unchanged. on_block_core runs the same checks in the same order; the extraction is behavior-preserving.
  • Deep gaps still fill. A child of a pending parent passes validation, so the ancestor walk and cascade work as before.
  • Imported blocks are never deleted. delete_pending_block returns early for any root with a state, and never touches LiveChain or BlockRoots. This matters because discard_pending_subtree also runs on blocks at or below the finalized slot, which can be canonical.
  • A bad copy cannot condemn a real block. The block root commits to the message but not the proof, so a real block with a garbage proof fails signature verification under the real root. Only state transition failures, which depend on the message alone, mark a root invalid or discard its children.
  • invalid_blocks stays small. An entry requires a block that passed signature verification, so only a misbehaving validator can add one, and entries are pruned at finality.
  • Known cost. A pending block's signature is verified twice: once before it is stored and again at import. Pending is a rare path, so this was preferred over tracking verified roots.

Out of Scope / Follow-ups

  • Pending block cap. Bounding pending blocks in total and per slot is left for a follow-up PR.
  • Stranded pending blocks. A pending block whose missing ancestor never arrives is not swept when finality passes it. prune_old_block_proofs removes its proof row after about a day, so it stops being served, but its header and body stay.
  • Considered and dropped:
    • Dropping BlocksByRange blocks with a missing parent. Range sync starts at our head slot plus one, so an honest peer on a fork that split below our head sends exactly that. Dropping them would stop the node reorging onto that fork.
    • Skipping the rewrite of a block that is already pending or stored. After a restart, rows from the previous run are re-registered without validation, so skipping could let an unverified copy win over a freshly verified one.

Tests Added / Run

  • crates/storage: 3 tests for delete_pending_block covering a pending block, an imported block, and an unknown root.
  • crates/blockchain/src/store.rs:
    • on_block_rejects_too_many_attestation_data, since the cap was only covered by the currently skipped spec fixtures.
    • 10 validate_pending_block_* tests: two acceptance cases, one rejection per check, and an invalid-proof rejection.
  • crates/blockchain/src/lib.rs: 9 actor tests:
    • Discarded subtrees delete their rows and keep imported roots.
    • An invalid orphan is neither stored nor pended, and removes children waiting on it.
    • Children of an invalid block are rejected.
    • A state transition failure condemns the root, and a signature failure does not.
    • Invalid roots are pruned at finality.
  • Limitations:
    • The acceptance tests skip signature verification through a private validate_pending_block_core(.., verify: false), mirroring on_block_without_verification. Actor tests that need a block already pending set that state up directly. A real proof needs the leanVM prover.
    • on_import_failure is tested directly, because reaching a real state transition failure needs a validly signed block. Its one-line call in the import failure arm is not covered.
  • Ran:
    • cargo test --profile release-fast -p ethlambda-blockchain -p ethlambda-storage --lib: all passing
    • cargo clippy --profile release-fast -p ethlambda-blockchain -p ethlambda-storage --all-targets -- -D warnings: clean
    • cargo check --profile release-fast --workspace --all-targets: clean

Related Issues / PRs

✅ Verification Checklist

  • Ran make fmt
  • Ran make lint (clippy with -D warnings)
  • Ran make test

@Aliemeka
Aliemeka marked this pull request as draft October 3, 2026 23:41
@Aliemeka
Aliemeka force-pushed the fix/verify-blocks-before-storing-as-pending branch from c7220cd to 18df6b2 Compare October 3, 2026 23:45
@Aliemeka
Aliemeka marked this pull request as ready for review October 3, 2026 23:46

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Verify blocks before storing them as pending

1 participant