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

feat(trie): add sparse trie Display impl #14544

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Rjected
Copy link
Member

@Rjected Rjected commented Feb 17, 2025

Adds a display impl and tests it, uses indents by level when printing alternate form

@Rjected Rjected added C-enhancement New feature or request A-trie Related to Merkle Patricia Trie implementation labels Feb 17, 2025
@Rjected Rjected force-pushed the dan/add-display-impl-sparse-trie branch 2 times, most recently from 2f3046f to ccee058 Compare February 17, 2025 19:45
Comment on lines 209 to 213
/// Turns a [`Nibbles`] into a [`String`] by converting each nibble to its hex representation.
fn encode_nibbles(nibbles: &Nibbles) -> String {
let mut output = String::with_capacity(nibbles.len().div_ceil(2));
for nibble in nibbles.as_slice() {
let hex = format!("{nibble:01x}");
output.push_str(&hex);
}
output
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm won't this be confusing because 10 can be interpreted as both 0100 and a

maybe just do hex::encode(nibbles.pack())?

Copy link
Member Author

Choose a reason for hiding this comment

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

doing hex::encode(nibbles.pack()) incorrectly encodes paths with odd length (we get an extra 0 at the end). I'm also expecting paths to be interpreted as hex, so I'm not sure why 10 should be interpreted as a?

Copy link
Member

Choose a reason for hiding this comment

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

just pop the char at the end if len % 2 != 0, idk what you mean with the second sentence

@Rjected Rjected force-pushed the dan/add-display-impl-sparse-trie branch 2 times, most recently from fcc5538 to f1b5b28 Compare February 17, 2025 20:03
let mut output = String::with_capacity(nibbles.len().div_ceil(2));
for nibble in nibbles.as_slice() {
let hex = format!("{nibble:01x}");
output.push_str(&hex);
Copy link
Member

@DaniPopes DaniPopes Feb 17, 2025

Choose a reason for hiding this comment

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

push(char::from_digit(*nibble as u32, 16).unwrap())

Comment on lines 209 to 213
/// Turns a [`Nibbles`] into a [`String`] by converting each nibble to its hex representation.
fn encode_nibbles(nibbles: &Nibbles) -> String {
let mut output = String::with_capacity(nibbles.len().div_ceil(2));
for nibble in nibbles.as_slice() {
let hex = format!("{nibble:01x}");
output.push_str(&hex);
}
output
}
Copy link
Member

Choose a reason for hiding this comment

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

just pop the char at the end if len % 2 != 0, idk what you mean with the second sentence

@Rjected Rjected force-pushed the dan/add-display-impl-sparse-trie branch from 757b772 to be68954 Compare February 18, 2025 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-trie Related to Merkle Patricia Trie implementation C-enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants