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

Trie leaf iterate #1907

Closed
wants to merge 2 commits into from
Closed

Trie leaf iterate #1907

wants to merge 2 commits into from

Conversation

asdacap
Copy link
Contributor

@asdacap asdacap commented Jun 14, 2024

  • Add ability to iterate trie leaf.
  • Important for snap sync.
  • Make it easier to make range proof tests.
  • Uses a consumer func, as the stop condition can be complicated.
    • I guess you can convert it to iter.Seq2[KV, err] if you want.
  • Can be optimized, but unlikely to make much of a difference.

@asdacap asdacap requested a review from rianhughes June 14, 2024 08:50
Copy link

codecov bot commented Jun 14, 2024

Codecov Report

Attention: Patch coverage is 76.47059% with 8 lines in your changes missing coverage. Please review.

Project coverage is 75.39%. Comparing base (4e6dcd7) to head (b614c2e).
Report is 107 commits behind head on main.

Files Patch % Lines
core/trie/trie.go 71.42% 3 Missing and 3 partials ⚠️
core/trie/key.go 84.61% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1907      +/-   ##
==========================================
- Coverage   75.43%   75.39%   -0.05%     
==========================================
  Files          97       97              
  Lines        8342     8376      +34     
==========================================
+ Hits         6293     6315      +22     
- Misses       1519     1526       +7     
- Partials      530      535       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

core/trie/trie.go Outdated Show resolved Hide resolved
core/trie/trie.go Outdated Show resolved Hide resolved
@rianhughes rianhughes requested a review from pnowosie June 14, 2024 11:33
core/trie/trie.go Outdated Show resolved Hide resolved
@pnowosie pnowosie self-requested a review June 18, 2024 10:40
Copy link
Contributor

@pnowosie pnowosie left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me ;)

// No its not aligned, so need to convert to bigint then left shift it so that the MSB is of the same index
height := k.len
if other.len > height {
height = other.len
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be nice to write a test that meets this condition :)

@pnowosie
Copy link
Contributor

Closing. This is already part of #2035 and then we will continue from that one.

@pnowosie pnowosie closed this Aug 22, 2024
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.

4 participants