fix(state): Change StorageState methods to always expect a block hash
#4792
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
haikoschol
changed the base branch from
development
to
haiko/read-only-storage-state
haikoschol
changed the title
StorageState methods to always expect a block hash
haikoschol
force-pushed
the
haiko/fix-StorageState
branch
from
f810b70 to
7fdcd0b
Compare
P1sar
approved these changes
Jun 30, 2025
haikoschol
force-pushed
the
haiko/fix-StorageState
branch
from
7fdcd0b to
c0524b4
Compare
Before, it expected a state root hash. This also fixes notifications for the state_subscribeStorage RPC call.
haikoschol
force-pushed
the
haiko/fix-StorageState
branch
from
c0524b4 to
f412899
Compare
haikoschol
force-pushed
the
haiko/fix-StorageState
branch
from
33d59e9 to
1b1c4d6
Compare
haikoschol
force-pushed
the
haiko/fix-StorageState
branch
from
3be6ad6 to
f6b3457
Compare
haikoschol
requested review from
EclesioMeloJunior,
jimjbrettj and
timwu20
as code owners
dimartiro
approved these changes
Jul 3, 2025
timwu20
reviewed
Jul 4, 2025
timwu20
reviewed
Jul 4, 2025
Yeah, I thought about that too. Shouldn't make too much of a difference since I'll probably merge |
haikoschol
changed the base branch from
haiko/read-only-storage-state
to
development
haikoschol
changed the base branch from
development
to
haiko/read-only-storage-state
Just tried this and actually it does make a difference because I based this branch off Let's just get #4769 done quickly after merging this to get these fixes onto |
timwu20
approved these changes
Jul 4, 2025
haikoschol
merged commit 3a71c45
into
haiko/read-only-storage-state
94 of 104 checks passed
haikoschol
added a commit
that referenced
this pull request
Jul 5, 2025
After #4792, StorageState methods that expected a state root hash now take a block hash. This updates all affected StorageState methods on ClientAdapter and simplifies their implementation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
This PR changes the semantics of some methods in the
StorageStateinterface and its implementations byInmemoryStorageStateandClientAdapter.Before this change, those methods expected a state root hash as parameter. This was probably done due to how the internal
Triesdata structure works, maintaining a map from state root hash toTrie. In any case, its inconvenient, unintuitive and in some cases just plain wrong. These methods are also sometimes called incorrectly with a block hash.In order to enable implementing
StorageStateonClientAdapterfor the ongoingdb.Backendrework, without requiring hacky workarounds, I propose to merge this PR into my feature branch for #4467.Tests
go test -tags integration github.com/ChainSafe/gossamerIssues
#4467