Skip to content

Conversation

kmchicoine
Copy link
Contributor

@kmchicoine kmchicoine commented Jun 11, 2025

This PR contains

  • InMemoryNode and InMemoryChild types to build recursive in-memory tries
  • build_in_memory_trie and build_in_memory_trie_rec functions to build in-memory trie for (Nibbles, Option<TrieValue>) key-value pairs. The Option allows for both insertion/update and delete for a key at a given value.
  • set_subtrie and merge_in_memory_with_disk functions which allow merging an in-memory subtrie with an existing on-disk trie at the specified nibbles path.
  • tests for building the in-memory trie

Still in progress:

  • Hash checking: there is a HashVerificationMode enum which will be used to set the level of trust for hashes, either Ignore (always recompute), Check (optimistic hash checking, i.e. verify hashes and use if correct, otherwise recompute), or UnsafeTrust (blindly trust given hashes- currently using for quick tests but could be removed if deemed too risky)
  • Tests for set_subtrie and merge_in_memory_with_disk functions

More context in the Linear Ticket

@kmchicoine kmchicoine self-assigned this Jun 11, 2025
@cb-heimdall
Copy link
Collaborator

cb-heimdall commented Jun 11, 2025

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

});
} else {
// None value means deletion, skip
return None;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We'll probably need to track explicit deletions here, so that this can be applied when merging with the on-disk nodes.

new_children[i] = Some(ptr);
}
(Some(d_ptr), None) => {
// Only on-disk: keep
Copy link
Collaborator

Choose a reason for hiding this comment

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

As noted above, we'll need to be able to merge in deletions from the in-memory trie.


/// Converts a Node to an InMemoryNode for writing to disk. Necessary for merging an in-memory
/// trie with an on-disk trie
fn node_to_in_memory_node(&self, node: &Node) -> Result<InMemoryNode, Error> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can probably consume node to avoid cloning the contents.

Copy link
Collaborator

Choose a reason for hiding this comment

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

As this is only an in-memory conversion, this can also just implement https://doc.rust-lang.org/std/convert/trait.TryInto.html

fn in_memory_to_disk_node(
&mut self,
context: &mut TransactionContext,
node: &InMemoryNode,
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can probably also consume node to avoid cloning contents.

context: &mut TransactionContext,
node: &InMemoryNode,
) -> Result<Pointer, Error> {
let page = self.allocate_page(context)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We'll want to avoid allocating a fresh page for each node. Ideally this would have a designated page already picked out

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.

3 participants