Skip to content
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

Introduce RocksDB-backed Merkle Tree #6

Merged
merged 2 commits into from
Oct 2, 2023
Merged

Introduce RocksDB-backed Merkle Tree #6

merged 2 commits into from
Oct 2, 2023

Conversation

zeapoz
Copy link
Member

@zeapoz zeapoz commented Sep 29, 2023

  • Introduces a RocksDB-backed Merkle Tree:
    • Populates the genesis state from a CSV-file on creation.
    • Support for inserting a given CommitBlockInfoV1 and returning the new L2 block number.
  • Fixes some minor clippy complaints.

For now the database is purged before each run since we need to figure out a way to also persist the index to key mappings.

@zeapoz zeapoz requested a review from tuommaki September 29, 2023 12:17
@zeapoz zeapoz self-assigned this Sep 29, 2023
Cargo.toml Show resolved Hide resolved
src/tree.rs Show resolved Hide resolved
let mut tmp = [0u8; 32];
value.to_big_endian(&mut tmp);

let key = U256::from_little_endian(&derived_key);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do these endianness conversions work correctly on all platforms? i.e. is derived_key little-endian even on big-endian platform?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would assume so, though I can't guarantee. Do you have an idea on how to assert this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe checking the LogQuery implementation is one way? I'm not sure if this is a big concern right now, since there are very little big endian platforms alive anymore, and probably none of them will run this code, but I got curious since there's both endianness conversion / assumption in our code base. 🙂

pub struct TreeWrapper<'a> {
pub tree: MerkleTree<'a, RocksDBWrapper>,
// FIXME: How to save this for persistant storage?
pub index_to_key: Vec<U256>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we just write it ~as is? i.e. <num values: 32bit unsigned>,<value0>, <value1>, <value2>... - all numbers big endian encoded?

Copy link
Member Author

@zeapoz zeapoz Oct 2, 2023

Choose a reason for hiding this comment

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

I'm not fully sure of what you want, would it still be a Vec, containing the length as the first element and the rest of the values (keys) in their non-parsed variant?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, sorry I expressed myself a bit poorly. I basically responded to your FIXME comment question from above. No need to change anything here, but my thinking was that one possibility for persistent storage solution here would be to just write the values out to file, prefixing them by number of elements.

No need to do anything here 🙂

Copy link
Collaborator

@tuommaki tuommaki left a comment

Choose a reason for hiding this comment

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

All good 👍

pub struct TreeWrapper<'a> {
pub tree: MerkleTree<'a, RocksDBWrapper>,
// FIXME: How to save this for persistant storage?
pub index_to_key: Vec<U256>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, sorry I expressed myself a bit poorly. I basically responded to your FIXME comment question from above. No need to change anything here, but my thinking was that one possibility for persistent storage solution here would be to just write the values out to file, prefixing them by number of elements.

No need to do anything here 🙂

src/tree.rs Show resolved Hide resolved
let mut tmp = [0u8; 32];
value.to_big_endian(&mut tmp);

let key = U256::from_little_endian(&derived_key);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe checking the LogQuery implementation is one way? I'm not sure if this is a big concern right now, since there are very little big endian platforms alive anymore, and probably none of them will run this code, but I got curious since there's both endianness conversion / assumption in our code base. 🙂

@zeapoz zeapoz merged commit 9952538 into main Oct 2, 2023
4 checks passed
@zeapoz zeapoz deleted the feat/merkle-tree branch October 2, 2023 10:08
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.

2 participants