-
Notifications
You must be signed in to change notification settings - Fork 28
refactor(storage): rename BlockSignatures to BlockProof #553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,8 @@ use rocksdb::{ | |
| use std::path::Path; | ||
| use std::sync::Arc; | ||
|
|
||
| const LEGACY_BLOCK_SIGNATURES_CF: &str = "block_signatures"; | ||
|
|
||
| /// Returns the column family name for a table. | ||
| /// | ||
| /// Delegates to [`Table::name`] so the CF name and the metrics label share a | ||
|
|
@@ -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); | ||
|
|
@@ -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(); | ||
|
|
@@ -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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a node upgrades with an existing Knowledge Base Used: Storage ( Prompt To Fix With AIThis 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)?; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
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