Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bin/ethlambda/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,7 @@ async fn fetch_initial_state(
// Initialize the store from state + anchor block body, then persist the
// signatures so we can serve the anchor on BlocksByRoot. `insert_signed_block`
// overlaps with what `get_forkchoice_store` already wrote, but it's
// idempotent and the only path that also stores `BlockSignatures`.
// idempotent and the only path that also stores `BlockProof`.
let anchor_root = signed_block.message.header().hash_tree_root();
let mut store = Store::get_forkchoice_store(backend, state, signed_block.message.clone())
.inspect_err(|err| error!(%err, "Failed to initialize store from anchor state and block"))
Expand Down
2 changes: 1 addition & 1 deletion crates/net/rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ mod tests {
use libssz::SszEncode;

// Genesis-anchored store: `init_store` writes the header + state but no
// `BlockSignatures` (proof) row. `get_signed_block` synthesizes an empty
// `BlockProof` (proof) row. `get_signed_block` synthesizes an empty
// proof so peers can still receive the genesis block on BlocksByRoot;
// the HTTP endpoint stays consistent and returns 200 rather than 404.
let state = create_test_state();
Expand Down
14 changes: 7 additions & 7 deletions crates/storage/src/api/tables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ pub enum Table {
BlockHeaders,
/// Block body storage: H256 -> BlockBody
BlockBodies,
/// Block signatures storage: (slot || root) -> BlockSignatures
/// Block proof storage: (slot || root) -> BlockProof
///
/// Stored separately from blocks because the genesis block has no signatures.
/// Stored separately from blocks because the genesis block has no proof.
/// Keyed by slot || root so pruning can scan in slot order and stop early.
/// Non-genesis blocks have an entry until finalized: signatures below the
/// finalized boundary are pruned (`prune_old_block_signatures`), while
/// Non-genesis blocks have an entry until finalized: proofs below the
/// finalized boundary are pruned (`prune_old_block_proofs`), while
/// headers and bodies are kept forever.
BlockSignatures,
BlockProof,
/// Canonical block index: slot -> block root
BlockRoots,
/// State storage: H256 -> State
Expand Down Expand Up @@ -40,7 +40,7 @@ pub enum Table {
pub const ALL_TABLES: [Table; 8] = [
Table::BlockHeaders,
Table::BlockBodies,
Table::BlockSignatures,
Table::BlockProof,
Table::BlockRoots,
Table::States,
Table::StateDiffs,
Expand All @@ -54,7 +54,7 @@ impl Table {
match self {
Table::BlockHeaders => "block_headers",
Table::BlockBodies => "block_bodies",
Table::BlockSignatures => "block_signatures",
Table::BlockProof => "block_proof",
Table::BlockRoots => "block_roots",
Table::States => "states",
Table::StateDiffs => "state_diffs",
Expand Down
17 changes: 16 additions & 1 deletion crates/storage/src/backend/rocksdb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ use rocksdb::{
use std::path::Path;
use std::sync::Arc;

const LEGACY_BLOCK_SIGNATURES_CF: &str = "block_signatures";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for backwards compatibility


/// Returns the column family name for a table.
///
/// Delegates to [`Table::name`] so the CF name and the metrics label share a
Expand All @@ -27,6 +29,7 @@ pub struct RocksDBBackend {
impl RocksDBBackend {
/// Open a RocksDB database at the given path.
pub fn open(path: impl AsRef<Path>) -> Result<Self, Error> {
let path = path.as_ref();
let mut opts = Options::default();
opts.create_if_missing(true);
opts.create_missing_column_families(true);
Expand All @@ -47,7 +50,7 @@ impl RocksDBBackend {
let mut block_opts = BlockBasedOptions::default();
block_opts.set_block_cache(&block_cache);

let cf_descriptors: Vec<_> = ALL_TABLES
let mut cf_descriptors: Vec<_> = ALL_TABLES
.iter()
.map(|t| {
let mut cf_opts = Options::default();
Expand All @@ -56,6 +59,18 @@ impl RocksDBBackend {
})
.collect();

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,
));
}
Comment on lines +62 to +72

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.


let db =
DBWithThreadMode::<MultiThreaded>::open_cf_descriptors(&opts, path, cf_descriptors)?;

Expand Down
Loading