refactor(storage): rename BlockSignatures to BlockProof - #553
Conversation
Greptile SummaryThis PR consistently renames block-signature storage concepts to block proofs across the API, pruning helpers, documentation, and RocksDB schema.
Confidence Score: 4/5The legacy proof records must be migrated or read through a fallback before this PR is safe to merge. Existing databases open both column families, but all signed-block reads target only the newly created Files Needing Attention: crates/storage/src/backend/rocksdb.rs, crates/storage/src/api/tables.rs, crates/storage/src/store.rs
|
| Filename | Overview |
|---|---|
| crates/storage/src/backend/rocksdb.rs | Adds legacy-CF opening compatibility, but leaves all records in that family inaccessible after upgrading. |
| crates/storage/src/api/tables.rs | Renames the public table variant and physical column-family label, creating the need for a real data migration or fallback. |
| crates/storage/src/store.rs | Consistently adopts the new naming, but reads only the new table and therefore cannot reconstruct blocks backed by legacy proof records. |
| docs/data_storage.md | Updates the storage documentation and table count consistently with the renamed table. |
| docs/infographics/ethlambda_architecture.html | Updates the architecture table label without changing executable behavior. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Existing RocksDB] --> B[legacy block_signatures CF]
A --> C[RocksDBBackend::open]
C --> D[Open legacy CF descriptor]
C --> E[Create/open block_proof CF]
F[Store reads Table::BlockProof] --> E
B -. no migration or fallback .-> F
E --> G[Missing legacy proofs]
Prompt To Fix All With AI
### Issue 1
crates/storage/src/backend/rocksdb.rs:62-72
**Legacy proofs become inaccessible**
When a node upgrades with an existing `block_signatures` column family, this code opens that family but neither migrates its records nor makes reads fall back to it. All non-genesis proofs are instead read from the newly created, empty `block_proof` family, causing previously stored signed blocks to return `None`, disappear from block responses, or trigger callers that require the persisted block to panic.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "refactor(storage): rename BlockSignature..." | Re-trigger Greptile
| if path.exists() | ||
| && DBWithThreadMode::<MultiThreaded>::list_cf(&opts, path) | ||
| .is_ok_and(|cfs| cfs.iter().any(|cf| cf == LEGACY_BLOCK_SIGNATURES_CF)) | ||
| { | ||
| let mut cf_opts = Options::default(); | ||
| cf_opts.set_block_based_table_factory(&block_opts); | ||
| cf_descriptors.push(ColumnFamilyDescriptor::new( | ||
| LEGACY_BLOCK_SIGNATURES_CF, | ||
| cf_opts, | ||
| )); | ||
| } |
There was a problem hiding this comment.
Legacy proofs become inaccessible
When a node upgrades with an existing block_signatures column family, this code opens that family but neither migrates its records nor makes reads fall back to it. All non-genesis proofs are instead read from the newly created, empty block_proof family, causing previously stored signed blocks to return None, disappear from block responses, or trigger callers that require the persisted block to panic.
Knowledge Base Used: Storage (ethlambda_storage)
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/storage/src/backend/rocksdb.rs
Line: 62-72
Comment:
**Legacy proofs become inaccessible**
When a node upgrades with an existing `block_signatures` column family, this code opens that family but neither migrates its records nor makes reads fall back to it. All non-genesis proofs are instead read from the newly created, empty `block_proof` family, causing previously stored signed blocks to return `None`, disappear from block responses, or trigger callers that require the persisted block to panic.
**Knowledge Base Used:** [Storage (`ethlambda_storage`)](https://app.greptile.com/lambdaclass/-/custom-context/knowledge-base/lambdaclass/ethlambda/-/docs/storage.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| use std::path::Path; | ||
| use std::sync::Arc; | ||
|
|
||
| const LEGACY_BLOCK_SIGNATURES_CF: &str = "block_signatures"; |
There was a problem hiding this comment.
No need for backwards compatibility
MegaRedHand
left a comment
There was a problem hiding this comment.
Looks good. However, please drop the BlockSignatures -> BlockProof migration logic, since we don't want backwards compatibility during devnets
| `slot ‖ root → MultiMessageAggregate`. This table stores the block's **merged | ||
| aggregate proof blob**, not individual signatures. It is keyed by `slot ‖ root` | ||
| (not plain root) precisely so that pruning can scan in slot order and stop | ||
| early. |
There was a problem hiding this comment.
| `slot ‖ root → MultiMessageAggregate`. This table stores the block's **merged | |
| aggregate proof blob**, not individual signatures. It is keyed by `slot ‖ root` | |
| (not plain root) precisely so that pruning can scan in slot order and stop | |
| early. | |
| `slot ‖ root → MultiMessageAggregate`. This table stores the block's **merged | |
| aggregate proof blob**. It is keyed by `slot ‖ root` so that pruning can scan in | |
| slot order and stop early. |
🗒️ Description / Motivation
This PR renames the storage table previously called
BlockSignaturestoBlockProof.The table stores the merged block proof (
MultiMessageAggregate), not individual block signatures, so the old name was misleading. This makes the storage API, RocksDB column-family name, pruning helpers, tests, and docs match the data that is actually stored.What Changed
Updated
crates/storage/src/api/tables.rsTable::BlockSignaturestoTable::BlockProof.block_signaturestoblock_proof.Updated
crates/storage/src/store.rsTable::BlockSignaturesusages withTable::BlockProof.Updated
crates/storage/src/backend/rocksdb.rsblock_signaturescolumn family.Updated docs and comments
BlockProof.Correctness / Behavior Guarantees
slot || rootkey format.get_signed_blockstill synthesizes an empty proof only for slot 0.prune_old_block_proofs.Tests Added / Run
Related Issues / PRs
BlockSignaturestoBlockProof#527✅ Verification Checklist
make fmt— cleanmake lint(clippy with-D warnings) — cleanmake test(cargo test --workspace --profile release-fast) — all passing