feat(client): Implement read-only StorageState functionality on ClientAdapter#4769
feat(client): Implement read-only StorageState functionality on ClientAdapter#4769haikoschol wants to merge 28 commits intodevelopmentfrom
StorageState functionality on ClientAdapter#4769Conversation
haikoschol
requested review from
EclesioMeloJunior,
jimjbrettj and
timwu20
as code owners
dimartiro
left a comment
dimartiro
left a comment
There was a problem hiding this comment.
LGTM but there are some things we can check or / and change before merging
| } | ||
|
|
||
| func (ca *ClientAdapter[H, Hasher, N, E, Header]) GenerateTrieProof(stateRoot common.Hash, keys [][]byte) ( | ||
| [][]byte, error) { |
There was a problem hiding this comment.
any reason why this is still unimplemented?
There was a problem hiding this comment.
Originally the answer to that question was because I overlooked it. Now the answer is because I don't know how. 😅 Looking at the old implementation hasn't helped so far.
I also overlooked StorageState.TrieState(), which I've implemented here.
There was a problem hiding this comment.
@dimartiro thanks for pointing me towards NewMerkleProof(). There's just one issue; it expects a hashdb.HashDB and all i have is a ClientAdapterDB and a statemachine.Backend.
haikoschol
force-pushed
the
haiko/read-only-storage-state
branch
from
7d9bbab to
37dd38d
Compare
| panic("unimplemented") | ||
| } | ||
|
|
||
| func (ca *ClientAdapter[H, Hasher, N, E, Header]) GetRuntime(blockHash common.Hash) (instance rt.Instance, err error) { |
There was a problem hiding this comment.
I think we should implement this in some point if we want to achieve a full read-only shim type. Do we have an issue for this one?
| ]] = &TestClient{} | ||
| _ api.StorageProvider[hash.H256, runtime.BlakeTwo256] = &TestClient{} | ||
| ) | ||
|
|
There was a problem hiding this comment.
I think you can add more tests cases here since you added the api.StorageProvider implementation for Client
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.
3 participants
Changes
This PR should cover all changes mentioned in the issue, except for changing
StorageState.Entries()to return astatemachine.PairsIter. As we discussed on Slack, this will be postponed since it is only used in RPC, which is currently not a priority.Regarding
LoadCode(); The implementation onClientAdapterexpects a block hash as the argument. The current implementation instate.InMemoryStorageStatecallsGetStorage(), which expects a state root hash. I'm pretty sure this is a long existing bug that hasn't been noticed becauseLoadCode()is called withnilon startup, which defaults to the best block.Tests
go test github.com/ChainSafe/gossamer/internal/client/adapterIssues
#4467