Skip to content

trie: use bitarray for path encoding#562

Open
weiihann wants to merge 10 commits intogballet:archival-state-expiryfrom
weiihann:archive-expiry/opt-path
Open

trie: use bitarray for path encoding#562
weiihann wants to merge 10 commits intogballet:archival-state-expiryfrom
weiihann:archive-expiry/opt-path

Conversation

@weiihann
Copy link
Copy Markdown
Collaborator

Fixes #559

Mostly ported the implementation from https://github.com/NethermindEth/juno/blob/main/core/trie/bitarray.go (I implemented this 1 year ago).

@weiihann weiihann requested a review from gballet as a code owner January 22, 2026 07:09
Copy link
Copy Markdown
Owner

@gballet gballet left a comment

Choose a reason for hiding this comment

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

There are a lot of methods we don't need, I recommend removing them so that the amount of code to review is smaller, and also if it's merged into geth, we don't get a ton of AI-generated PRs removing dead code one line at a time.

Comment thread trie/bintrie/empty.go Outdated
Comment thread trie/bintrie/bitarray.go
Comment thread trie/bintrie/bitarray.go Outdated
Comment thread trie/bintrie/bitarray.go Outdated
b.len = n

switch {
case n == 0:
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

hmmm, this means that you can not copy the full 256 bits of a bit array, because 0 means copying no bit, and n is in the 0..255 range

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

we don't care about it until we change the tree structure, but this is definitely an option for the researchers. So we don't have to fix it right now, but it might be necessary later.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'll leave it for now

Comment thread trie/bintrie/bitarray.go Outdated
Comment thread trie/bintrie/bitarray.go Outdated
Comment thread trie/bintrie/bitarray.go
@weiihann weiihann requested a review from gballet January 24, 2026 07:50
Copy link
Copy Markdown
Owner

@gballet gballet left a comment

Choose a reason for hiding this comment

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

Found some potential issues, ptal

Comment thread trie/bintrie/bitarray.go
Comment thread trie/bintrie/bitarray.go Outdated
Comment thread trie/bintrie/bitarray.go Outdated
Comment thread trie/bintrie/bitarray.go Outdated
Comment thread trie/bintrie/bitarray.go Outdated
Comment thread trie/bintrie/bitarray.go Outdated
Comment thread trie/bintrie/bitarray.go Outdated
@weiihann
Copy link
Copy Markdown
Collaborator Author

weiihann commented Jan 26, 2026

Most of the comments were applied on unused methods. Although they might be useful in the future, for the sake of time I've removed them for now. We can always add them back later.

@weiihann weiihann requested a review from gballet January 26, 2026 03:34
Comment thread trie/bintrie/bitarray.go Outdated
@weiihann weiihann requested a review from gballet January 27, 2026 03:48
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