From b6a687d35c12fbf41c163fee98dc9bde15d1e054 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ecl=C3=A9sio=20Junior?= Date: Fri, 23 Aug 2024 13:42:18 -0400 Subject: [PATCH] fix(runtime/storage): Wrap the block execution under a transaction (#4138) --- dot/core/service.go | 2 +- dot/rpc/modules/childstate_integration_test.go | 2 +- dot/rpc/modules/childstate_test.go | 2 +- dot/rpc/modules/state_integration_test.go | 2 +- dot/rpc/modules/system_integration_test.go | 2 +- dot/state/inmemory_storage.go | 2 +- dot/state/inmemory_storage_test.go | 10 +++++----- dot/state/service_integration_test.go | 2 +- dot/sync/chain_sync.go | 2 +- dot/sync/chain_sync_test.go | 10 +++++----- dot/sync/syncer_integration_test.go | 2 +- lib/runtime/storage/trie.go | 18 +++++++----------- lib/runtime/storage/trie_test.go | 4 ++-- lib/runtime/wazero/imports_test.go | 2 ++ lib/runtime/wazero/instance.go | 5 +++++ lib/runtime/wazero/instance_test.go | 3 +-- 16 files changed, 36 insertions(+), 34 deletions(-) diff --git a/dot/core/service.go b/dot/core/service.go index 73ef05bcce..740f186e8f 100644 --- a/dot/core/service.go +++ b/dot/core/service.go @@ -280,7 +280,7 @@ func (s *Service) handleBlock(block *types.Block, state *rtstorage.TrieState) er } logger.Debugf("imported block %s and stored state trie with root %s", - block.Header.Hash(), state.MustRoot()) + block.Header.Hash(), state.Trie().MustHash()) parentRuntimeInstance, err := s.blockState.GetRuntime(block.Header.ParentHash) if err != nil { diff --git a/dot/rpc/modules/childstate_integration_test.go b/dot/rpc/modules/childstate_integration_test.go index cde3613bc2..6e1e559605 100644 --- a/dot/rpc/modules/childstate_integration_test.go +++ b/dot/rpc/modules/childstate_integration_test.go @@ -257,7 +257,7 @@ func setupChildStateStorage(t *testing.T) (*ChildStateModule, common.Hash) { err = tr.SetChildStorage(childStorageKey, []byte(":another_child"), []byte("value")) require.NoError(t, err) - stateRoot, err := tr.Root() + stateRoot, err := tr.Trie().Hash() require.NoError(t, err) bb, err := st.Block.BestBlock() diff --git a/dot/rpc/modules/childstate_test.go b/dot/rpc/modules/childstate_test.go index 376ed53739..ede45c7adb 100644 --- a/dot/rpc/modules/childstate_test.go +++ b/dot/rpc/modules/childstate_test.go @@ -33,7 +33,7 @@ func createTestTrieState(t *testing.T) (trie.Trie, common.Hash) { err = tr.SetChildStorage([]byte(":child_storage_key"), []byte(":another_child"), []byte("value")) require.NoError(t, err) - stateRoot, err := tr.Root() + stateRoot, err := tr.Trie().Hash() require.NoError(t, err) return tr.Trie(), stateRoot diff --git a/dot/rpc/modules/state_integration_test.go b/dot/rpc/modules/state_integration_test.go index 2e96210b21..482bdbf060 100644 --- a/dot/rpc/modules/state_integration_test.go +++ b/dot/rpc/modules/state_integration_test.go @@ -574,7 +574,7 @@ func setupStateModule(t *testing.T) (*StateModule, *common.Hash, *common.Hash) { err = ts.SetChildStorage([]byte(`:child1`), []byte(`:key1`), []byte(`:childValue1`)) require.NoError(t, err) - sr1, err := ts.Root() + sr1, err := ts.Trie().Hash() require.NoError(t, err) err = chain.Storage.StoreTrie(ts, nil) require.NoError(t, err) diff --git a/dot/rpc/modules/system_integration_test.go b/dot/rpc/modules/system_integration_test.go index ea3fce3e97..275a741892 100644 --- a/dot/rpc/modules/system_integration_test.go +++ b/dot/rpc/modules/system_integration_test.go @@ -330,7 +330,7 @@ func setupSystemModule(t *testing.T) *SystemModule { Header: types.Header{ Number: 3, ParentHash: chain.Block.BestBlockHash(), - StateRoot: ts.MustRoot(), + StateRoot: ts.Trie().MustHash(), Digest: digest, }, Body: types.Body{}, diff --git a/dot/state/inmemory_storage.go b/dot/state/inmemory_storage.go index 52d7273c08..592852836d 100644 --- a/dot/state/inmemory_storage.go +++ b/dot/state/inmemory_storage.go @@ -60,7 +60,7 @@ func NewStorageState(db database.Database, blockState *BlockState, // StoreTrie stores the given trie in the StorageState and writes it to the database func (s *InmemoryStorageState) StoreTrie(ts *storage.TrieState, header *types.Header) error { - root := ts.MustRoot() + root := ts.Trie().MustHash() s.tries.softSet(root, ts.Trie()) if header != nil { diff --git a/dot/state/inmemory_storage_test.go b/dot/state/inmemory_storage_test.go index 2d98547614..7949cdaffe 100644 --- a/dot/state/inmemory_storage_test.go +++ b/dot/state/inmemory_storage_test.go @@ -34,7 +34,7 @@ func TestStorage_StoreAndLoadTrie(t *testing.T) { ts, err := storage.TrieState(&trie.EmptyHash) require.NoError(t, err) - root, err := ts.Root() + root, err := ts.Trie().Hash() require.NoError(t, err) err = storage.StoreTrie(ts, nil) require.NoError(t, err) @@ -57,7 +57,7 @@ func TestStorage_GetStorageByBlockHash(t *testing.T) { value := []byte("testvalue") ts.Put(key, value) - root, err := ts.Root() + root, err := ts.Trie().Hash() require.NoError(t, err) err = storage.StoreTrie(ts, nil) require.NoError(t, err) @@ -89,7 +89,7 @@ func TestStorage_TrieState(t *testing.T) { require.NoError(t, err) ts.Put([]byte("noot"), []byte("washere")) - root, err := ts.Root() + root, err := ts.Trie().Hash() require.NoError(t, err) err = storage.StoreTrie(ts, nil) require.NoError(t, err) @@ -123,7 +123,7 @@ func TestStorage_LoadFromDB(t *testing.T) { ts.Put(kv.key, kv.value) } - root, err := ts.Root() + root, err := ts.Trie().Hash() require.NoError(t, err) // Write trie to disk. @@ -196,7 +196,7 @@ func TestGetStorageChildAndGetStorageFromChild(t *testing.T) { trieState := runtime.NewTrieState(genTrie) - header := types.NewHeader(blockState.GenesisHash(), trieState.MustRoot(), + header := types.NewHeader(blockState.GenesisHash(), trieState.Trie().MustHash(), common.Hash{}, 1, types.NewDigest()) err = storage.StoreTrie(trieState, header) diff --git a/dot/state/service_integration_test.go b/dot/state/service_integration_test.go index 15c463aa2b..f3246ed23c 100644 --- a/dot/state/service_integration_test.go +++ b/dot/state/service_integration_test.go @@ -478,7 +478,7 @@ func generateBlockWithRandomTrie(t *testing.T, serv *Service, err = trieState.Put(key, value) require.NoError(t, err) - trieStateRoot, err := trieState.Root() + trieStateRoot, err := trieState.Trie().Hash() require.NoError(t, err) if parent == nil { diff --git a/dot/sync/chain_sync.go b/dot/sync/chain_sync.go index 49b057d6b8..333f545a44 100644 --- a/dot/sync/chain_sync.go +++ b/dot/sync/chain_sync.go @@ -932,7 +932,7 @@ func (cs *chainSync) handleBlock(block *types.Block, announceImportedBlock bool) return err } - root := ts.MustRoot() + root := ts.Trie().MustHash() if !bytes.Equal(parent.StateRoot[:], root[:]) { panic("parent state root does not match snapshot state root") } diff --git a/dot/sync/chain_sync_test.go b/dot/sync/chain_sync_test.go index 0a74eb09eb..628e44d38d 100644 --- a/dot/sync/chain_sync_test.go +++ b/dot/sync/chain_sync_test.go @@ -66,10 +66,10 @@ func Test_chainSync_onBlockAnnounce(t *testing.T) { errTest := errors.New("test error") emptyTrieState := storage.NewTrieState(inmemory_trie.NewEmptyTrie()) - block1AnnounceHeader := types.NewHeader(common.Hash{}, emptyTrieState.MustRoot(), + block1AnnounceHeader := types.NewHeader(common.Hash{}, emptyTrieState.Trie().MustHash(), common.Hash{}, 1, nil) block2AnnounceHeader := types.NewHeader(block1AnnounceHeader.Hash(), - emptyTrieState.MustRoot(), + emptyTrieState.Trie().MustHash(), common.Hash{}, 2, nil) testCases := map[string]struct { @@ -252,10 +252,10 @@ func Test_chainSync_onBlockAnnounceHandshake_tipModeNeedToCatchup(t *testing.T) const somePeer = peer.ID("abc") emptyTrieState := storage.NewTrieState(inmemory_trie.NewEmptyTrie()) - block1AnnounceHeader := types.NewHeader(common.Hash{}, emptyTrieState.MustRoot(), + block1AnnounceHeader := types.NewHeader(common.Hash{}, emptyTrieState.Trie().MustHash(), common.Hash{}, 1, nil) block2AnnounceHeader := types.NewHeader(block1AnnounceHeader.Hash(), - emptyTrieState.MustRoot(), + emptyTrieState.Trie().MustHash(), common.Hash{}, 130, nil) blockStateMock := NewMockBlockState(ctrl) @@ -1382,7 +1382,7 @@ func createSuccesfullBlockResponse(t *testing.T, parentHeader common.Hash, response.BlockData = make([]*types.BlockData, numBlocks) emptyTrieState := storage.NewTrieState(inmemory_trie.NewEmptyTrie()) - tsRoot := emptyTrieState.MustRoot() + tsRoot := emptyTrieState.Trie().MustHash() firstHeader := types.NewHeader(parentHeader, tsRoot, common.Hash{}, uint(startingAt), nil) diff --git a/dot/sync/syncer_integration_test.go b/dot/sync/syncer_integration_test.go index c10643da5e..e578ae2b12 100644 --- a/dot/sync/syncer_integration_test.go +++ b/dot/sync/syncer_integration_test.go @@ -101,7 +101,7 @@ func newTestSyncer(t *testing.T) *Service { stateSrvc.Block.StoreRuntime(block.Header.Hash(), instance) logger.Debugf("imported block %s and stored state trie with root %s", - block.Header.Hash(), ts.MustRoot()) + block.Header.Hash(), ts.Trie().MustHash()) return nil }).AnyTimes() cfg.BlockImportHandler = blockImportHandler diff --git a/lib/runtime/storage/trie.go b/lib/runtime/storage/trie.go index 074c6dba7d..c8a1eb5828 100644 --- a/lib/runtime/storage/trie.go +++ b/lib/runtime/storage/trie.go @@ -136,18 +136,14 @@ func (t *TrieState) Get(key []byte) []byte { return t.state.Get(key) } -// MustRoot returns the trie's root hash. It panics if it fails to compute the root. -func (t *TrieState) MustRoot() common.Hash { - hash, err := t.Root() - if err != nil { - panic(err) - } - - return hash -} - -// Root returns the trie's root hash +// Root is executed in the block finalisation +// when it is wrapping everything and needs to ensure +// the root hash matches the expected one, in this case +// we commit the changeset we started in the beginning +// WARN: this function should be called only by ext_storage_root_version_1 func (t *TrieState) Root() (common.Hash, error) { + t.CommitTransaction() + // Since the Root function is called without running transactions we can do: if currentTx := t.getCurrentTransaction(); currentTx != nil { panic("cannot calculate root with running transactions") diff --git a/lib/runtime/storage/trie_test.go b/lib/runtime/storage/trie_test.go index 9d9b2f05f0..e263f705f3 100644 --- a/lib/runtime/storage/trie_test.go +++ b/lib/runtime/storage/trie_test.go @@ -588,8 +588,8 @@ func TestTrieState_Root(t *testing.T) { ts.Put([]byte(tc), []byte(tc)) } - expected := ts.MustRoot() - require.Equal(t, expected, ts.MustRoot()) + expected := ts.Trie().MustHash() + require.Equal(t, expected, ts.Trie().MustHash()) } func TestTrieState_ChildRoot(t *testing.T) { diff --git a/lib/runtime/wazero/imports_test.go b/lib/runtime/wazero/imports_test.go index c21520349f..41eecc1827 100644 --- a/lib/runtime/wazero/imports_test.go +++ b/lib/runtime/wazero/imports_test.go @@ -2182,6 +2182,7 @@ func Test_ext_storage_read_version_1_OffsetLargerThanValue(t *testing.T) { func Test_ext_storage_root_version_1(t *testing.T) { inst := NewTestInstance(t, runtime.HOST_API_TEST_RUNTIME, TestWithVersion(DefaultVersion)) + inst.Context.Storage.StartTransaction() ret, err := inst.Exec("rtm_ext_storage_root_version_1", []byte{}) require.NoError(t, err) @@ -2200,6 +2201,7 @@ func Test_ext_storage_root_version_2(t *testing.T) { encVersion, err := scale.Marshal(stateVersion) require.NoError(t, err) + inst.Context.Storage.StartTransaction() ret, err := inst.Exec("rtm_ext_storage_root_version_2", encVersion) require.NoError(t, err) diff --git a/lib/runtime/wazero/instance.go b/lib/runtime/wazero/instance.go index d126f2f2a4..d817a2e617 100644 --- a/lib/runtime/wazero/instance.go +++ b/lib/runtime/wazero/instance.go @@ -930,6 +930,7 @@ func (in *Instance) InitializeBlock(header *types.Header) error { return fmt.Errorf("cannot encode header: %w", err) } + in.Context.Storage.StartTransaction() _, err = in.Exec(runtime.CoreInitializeBlock, encodedHeader) return err } @@ -992,6 +993,10 @@ func (in *Instance) ExecuteBlock(block *types.Block) ([]byte, error) { return nil, err } + // start an changeset at the beginning of the block execution + // then clear prefix can work correctly by ignoring + // keys included under current block execution + in.Context.Storage.StartTransaction() return in.Exec(runtime.CoreExecuteBlock, bdEnc) } diff --git a/lib/runtime/wazero/instance_test.go b/lib/runtime/wazero/instance_test.go index 46784db52a..a173fbeee0 100644 --- a/lib/runtime/wazero/instance_test.go +++ b/lib/runtime/wazero/instance_test.go @@ -1013,8 +1013,7 @@ func TestInstance_ExecuteBlock_PaseoRuntime_PaseoBlock1008649(t *testing.T) { require.NoError(t, err) expectedRootNew := common.MustHexToHash("0xc75b6a15438acb997f925a09714092fc463af3ba44ab93654c89b775c44dfe13") - require.Equal(t, expectedRootNew, state.MustRoot()) - + require.Equal(t, expectedRootNew, state.Trie().MustHash()) } func TestInstance_ExecuteBlock_PolkadotBlock1089328(t *testing.T) {