-
Notifications
You must be signed in to change notification settings - Fork 7
feat: add storage interface with pruning methods #182
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
Conversation
a24510a to
e1801e2
Compare
691fff9 to
9dc8078
Compare
e1801e2 to
d728795
Compare
6089737 to
3e311c7
Compare
5ef8999 to
84c813c
Compare
d728795 to
b5d8c02
Compare
84c813c to
9ca3987
Compare
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
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.
Pull Request Overview
This PR introduces a storage interface with pruning and reorganization methods to support external proof generation. The interface defines traits for cursor-based operations and atomic storage transactions.
- Adds comprehensive storage traits for external proof data management
- Implements cursor-based iteration for trie nodes and hashed entries
- Provides atomic methods for pruning, reorganization, and bulk operations
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/exex/external-proofs/src/storage.rs | New storage interface with traits for cursors and external storage operations |
| crates/exex/external-proofs/src/lib.rs | Adds storage module declaration |
| crates/exex/external-proofs/Cargo.toml | Adds required dependencies for the storage interface |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| leaves_diff: HashedPostState, | ||
| ) -> ExternalStorageResult<()>; | ||
|
|
||
| /// Deletes all updates > `latest_common_block_number` and |
Copilot
AI
Oct 8, 2025
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.
Incomplete documentation comment. The comment ends with 'and' but doesn't specify what happens after deletion.
| /// Deletes all updates > `latest_common_block_number` and | |
| /// Deletes all updates greater than `latest_common_block_number` and replaces them by adding the provided `blocks_to_add`. |
| pub enum ExternalStorageError { | ||
| Other(eyre::Error), | ||
| } |
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.
Can we use thiserror for easier error definition?
| pub enum ExternalStorageError { | |
| Other(eyre::Error), | |
| } | |
| #[derive(Debug, Error)] | |
| pub enum ExternalStorageError { | |
| #[error("Other error: {0}")] | |
| Other(eyre::Error), | |
| } |
emhane
left a comment
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.
lgtm, smol comment. pending @dhyaniarun1993
|
|
||
| /// Error type for storage operations | ||
| pub enum ExternalStorageError { | ||
| Other(eyre::Error), |
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.
pls add a todo comment that this is just a placeholder
| @@ -0,0 +1,164 @@ | |||
| #![allow(dead_code, unreachable_pub)] | |||
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.
| #![allow(dead_code, unreachable_pub)] | |
| #![expect(dead_code, unreachable_pub)] |
use expect instead of allow
emhane
left a comment
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.
let's rename ExternalProofs to OpProofs or OpProofsWindow or OpFPPreimages everywhere for better higher level naming of what we're implementing. i think OpProofs is the most suitable for sake of being short.
| use std::fmt::Debug; | ||
|
|
||
| /// Error type for storage operations | ||
| pub enum ExternalStorageError { |
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.
| pub enum ExternalStorageError { | |
| pub enum OpProofsWindowStorageError { |
let's rename this, alt to OpProofsStorage
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.
I'll do this on a PR on top of this stack to reduce merge conflicts!
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.
|
OpProofs or OpFPWindow |
c5ab8a4 to
49d51e1
Compare
Fixes #177 I added a few methods that should allow atomically pruning and reorging blocks. Each function should represent a single transaction. We can store trie nodes, reorg, prune in a single transaction now. I kept the bulk insert methods available for storing the current state. --------- Co-authored-by: Arun Dhyani <[email protected]>
Fixes #177 I added a few methods that should allow atomically pruning and reorging blocks. Each function should represent a single transaction. We can store trie nodes, reorg, prune in a single transaction now. I kept the bulk insert methods available for storing the current state. --------- Co-authored-by: Arun Dhyani <[email protected]>
Fixes #177 I added a few methods that should allow atomically pruning and reorging blocks. Each function should represent a single transaction. We can store trie nodes, reorg, prune in a single transaction now. I kept the bulk insert methods available for storing the current state. --------- Co-authored-by: Arun Dhyani <[email protected]>
Fixes #177 I added a few methods that should allow atomically pruning and reorging blocks. Each function should represent a single transaction. We can store trie nodes, reorg, prune in a single transaction now. I kept the bulk insert methods available for storing the current state. --------- Co-authored-by: Arun Dhyani <[email protected]>
Fixes #177 I added a few methods that should allow atomically pruning and reorging blocks. Each function should represent a single transaction. We can store trie nodes, reorg, prune in a single transaction now. I kept the bulk insert methods available for storing the current state. --------- Co-authored-by: Arun Dhyani <[email protected]>
Fixes #177 I added a few methods that should allow atomically pruning and reorging blocks. Each function should represent a single transaction. We can store trie nodes, reorg, prune in a single transaction now. I kept the bulk insert methods available for storing the current state. --------- Co-authored-by: Arun Dhyani <[email protected]>
Fixes #177
I added a few methods that should allow atomically pruning and reorging blocks. Each function should represent a single transaction. We can store trie nodes, reorg, prune in a single transaction now. I kept the bulk insert methods available for storing the current state.