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

fix(lib/trie): use MustBeHashed for V1 trie nodes with larger storage values #3739

Merged
merged 42 commits into from
Feb 28, 2024
Merged

fix(lib/trie): use MustBeHashed for V1 trie nodes with larger storage values #3739

merged 42 commits into from
Feb 28, 2024

Conversation

EclesioMeloJunior
Copy link
Member

@EclesioMeloJunior EclesioMeloJunior commented Feb 6, 2024 •
edited

Changes

  • Uses SetContextStorage function to update the trie being set with the runtime's state version
  • Any new node written in the trie that has a storage value greater than 32 bytes and the trie is V1 will contain the MustBeHashed property as true
  • While hashing the root node of the trie we check if this property is true and then we encode only the hash, nodes that were written while trie in V0 will be not affected

Tests

N/A

Issues

Primary Reviewer

@dimartiro

@EclesioMeloJunior EclesioMeloJunior added S-sync-westend related to particular network syncing. S-trie issues related to the pkg/trie package. labels Feb 6, 2024
@EclesioMeloJunior EclesioMeloJunior changed the title Trie hybrid state fix(lib/trie): use MustBeHashed for V1 trie nodes with larger storage values Feb 6, 2024
@EclesioMeloJunior EclesioMeloJunior force-pushed the trie-hybrid-state branch 2 times, most recently from ae493f2 to 715c8ae Compare February 7, 2024 19:08
@EclesioMeloJunior EclesioMeloJunior marked this pull request as ready for review February 8, 2024 15:41
dimartiro
dimartiro requested changes Feb 8, 2024
View reviewed changes
lib/trie/trie.go Outdated Show resolved Hide resolved
lib/runtime/storage/trie.go Outdated Show resolved Hide resolved
lib/runtime/storage/trie.go Outdated Show resolved Hide resolved
jimjbrettj
jimjbrettj reviewed Feb 26, 2024
View reviewed changes
dot/import.go Outdated Show resolved Hide resolved
jimjbrettj
jimjbrettj reviewed Feb 26, 2024
View reviewed changes
tests/sync/sync_test.go Outdated Show resolved Hide resolved
jimjbrettj
jimjbrettj approved these changes Feb 26, 2024
View reviewed changes
Copy link
Contributor

@jimjbrettj jimjbrettj left a comment

Choose a reason for hiding this comment

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

A few questions/comments, but overall really nice work!

dimartiro
dimartiro reviewed Feb 27, 2024
View reviewed changes
dot/core/service_test.go Outdated Show resolved Hide resolved
dimartiro
dimartiro approved these changes Feb 27, 2024
View reviewed changes
Copy link
Contributor

@dimartiro dimartiro left a comment •
edited

Choose a reason for hiding this comment

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

This is a really great work @EclesioMeloJunior congrats! 👏 and thanks you for the time you spent fixing it and testing it.
I really like the way you get rid of the max inline value parameter, that makes the code much more clear.

Good work!

@EclesioMeloJunior EclesioMeloJunior merged commit f5e48a9 into development Feb 28, 2024
22 of 23 checks passed
@EclesioMeloJunior EclesioMeloJunior deleted the trie-hybrid-state branch February 28, 2024 21:05
github-actions bot pushed a commit that referenced this pull request Mar 1, 2024
@semantic-release-bot
chore(release): 0.9.0 [skip ci]
1a13e4c 
# [0.9.0](v0.8.0...v0.9.0) (2024-3-1)

### Bug Fixes

* add a limit of number of bytes while scale decoding a slice ([#3733](#3733)) ([5edbf89](5edbf89))
* **docs:** Fixing link to polkadot runtime fundamentals to the right one ([#3763](#3763)) ([a785d32](a785d32))
* don't panic if we fail to convert hex to bytes ([#3734](#3734)) ([12234de](12234de))
* **dot/sync:** execute p2p handshake when there is no target ([#3695](#3695)) ([a9db0ec](a9db0ec))
* fix index out of range undeterministic error in rpc test ([#3718](#3718)) ([d099384](d099384))
* fix non deterministic  panic during TestStableNetworkRPC integration test ([#3756](#3756)) ([ee3d243](ee3d243))
* **lib/trie:** use `MustBeHashed` for V1 trie nodes with larger storage values ([#3739](#3739)) ([f5e48a9](f5e48a9))
* **mocks:** Set fixed version for uber mockgen in CI ([#3656](#3656)) ([ea9877e](ea9877e))
* **runtime/storage:** support nested storage transactions ([#3670](#3670)) ([3e99f6d](3e99f6d))
* segfault on node restart ([#3736](#3736)) ([d1ca7aa](d1ca7aa))
* **state-version:** should be uint8 instead of uint32 ([#3779](#3779)) ([c8fdb14](c8fdb14))
* update paseo chain spec ([#3770](#3770)) ([6a54f28](6a54f28))
* use last finalized block on startup ([#3737](#3737)) ([c262642](c262642))

### Features

* **config:** dynamically set version based on environment ([#3693](#3693)) ([5c534c9](5c534c9))
* **staging:** Expose RPC on Westend Staging Node ([#3687](#3687)) ([c374eaa](c374eaa))
* **tests/scripts:** create script to retrieve trie state via rpc ([#3714](#3714)) ([5ccea40](5ccea40))
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 1, 2024

🎉 This PR is included in version 0.9.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

timwu20 pushed a commit that referenced this pull request Apr 19, 2024
@EclesioMeloJunior @timwu20
fix(lib/trie): use MustBeHashed for V1 trie nodes with larger stora…
a61a5dc 
…ge values (#3739)
timwu20 pushed a commit that referenced this pull request Apr 19, 2024
@semantic-release-bot @timwu20
chore(release): 0.9.0 [skip ci]
7f7c0a7 
# [0.9.0](v0.8.0...v0.9.0) (2024-3-1)

### Bug Fixes

* add a limit of number of bytes while scale decoding a slice ([#3733](#3733)) ([5edbf89](5edbf89))
* **docs:** Fixing link to polkadot runtime fundamentals to the right one ([#3763](#3763)) ([a785d32](a785d32))
* don't panic if we fail to convert hex to bytes ([#3734](#3734)) ([12234de](12234de))
* **dot/sync:** execute p2p handshake when there is no target ([#3695](#3695)) ([a9db0ec](a9db0ec))
* fix index out of range undeterministic error in rpc test ([#3718](#3718)) ([d099384](d099384))
* fix non deterministic  panic during TestStableNetworkRPC integration test ([#3756](#3756)) ([ee3d243](ee3d243))
* **lib/trie:** use `MustBeHashed` for V1 trie nodes with larger storage values ([#3739](#3739)) ([f5e48a9](f5e48a9))
* **mocks:** Set fixed version for uber mockgen in CI ([#3656](#3656)) ([ea9877e](ea9877e))
* **runtime/storage:** support nested storage transactions ([#3670](#3670)) ([3e99f6d](3e99f6d))
* segfault on node restart ([#3736](#3736)) ([d1ca7aa](d1ca7aa))
* **state-version:** should be uint8 instead of uint32 ([#3779](#3779)) ([c8fdb14](c8fdb14))
* update paseo chain spec ([#3770](#3770)) ([6a54f28](6a54f28))
* use last finalized block on startup ([#3737](#3737)) ([c262642](c262642))

### Features

* **config:** dynamically set version based on environment ([#3693](#3693)) ([5c534c9](5c534c9))
* **staging:** Expose RPC on Westend Staging Node ([#3687](#3687)) ([c374eaa](c374eaa))
* **tests/scripts:** create script to retrieve trie state via rpc ([#3714](#3714)) ([5ccea40](5ccea40))
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

@jimjbrettj jimjbrettj jimjbrettj approved these changes

@edwardmack edwardmack Awaiting requested review from edwardmack

@timwu20 timwu20 Awaiting requested review from timwu20 timwu20 is a code owner

@kishansagathiya kishansagathiya Awaiting requested review from kishansagathiya kishansagathiya is a code owner

@axaysagathiya axaysagathiya Awaiting requested review from axaysagathiya

Assignees

@dimartiro dimartiro

@timwu20 timwu20

Labels
S-sync-westend related to particular network syncing. S-trie issues related to the pkg/trie package.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

westend: Gossamer fails with Storage root must match that calculated at block #14576856
4 participants
@EclesioMeloJunior @dimartiro @jimjbrettj @timwu20