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.

Sign up for GitHub

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

Jump to bottom

refactor(pkg/trie/triedb): Revise NewValueFromEncoded to only store a hash instead of prefixed key #4183

Merged
merged 1 commit into from
Sep 17, 2024
Merged

refactor(pkg/trie/triedb): Revise NewValueFromEncoded to only store a hash instead of prefixed key #4183

merged 1 commit into from
Sep 17, 2024

Conversation

timwu20
Copy link
Contributor

@timwu20 timwu20 commented Sep 16, 2024

Changes

  • Changes NewValueFromEncoded to store a hash instead of a prefixed key
  • Changes deathRow from map[common.Hash]any to map[string]any since the deathRow could contain prefixed keys to be removed in TrieDB.commit.
  • Changes TrieDB.replaceOldValue to populate death row with prefixed key.

Tests

go test -tags integration github.com/ChainSafe/gossamer

Issues

@timwu20 timwu20 marked this pull request as ready for review September 16, 2024 19:03
@timwu20 timwu20 changed the title refactor(pkg/trie/triedb): Revise NewValueFromEncoded to only store a hash instead of prefixed key refactor(pkg/trie/triedb): Revise NewValueFromEncoded to only store a hash instead of prefixed key Sep 16, 2024
dimartiro
dimartiro approved these changes Sep 16, 2024
View reviewed changes
Copy link
Contributor

@dimartiro dimartiro left a comment

Choose a reason for hiding this comment

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

Good catch! Thanks for the fixes and for the discussion around it

@timwu20 timwu20 merged commit f1e3ad3 into development Sep 17, 2024
24 checks passed
@timwu20 timwu20 deleted the tim/revise-triedb-valueref branch September 17, 2024 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dimartiro dimartiro dimartiro approved these changes

@EclesioMeloJunior EclesioMeloJunior EclesioMeloJunior approved these changes

@edwardmack edwardmack Awaiting requested review from edwardmack

@jimjbrettj jimjbrettj Awaiting requested review from jimjbrettj

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@timwu20 @dimartiro @EclesioMeloJunior