From e5c675e9354bec410f87f7817ebb2c8bbd9dcf61 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Thu, 10 Nov 2022 16:40:54 +0000 Subject: [PATCH] wip --- dot/core/messages.go | 24 +- dot/core/messages_test.go | 641 ++++++++++++++++--------------- dot/core/service.go | 25 +- dot/core/service_test.go | 32 +- dot/services.go | 3 + dot/state/storage.go | 6 +- dot/state/storage_notify.go | 1 + dot/state/storage_test.go | 6 +- dot/state/test_helpers.go | 2 + dot/sync/chain_processor.go | 17 +- dot/sync/chain_processor_test.go | 48 ++- lib/babe/babe.go | 4 +- lib/runtime/storage/trie.go | 9 +- 13 files changed, 449 insertions(+), 369 deletions(-) diff --git a/dot/core/messages.go b/dot/core/messages.go index 53988c07b33..a28d50954ab 100644 --- a/dot/core/messages.go +++ b/dot/core/messages.go @@ -20,12 +20,16 @@ func (s *Service) validateTransaction(head *types.Header, rt RuntimeInstance, s.storageState.Lock() ts, err := s.storageState.TrieState(&head.StateRoot) - s.storageState.Unlock() if err != nil { - return nil, fmt.Errorf("cannot get trie state from storage for root %s: %w", head.StateRoot, err) + s.storageState.Unlock() + return nil, fmt.Errorf("getting trie state from storage: %w", err) } - rt.SetContextStorage(ts) + // ValidateTransaction modifies the trie state so we want to snapshot it + // so that the original trie state remains unaffected. + temporaryState := ts.Snapshot() + + s.storageState.Unlock() // validate each transaction externalExt, err := s.buildExternalTransaction(rt, tx) @@ -33,7 +37,13 @@ func (s *Service) validateTransaction(head *types.Header, rt RuntimeInstance, return nil, fmt.Errorf("building external transaction: %w", err) } + rt.SetContextStorage(temporaryState) + validity, err = rt.ValidateTransaction(externalExt) + + // Clear the reference to the snapshoted trie state so it can get garbage collected. + rt.SetContextStorage(nil) + if err != nil { logger.Debugf("failed to validate transaction: %s", err) return nil, err @@ -50,8 +60,8 @@ func (s *Service) validateTransaction(head *types.Header, rt RuntimeInstance, // HandleTransactionMessage validates each transaction in the message and // adds valid transactions to the transaction queue of the BABE session -// returns boolean for transaction propagation, true - transactions should be propagated -func (s *Service) HandleTransactionMessage(peerID peer.ID, msg *network.TransactionMessage) (bool, error) { +func (s *Service) HandleTransactionMessage(peerID peer.ID, msg *network.TransactionMessage) ( + propagateTransactions bool, err error) { logger.Debug("received TransactionMessage") if !s.net.IsSynced() { @@ -65,13 +75,13 @@ func (s *Service) HandleTransactionMessage(peerID peer.ID, msg *network.Transact head, err := s.blockState.BestBlockHeader() if err != nil { - return false, err + return false, fmt.Errorf("getting best block header: %w", err) } bestBlockHash := head.Hash() rt, err := s.blockState.GetRuntime(bestBlockHash) if err != nil { - return false, err + return false, fmt.Errorf("getting runtime from block state: %w", err) } allTxnsAreValid := true diff --git a/dot/core/messages_test.go b/dot/core/messages_test.go index 75679c94832..a1f57d6d700 100644 --- a/dot/core/messages_test.go +++ b/dot/core/messages_test.go @@ -4,7 +4,6 @@ package core import ( - "bytes" "errors" "testing" @@ -15,68 +14,15 @@ import ( "github.com/ChainSafe/gossamer/lib/runtime" "github.com/ChainSafe/gossamer/lib/runtime/storage" "github.com/ChainSafe/gossamer/lib/transaction" + "github.com/ChainSafe/gossamer/lib/trie" "github.com/golang/mock/gomock" "github.com/libp2p/go-libp2p-core/peer" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) var errDummyErr = errors.New("dummy error for testing") -type mockReportPeer struct { - change peerset.ReputationChange - id peer.ID -} - -type mockNetwork struct { - IsSynced bool - ReportPeer *mockReportPeer -} - -type mockBestHeader struct { - header *types.Header - err error -} - -type mockGetRuntime struct { - runtime RuntimeInstance - err error -} - -type mockBlockState struct { - bestHeader *mockBestHeader - getRuntime *mockGetRuntime - callsBestBlockHash bool -} - -type mockStorageState struct { - input *common.Hash - trieState *storage.TrieState - err error -} - -type mockTxnState struct { - input *transaction.ValidTransaction - hash common.Hash -} - -type mockSetContextStorage struct { - trieState *storage.TrieState -} - -type mockValidateTxn struct { - input types.Extrinsic - validity *transaction.Validity - err error -} - -type mockRuntime struct { - runtime *MockRuntimeInstance - setContextStorage *mockSetContextStorage - validateTxn *mockValidateTxn -} - func TestService_TransactionsCount(t *testing.T) { ctrl := gomock.NewController(t) mockTxnStateEmpty := NewMockTransactionState(ctrl) @@ -112,295 +58,364 @@ func TestService_TransactionsCount(t *testing.T) { } } -func TestServiceHandleTransactionMessage(t *testing.T) { - testEmptyHeader := types.NewEmptyHeader() - testExtrinsic := []types.Extrinsic{{1, 2, 3}} +func Test_Service_HandleTransactionMessage(t *testing.T) { + t.Parallel() - ctrl := gomock.NewController(t) - runtimeMock := NewMockRuntimeInstance(ctrl) - runtimeMock2 := NewMockRuntimeInstance(ctrl) - runtimeMock3 := NewMockRuntimeInstance(ctrl) + errTest := errors.New("test error") - invalidTransaction := runtime.NewInvalidTransaction() - err := invalidTransaction.Set(runtime.Future{}) - require.NoError(t, err) - - type args struct { - peerID peer.ID - msg *network.TransactionMessage + someHeader := &types.Header{ + Number: 2, + StateRoot: common.Hash{2}, } - tests := []struct { - name string - service *Service - args args - exp bool - expErr error - expErrMsg string - ctrl *gomock.Controller - mockNetwork *mockNetwork - mockBlockState *mockBlockState - mockStorageState *mockStorageState - mockTxnState *mockTxnState - mockRuntime *mockRuntime + someHeaderHash := someHeader.Hash() + + testCases := map[string]struct { + serviceBuilder func(ctrl *gomock.Controller) *Service + peerID peer.ID + message *network.TransactionMessage + propagateTransactions bool + errSentinel error + errMessage string + expectedMessage *network.TransactionMessage }{ - { - name: "not synced", - mockNetwork: &mockNetwork{ - IsSynced: false, + "not synced": { + serviceBuilder: func(ctrl *gomock.Controller) *Service { + network := NewMockNetwork(ctrl) + network.EXPECT().IsSynced().Return(false) + return &Service{ + net: network, + } }, }, - { - name: "best block header error", - mockNetwork: &mockNetwork{ - IsSynced: true, - }, - mockBlockState: &mockBlockState{ - bestHeader: &mockBestHeader{ - err: errDummyErr, - }, - }, - args: args{ - msg: &network.TransactionMessage{Extrinsics: []types.Extrinsic{}}, + "best block header error": { + serviceBuilder: func(ctrl *gomock.Controller) *Service { + network := NewMockNetwork(ctrl) + network.EXPECT().IsSynced().Return(true) + + blockState := NewMockBlockState(ctrl) + blockState.EXPECT().BestBlockHeader(). + Return(nil, errTest) + + return &Service{ + net: network, + blockState: blockState, + } }, - expErr: errDummyErr, - expErrMsg: errDummyErr.Error(), + message: &network.TransactionMessage{}, + errSentinel: errTest, + errMessage: "getting best block header: test error", + expectedMessage: &network.TransactionMessage{}, }, - { - name: "get runtime error", - mockNetwork: &mockNetwork{ - IsSynced: true, - }, - mockBlockState: &mockBlockState{ - bestHeader: &mockBestHeader{ - header: testEmptyHeader, - }, - getRuntime: &mockGetRuntime{ - err: errDummyErr, - }, - }, - args: args{ - msg: &network.TransactionMessage{Extrinsics: []types.Extrinsic{}}, + "get runtime error": { + serviceBuilder: func(ctrl *gomock.Controller) *Service { + network := NewMockNetwork(ctrl) + network.EXPECT().IsSynced().Return(true) + + blockState := NewMockBlockState(ctrl) + blockState.EXPECT().BestBlockHeader(). + Return(someHeader, nil) + blockState.EXPECT().GetRuntime(someHeaderHash). + Return(nil, errTest) + + return &Service{ + net: network, + blockState: blockState, + } }, - expErr: errDummyErr, - expErrMsg: errDummyErr.Error(), + message: &network.TransactionMessage{}, + errSentinel: errTest, + errMessage: "getting runtime from block state: test error", + expectedMessage: &network.TransactionMessage{}, }, - { - name: "happy path no loop", - mockNetwork: &mockNetwork{ - IsSynced: true, - ReportPeer: &mockReportPeer{ - change: peerset.ReputationChange{ - Value: peerset.GoodTransactionValue, - Reason: peerset.GoodTransactionReason, - }, - }, - }, - mockBlockState: &mockBlockState{ - bestHeader: &mockBestHeader{ - header: testEmptyHeader, - }, - getRuntime: &mockGetRuntime{ - runtime: runtimeMock, - }, - }, - args: args{ - peerID: peer.ID("jimbo"), - msg: &network.TransactionMessage{Extrinsics: []types.Extrinsic{}}, + "zero transaction": { + serviceBuilder: func(ctrl *gomock.Controller) *Service { + network := NewMockNetwork(ctrl) + network.EXPECT().IsSynced().Return(true) + + blockState := NewMockBlockState(ctrl) + blockState.EXPECT().BestBlockHeader(). + Return(someHeader, nil) + blockState.EXPECT().GetRuntime(someHeaderHash). + Return(nil, nil) + + network.EXPECT().ReportPeer(peerset.ReputationChange{ + Value: peerset.GoodTransactionValue, + Reason: peerset.GoodTransactionReason, + }, peer.ID("a")) + + return &Service{ + net: network, + blockState: blockState, + } }, + peerID: peer.ID("a"), + message: &network.TransactionMessage{}, + expectedMessage: &network.TransactionMessage{}, }, - { - name: "trie state error", - mockNetwork: &mockNetwork{ - IsSynced: true, - }, - mockBlockState: &mockBlockState{ - bestHeader: &mockBestHeader{ - header: testEmptyHeader, - }, - getRuntime: &mockGetRuntime{ - runtime: runtimeMock, - }, + "valid transaction to propagate": { + serviceBuilder: func(ctrl *gomock.Controller) *Service { + network := NewMockNetwork(ctrl) + network.EXPECT().IsSynced().Return(true) + + blockState := NewMockBlockState(ctrl) + blockState.EXPECT().BestBlockHeader(). + Return(someHeader, nil) + runtimeInstance := NewMockRuntimeInstance(ctrl) + blockState.EXPECT().GetRuntime(someHeaderHash). + Return(runtimeInstance, nil) + + storageState := NewMockStorageState(ctrl) + storageState.EXPECT().Lock() + trieState := storage.NewTrieState(trie.NewEmptyTrie()) + storageState.EXPECT().TrieState(&common.Hash{2}). + Return(trieState, nil) + storageState.EXPECT().Unlock() + + version := runtime.Version{ + APIItems: []runtime.APIItem{{ + Name: common.MustBlake2b8([]byte("TaggedTransactionQueue")), + Ver: 2, + }}, + } + runtimeInstance.EXPECT().Version().Return(version) + runtimeInstance.EXPECT().SetContextStorage(trieState.Snapshot()) + validity := &transaction.Validity{Propagate: true} + externalExtrinsic := []byte{2, 1, 2, 3} + runtimeInstance.EXPECT().ValidateTransaction(externalExtrinsic). + Return(validity, nil) + runtimeInstance.EXPECT().SetContextStorage(nil) + + transactionState := NewMockTransactionState(ctrl) + validTransaction := &transaction.ValidTransaction{ + Extrinsic: types.Extrinsic{1, 2, 3}, + Validity: validity, + } + transactionState.EXPECT().AddToPool(validTransaction). + Return(common.Hash{3}) + + network.EXPECT().ReportPeer(peerset.ReputationChange{ + Value: peerset.GoodTransactionValue, + Reason: peerset.GoodTransactionReason, + }, peer.ID("a")) + + return &Service{ + net: network, + blockState: blockState, + storageState: storageState, + transactionState: transactionState, + } }, - mockStorageState: &mockStorageState{ - input: &common.Hash{}, - err: errDummyErr, + peerID: peer.ID("a"), + message: &network.TransactionMessage{ + Extrinsics: []types.Extrinsic{{1, 2, 3}}, }, - args: args{ - peerID: peer.ID("jimbo"), - msg: &network.TransactionMessage{ - Extrinsics: []types.Extrinsic{{1, 2, 3}, {7, 8, 9, 0}, {0xa, 0xb}}, - }, + propagateTransactions: true, + expectedMessage: &network.TransactionMessage{ + Extrinsics: []types.Extrinsic{{1, 2, 3}}, }, - expErr: errDummyErr, - expErrMsg: "validating transaction from peerID D1KeRhQ: cannot get trie state from storage" + - " for root 0x0000000000000000000000000000000000000000000000000000000000000000: dummy error for testing", }, - { - name: "runtime.ErrInvalidTransaction", - mockNetwork: &mockNetwork{ - IsSynced: true, - ReportPeer: &mockReportPeer{ - change: peerset.ReputationChange{ - Value: peerset.BadTransactionValue, - Reason: peerset.BadTransactionReason, - }, - id: peer.ID("jimbo"), - }, - }, - mockBlockState: &mockBlockState{ - bestHeader: &mockBestHeader{ - header: testEmptyHeader, - }, - getRuntime: &mockGetRuntime{ - runtime: runtimeMock2, - }, - callsBestBlockHash: true, - }, - mockStorageState: &mockStorageState{ - input: &common.Hash{}, - trieState: &storage.TrieState{}, - }, - mockRuntime: &mockRuntime{ - runtime: runtimeMock2, - setContextStorage: &mockSetContextStorage{trieState: &storage.TrieState{}}, - validateTxn: &mockValidateTxn{ - input: types.Extrinsic(bytes.Join([][]byte{ - {byte(types.TxnExternal)}, - testExtrinsic[0], - testEmptyHeader.StateRoot.ToBytes(), - }, nil)), - err: invalidTransaction, - }, + "valid transaction to not propagate": { + serviceBuilder: func(ctrl *gomock.Controller) *Service { + network := NewMockNetwork(ctrl) + network.EXPECT().IsSynced().Return(true) + + blockState := NewMockBlockState(ctrl) + blockState.EXPECT().BestBlockHeader(). + Return(someHeader, nil) + runtimeInstance := NewMockRuntimeInstance(ctrl) + blockState.EXPECT().GetRuntime(someHeaderHash). + Return(runtimeInstance, nil) + + storageState := NewMockStorageState(ctrl) + storageState.EXPECT().Lock() + trieState := storage.NewTrieState(trie.NewEmptyTrie()) + storageState.EXPECT().TrieState(&common.Hash{2}). + Return(trieState, nil) + storageState.EXPECT().Unlock() + + version := runtime.Version{ + APIItems: []runtime.APIItem{{ + Name: common.MustBlake2b8([]byte("TaggedTransactionQueue")), + Ver: 2, + }}, + } + runtimeInstance.EXPECT().Version().Return(version) + runtimeInstance.EXPECT().SetContextStorage(trieState.Snapshot()) + validity := &transaction.Validity{} + externalExtrinsic := []byte{2, 1, 2, 3} + runtimeInstance.EXPECT().ValidateTransaction(externalExtrinsic). + Return(validity, nil) + runtimeInstance.EXPECT().SetContextStorage(nil) + + transactionState := NewMockTransactionState(ctrl) + validTransaction := &transaction.ValidTransaction{ + Extrinsic: types.Extrinsic{1, 2, 3}, + Validity: validity, + } + transactionState.EXPECT().AddToPool(validTransaction). + Return(common.Hash{3}) + + network.EXPECT().ReportPeer(peerset.ReputationChange{ + Value: peerset.GoodTransactionValue, + Reason: peerset.GoodTransactionReason, + }, peer.ID("a")) + + return &Service{ + net: network, + blockState: blockState, + storageState: storageState, + transactionState: transactionState, + } }, - args: args{ - peerID: peer.ID("jimbo"), - msg: &network.TransactionMessage{ - Extrinsics: []types.Extrinsic{{1, 2, 3}}, - }, + peerID: peer.ID("a"), + message: &network.TransactionMessage{ + Extrinsics: []types.Extrinsic{{1, 2, 3}}, }, + expectedMessage: &network.TransactionMessage{}, }, - { - name: "validTransaction", - mockNetwork: &mockNetwork{ - IsSynced: true, - ReportPeer: &mockReportPeer{ - change: peerset.ReputationChange{ - Value: peerset.GoodTransactionValue, - Reason: peerset.GoodTransactionReason, - }, - id: peer.ID("jimbo"), - }, - }, - mockBlockState: &mockBlockState{ - bestHeader: &mockBestHeader{ - header: testEmptyHeader, - }, - getRuntime: &mockGetRuntime{ - runtime: runtimeMock3, - }, - callsBestBlockHash: true, - }, - mockStorageState: &mockStorageState{ - input: &common.Hash{}, - trieState: &storage.TrieState{}, - }, - mockTxnState: &mockTxnState{ - input: transaction.NewValidTransaction( - types.Extrinsic{1, 2, 3}, - &transaction.Validity{ - Propagate: true, - }), - hash: common.Hash{}, - }, - mockRuntime: &mockRuntime{ - runtime: runtimeMock3, - setContextStorage: &mockSetContextStorage{trieState: &storage.TrieState{}}, - validateTxn: &mockValidateTxn{ - input: types.Extrinsic(bytes.Join([][]byte{ - {byte(types.TxnExternal)}, - testExtrinsic[0], - testEmptyHeader.StateRoot.ToBytes(), - }, nil)), - validity: &transaction.Validity{Propagate: true}, - }, + "invalid transaction": { + serviceBuilder: func(ctrl *gomock.Controller) *Service { + network := NewMockNetwork(ctrl) + network.EXPECT().IsSynced().Return(true) + + blockState := NewMockBlockState(ctrl) + blockState.EXPECT().BestBlockHeader(). + Return(someHeader, nil) + runtimeInstance := NewMockRuntimeInstance(ctrl) + blockState.EXPECT().GetRuntime(someHeaderHash). + Return(runtimeInstance, nil) + + storageState := NewMockStorageState(ctrl) + storageState.EXPECT().Lock() + trieState := storage.NewTrieState(trie.NewEmptyTrie()) + storageState.EXPECT().TrieState(&common.Hash{2}). + Return(trieState, nil) + storageState.EXPECT().Unlock() + + version := runtime.Version{ + APIItems: []runtime.APIItem{{ + Name: common.MustBlake2b8([]byte("TaggedTransactionQueue")), + Ver: 2, + }}, + } + runtimeInstance.EXPECT().Version().Return(version) + runtimeInstance.EXPECT().SetContextStorage(trieState.Snapshot()) + externalExtrinsic := []byte{2, 1, 2, 3} + runtimeInstance.EXPECT().ValidateTransaction(externalExtrinsic). + Return(nil, runtime.InvalidTransaction{}) + runtimeInstance.EXPECT().SetContextStorage(nil) + + network.EXPECT().ReportPeer(peerset.ReputationChange{ + Value: peerset.BadTransactionValue, + Reason: peerset.BadTransactionReason, + }, peer.ID("a")) + + return &Service{ + net: network, + blockState: blockState, + storageState: storageState, + } }, - args: args{ - peerID: peer.ID("jimbo"), - msg: &network.TransactionMessage{ - Extrinsics: []types.Extrinsic{{1, 2, 3}}, - }, + peerID: peer.ID("a"), + message: &network.TransactionMessage{ + Extrinsics: []types.Extrinsic{{1, 2, 3}}, }, - exp: true, + expectedMessage: &network.TransactionMessage{}, }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - s := &Service{} - ctrl := gomock.NewController(t) - if tt.mockNetwork != nil { - mockNet := NewMockNetwork(ctrl) - mockNet.EXPECT().IsSynced().Return(tt.mockNetwork.IsSynced) - if tt.mockNetwork.ReportPeer != nil { - mockNet.EXPECT().ReportPeer(tt.mockNetwork.ReportPeer.change, tt.args.peerID) - } - s.net = mockNet - } - if tt.mockBlockState != nil { + "unknown transaction": { + serviceBuilder: func(ctrl *gomock.Controller) *Service { + network := NewMockNetwork(ctrl) + network.EXPECT().IsSynced().Return(true) + blockState := NewMockBlockState(ctrl) - blockState.EXPECT().BestBlockHeader().Return( - tt.mockBlockState.bestHeader.header, - tt.mockBlockState.bestHeader.err) - - if tt.mockBlockState.getRuntime != nil { - blockState.EXPECT().GetRuntime(gomock.Any()).Return( - tt.mockBlockState.getRuntime.runtime, - tt.mockBlockState.getRuntime.err) - } - if tt.mockBlockState.callsBestBlockHash { - blockState.EXPECT().BestBlockHash().Return(common.Hash{}) - } - s.blockState = blockState - } - if tt.mockStorageState != nil { + blockState.EXPECT().BestBlockHeader(). + Return(someHeader, nil) + runtimeInstance := NewMockRuntimeInstance(ctrl) + blockState.EXPECT().GetRuntime(someHeaderHash). + Return(runtimeInstance, nil) + storageState := NewMockStorageState(ctrl) storageState.EXPECT().Lock() + trieState := storage.NewTrieState(trie.NewEmptyTrie()) + storageState.EXPECT().TrieState(&common.Hash{2}). + Return(trieState, nil) storageState.EXPECT().Unlock() - storageState.EXPECT().TrieState(tt.mockStorageState.input).Return( - tt.mockStorageState.trieState, - tt.mockStorageState.err) - s.storageState = storageState - } - if tt.mockTxnState != nil { - txnState := NewMockTransactionState(ctrl) - txnState.EXPECT().AddToPool(tt.mockTxnState.input).Return(tt.mockTxnState.hash) - s.transactionState = txnState - } - if tt.mockRuntime != nil { - rt := tt.mockRuntime.runtime - rt.EXPECT().SetContextStorage(tt.mockRuntime.setContextStorage.trieState) - rt.EXPECT().ValidateTransaction(tt.mockRuntime.validateTxn.input). - Return(tt.mockRuntime.validateTxn.validity, tt.mockRuntime.validateTxn.err) - rt.EXPECT().Version().Return(runtime.Version{ - SpecName: []byte("polkadot"), - ImplName: []byte("parity-polkadot"), - AuthoringVersion: authoringVersion, - SpecVersion: specVersion, - ImplVersion: implVersion, + + version := runtime.Version{ APIItems: []runtime.APIItem{{ Name: common.MustBlake2b8([]byte("TaggedTransactionQueue")), - Ver: 3, + Ver: 2, }}, - TransactionVersion: transactionVersion, - StateVersion: stateVersion, - }) - } + } + runtimeInstance.EXPECT().Version().Return(version) + runtimeInstance.EXPECT().SetContextStorage(trieState.Snapshot()) + externalExtrinsic := []byte{2, 1, 2, 3} + runtimeInstance.EXPECT().ValidateTransaction(externalExtrinsic). + Return(nil, runtime.UnknownTransaction{}) + runtimeInstance.EXPECT().SetContextStorage(nil) + + return &Service{ + net: network, + blockState: blockState, + storageState: storageState, + } + }, + peerID: peer.ID("a"), + message: &network.TransactionMessage{ + Extrinsics: []types.Extrinsic{{1, 2, 3}}, + }, + expectedMessage: &network.TransactionMessage{}, + }, + "validate transaction other error": { + serviceBuilder: func(ctrl *gomock.Controller) *Service { + network := NewMockNetwork(ctrl) + network.EXPECT().IsSynced().Return(true) - res, err := s.HandleTransactionMessage(tt.args.peerID, tt.args.msg) - assert.ErrorIs(t, err, tt.expErr) - if tt.expErr != nil { - assert.EqualError(t, err, tt.expErrMsg) + blockState := NewMockBlockState(ctrl) + blockState.EXPECT().BestBlockHeader(). + Return(someHeader, nil) + runtimeInstance := NewMockRuntimeInstance(ctrl) + blockState.EXPECT().GetRuntime(someHeaderHash). + Return(runtimeInstance, nil) + + storageState := NewMockStorageState(ctrl) + storageState.EXPECT().Lock() + storageState.EXPECT().TrieState(&common.Hash{2}). + Return(nil, errTest) + storageState.EXPECT().Unlock() + + return &Service{ + net: network, + blockState: blockState, + storageState: storageState, + } + }, + peerID: peer.ID("a"), + message: &network.TransactionMessage{ + Extrinsics: []types.Extrinsic{{1, 2, 3}}, + }, + errSentinel: errTest, + errMessage: "validating transaction from peerID 2g: " + + "getting trie state from storage: test error", + expectedMessage: &network.TransactionMessage{}, + }, + } + + for name, testCase := range testCases { + testCase := testCase + t.Run(name, func(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + + service := testCase.serviceBuilder(ctrl) + + propagateTransactions, err := service.HandleTransactionMessage(testCase.peerID, testCase.message) + + assert.Equal(t, testCase.propagateTransactions, propagateTransactions) + assert.ErrorIs(t, err, testCase.errSentinel) + if testCase.errSentinel != nil { + assert.EqualError(t, err, testCase.errMessage) } - assert.Equal(t, tt.exp, res) }) } } diff --git a/dot/core/service.go b/dot/core/service.go index 4bead9b676d..0048393d773 100644 --- a/dot/core/service.go +++ b/dot/core/service.go @@ -122,6 +122,7 @@ func (s *Service) StorageRoot() (common.Hash, error) { if err != nil { return common.Hash{}, err } + ts = ts.Snapshot() // TODO remove return ts.Root() } @@ -419,8 +420,7 @@ func (s *Service) maintainTransactionPool(block *types.Block, bestBlockHash comm ts, err := s.storageState.TrieState(stateRoot) if err != nil { - logger.Errorf(err.Error()) - return err + return fmt.Errorf("getting trie state: %w", err) } // re-validate transactions in the pool and move them to the queue @@ -432,13 +432,21 @@ func (s *Service) maintainTransactionPool(block *types.Block, bestBlockHash comm return fmt.Errorf("failed to get runtime to re-validate transactions in pool: %s", err) } - rt.SetContextStorage(ts) externalExt, err := s.buildExternalTransaction(rt, tx.Extrinsic) if err != nil { return fmt.Errorf("building external transaction: %s", err) } + // ValidateTransaction modifies the trie state so we want to snapshot it + // so that the original trie state remains unaffected. + temporaryState := ts.Snapshot() + rt.SetContextStorage(temporaryState) + txnValidity, err := rt.ValidateTransaction(externalExt) + + // Clear the reference to the snapshoted trie state so it can get garbage collected. + rt.SetContextStorage(nil) + if err != nil { logger.Debugf("failed to validate transaction for extrinsic %s: %s", tx.Extrinsic, err) s.transactionState.RemoveExtrinsic(tx.Extrinsic) @@ -534,14 +542,21 @@ func (s *Service) HandleSubmittedExtrinsic(ext types.Extrinsic) error { return err } - rt.SetContextStorage(ts) - externalExt, err := s.buildExternalTransaction(rt, ext) if err != nil { return fmt.Errorf("building external transaction: %w", err) } + // ValidateTransaction modifies the trie state so we want to snapshot it + // so that the original trie state remains unaffected. + temporaryState := ts.Snapshot() + rt.SetContextStorage(temporaryState) + transactionValidity, err := rt.ValidateTransaction(externalExt) + + // Clear the reference to the snapshoted trie state so it can get garbage collected. + rt.SetContextStorage(nil) + if err != nil { return err } diff --git a/dot/core/service_test.go b/dot/core/service_test.go index 0f8167cbaaf..98b6f3dd80a 100644 --- a/dot/core/service_test.go +++ b/dot/core/service_test.go @@ -22,6 +22,7 @@ import ( rtstorage "github.com/ChainSafe/gossamer/lib/runtime/storage" "github.com/ChainSafe/gossamer/lib/runtime/wasmer" "github.com/ChainSafe/gossamer/lib/transaction" + "github.com/ChainSafe/gossamer/lib/trie" "github.com/ChainSafe/gossamer/pkg/scale" cscale "github.com/centrifuge/go-substrate-rpc-client/v4/scale" "github.com/centrifuge/go-substrate-rpc-client/v4/signature" @@ -583,7 +584,12 @@ func Test_Service_maintainTransactionPool(t *testing.T) { TransactionVersion: transactionVersion, StateVersion: stateVersion, }) - runtimeMock.EXPECT().SetContextStorage(&rtstorage.TrieState{}) + + originalTrie := trie.NewEmptyTrie() + originalTrieState := rtstorage.NewTrieState(originalTrie) + snapshotTrieStateOnce := originalTrieState.Snapshot() + setCtxStorageCallOne := runtimeMock.EXPECT().SetContextStorage(snapshotTrieStateOnce) + runtimeMock.EXPECT().SetContextStorage(nil).After(setCtxStorageCallOne) mockTxnState := NewMockTransactionState(ctrl) mockTxnState.EXPECT().RemoveExtrinsic(types.Extrinsic{21}).Times(2) @@ -596,7 +602,7 @@ func Test_Service_maintainTransactionPool(t *testing.T) { Return(common.Hash{}).After(runtimeBlockHashCall) mockStorageState := NewMockStorageState(ctrl) - mockStorageState.EXPECT().TrieState(&common.Hash{1}).Return(&rtstorage.TrieState{}, nil) + mockStorageState.EXPECT().TrieState(&common.Hash{1}).Return(originalTrieState, nil) mockStorageState.EXPECT().GetStateRootFromBlock(&common.Hash{1}).Return(&common.Hash{1}, nil) service := &Service{ transactionState: mockTxnState, @@ -648,7 +654,13 @@ func Test_Service_maintainTransactionPool(t *testing.T) { TransactionVersion: transactionVersion, StateVersion: stateVersion, }) - runtimeMock.EXPECT().SetContextStorage(&rtstorage.TrieState{}) + + originalTrie := trie.NewEmptyTrie() + originalTrieState := rtstorage.NewTrieState(originalTrie) + snapshotTrieStateOnce := originalTrieState.Snapshot() + setCtxStorageCallOne := runtimeMock.EXPECT().SetContextStorage(snapshotTrieStateOnce) + runtimeMock.EXPECT().SetContextStorage(nil).After(setCtxStorageCallOne) + mockTxnState := NewMockTransactionState(ctrl) mockTxnState.EXPECT().RemoveExtrinsic(types.Extrinsic{21}) mockTxnState.EXPECT().PendingInPool().Return([]*transaction.ValidTransaction{vt}) @@ -663,7 +675,7 @@ func Test_Service_maintainTransactionPool(t *testing.T) { Return(common.Hash{}).After(runtimeBlockHashCall) mockStorageState := NewMockStorageState(ctrl) - mockStorageState.EXPECT().TrieState(&common.Hash{1}).Return(&rtstorage.TrieState{}, nil) + mockStorageState.EXPECT().TrieState(&common.Hash{1}).Return(originalTrieState, nil) mockStorageState.EXPECT().GetStateRootFromBlock(&common.Hash{1}).Return(&common.Hash{1}, nil) service := &Service{ transactionState: mockTxnState, @@ -1206,7 +1218,8 @@ func TestServiceHandleSubmittedExtrinsic(t *testing.T) { mockBlockState.EXPECT().BestBlockHash().Return(common.Hash{}) mockStorageState := NewMockStorageState(ctrl) - mockStorageState.EXPECT().TrieState(&common.Hash{}).Return(&rtstorage.TrieState{}, nil) + trieState := rtstorage.NewTrieState(trie.NewEmptyTrie()) + mockStorageState.EXPECT().TrieState(&common.Hash{}).Return(trieState, nil) mockStorageState.EXPECT().GetStateRootFromBlock(&common.Hash{}).Return(&common.Hash{}, nil) mockTxnState := NewMockTransactionState(ctrl) @@ -1226,7 +1239,8 @@ func TestServiceHandleSubmittedExtrinsic(t *testing.T) { TransactionVersion: transactionVersion, StateVersion: stateVersion, }) - runtimeMockErr.EXPECT().SetContextStorage(&rtstorage.TrieState{}) + firstSetCtxStorageCall := runtimeMockErr.EXPECT().SetContextStorage(trieState.Snapshot()) + runtimeMockErr.EXPECT().SetContextStorage(nil).After(firstSetCtxStorageCall) service := &Service{ storageState: mockStorageState, transactionState: mockTxnState, @@ -1260,10 +1274,12 @@ func TestServiceHandleSubmittedExtrinsic(t *testing.T) { TransactionVersion: transactionVersion, StateVersion: stateVersion, }) - runtimeMock.EXPECT().SetContextStorage(&rtstorage.TrieState{}) mockStorageState := NewMockStorageState(ctrl) - mockStorageState.EXPECT().TrieState(&common.Hash{}).Return(&rtstorage.TrieState{}, nil) + trieState := rtstorage.NewTrieState(trie.NewEmptyTrie()) + mockStorageState.EXPECT().TrieState(&common.Hash{}).Return(trieState, nil) + firstSetCtxStorageCall := runtimeMock.EXPECT().SetContextStorage(trieState.Snapshot()) + runtimeMock.EXPECT().SetContextStorage(nil).After(firstSetCtxStorageCall) mockStorageState.EXPECT().GetStateRootFromBlock(&common.Hash{}).Return(&common.Hash{}, nil) mockTxnState := NewMockTransactionState(ctrl) diff --git a/dot/services.go b/dot/services.go index 1aaf7b2295a..eff26bd1e91 100644 --- a/dot/services.go +++ b/dot/services.go @@ -125,6 +125,9 @@ func createRuntime(cfg *Config, ns runtime.NodeStorage, st *state.Service, if err != nil { return nil, err } + // TODO: Do we want to snapshot the trie to leave the previous best block + // state untouched? Why? + ts.Snapshot() codeHash, err := st.Storage.LoadCodeHash(nil) if err != nil { diff --git a/dot/state/storage.go b/dot/state/storage.go index 8023598ca80..0544232338d 100644 --- a/dot/state/storage.go +++ b/dot/state/storage.go @@ -123,11 +123,7 @@ func (s *StorageState) TrieState(root *common.Hash) (*rtstorage.TrieState, error panic("trie does not have expected root") } - nextTrie := t.Snapshot() - next := rtstorage.NewTrieState(nextTrie) - - logger.Tracef("returning trie with root %s to be modified", root) - return next, nil + return rtstorage.NewTrieState(t), nil } // LoadFromDB loads an encoded trie from the DB where the key is `root` diff --git a/dot/state/storage_notify.go b/dot/state/storage_notify.go index bfdad8a1b25..4bf00dd119a 100644 --- a/dot/state/storage_notify.go +++ b/dot/state/storage_notify.go @@ -89,6 +89,7 @@ func (s *StorageState) notifyObserver(root common.Hash, o Observer) error { if err != nil { return err } + t = t.Snapshot() // TODO remove subRes := &SubscriptionResult{ Hash: root, diff --git a/dot/state/storage_test.go b/dot/state/storage_test.go index 968a16fad00..b46adf139f2 100644 --- a/dot/state/storage_test.go +++ b/dot/state/storage_test.go @@ -41,13 +41,9 @@ func TestStorage_StoreAndLoadTrie(t *testing.T) { err = storage.StoreTrie(ts, nil) require.NoError(t, err) - time.Sleep(time.Millisecond * 100) - trie, err := storage.LoadFromDB(root) require.NoError(t, err) - ts2 := runtime.NewTrieState(trie) - new := ts2.Snapshot() - require.Equal(t, ts.Trie(), new) + require.Equal(t, ts.Trie(), trie) } func TestStorage_GetStorageByBlockHash(t *testing.T) { diff --git a/dot/state/test_helpers.go b/dot/state/test_helpers.go index 510a6c18a31..a9c3ef09359 100644 --- a/dot/state/test_helpers.go +++ b/dot/state/test_helpers.go @@ -240,6 +240,8 @@ func generateBlockWithRandomTrie(t *testing.T, serv *Service, trieState, err := serv.Storage.TrieState(nil) require.NoError(t, err) + trieState = trieState.Snapshot() + // Generate random data for trie state. rand := time.Now().UnixNano() key := []byte("testKey" + fmt.Sprint(rand)) diff --git a/dot/sync/chain_processor.go b/dot/sync/chain_processor.go index 65308f51731..64060dc52ea 100644 --- a/dot/sync/chain_processor.go +++ b/dot/sync/chain_processor.go @@ -196,6 +196,11 @@ func (c *chainProcessor) processBlockDataWithStateHeaderAndBody(blockData types. return fmt.Errorf("loading trie state: %w", err) } + // Note we don't want to snapshot the state since we want to modify the underlying trie + // for the block header state root. + // TODO + state = state.Snapshot() + err = c.blockImportHandler.HandleBlockImport(block, state, announceImportedBlock) if err != nil { return fmt.Errorf("handling block import: %w", err) @@ -258,15 +263,21 @@ func (s *chainProcessor) handleBlock(block *types.Block, announceImportedBlock b return err } - rt.SetContextStorage(ts) + nextStorageState := ts.Snapshot() + rt.SetContextStorage(nextStorageState) _, err = rt.ExecuteBlock(block) + + // Clear the runtime storage + rt.SetContextStorage(nil) + if err != nil { return fmt.Errorf("failed to execute block %d: %w", block.Header.Number, err) } - if err = s.blockImportHandler.HandleBlockImport(block, ts, announceImportedBlock); err != nil { - return err + err = s.blockImportHandler.HandleBlockImport(block, nextStorageState, announceImportedBlock) + if err != nil { + return fmt.Errorf("handling block import: %w", err) } logger.Debugf("🔗 imported block number %d with hash %s", block.Header.Number, block.Header.Hash()) diff --git a/dot/sync/chain_processor_test.go b/dot/sync/chain_processor_test.go index ed54caa7c4e..57ead415867 100644 --- a/dot/sync/chain_processor_test.go +++ b/dot/sync/chain_processor_test.go @@ -82,14 +82,16 @@ func Test_chainProcessor_handleBlock(t *testing.T) { }, "handle runtime ExecuteBlock error": { chainProcessorBuilder: func(ctrl *gomock.Controller) (chainProcessor chainProcessor) { - trieState := storage.NewTrieState(nil) + trieState := storage.NewTrieState(trie.NewEmptyTrie()) + snapshottedTrie := trieState.Snapshot() mockBlockState := NewMockBlockState(ctrl) mockBlockState.EXPECT().GetHeader(common.Hash{}).Return(&types.Header{ StateRoot: testHash, }, nil) mockInstance := NewMockRuntimeInstance(ctrl) - mockInstance.EXPECT().SetContextStorage(trieState) + mockInstance.EXPECT().SetContextStorage(snapshottedTrie) mockInstance.EXPECT().ExecuteBlock(&types.Block{Body: types.Body{}}).Return(nil, mockError) + mockInstance.EXPECT().SetContextStorage(nil) mockBlockState.EXPECT().GetRuntime(testParentHash).Return(mockInstance, nil) chainProcessor.blockState = mockBlockState mockStorageState := NewMockStorageState(ctrl) @@ -106,15 +108,17 @@ func Test_chainProcessor_handleBlock(t *testing.T) { }, "handle block import error": { chainProcessorBuilder: func(ctrl *gomock.Controller) (chainProcessor chainProcessor) { - trieState := storage.NewTrieState(nil) + trieState := storage.NewTrieState(trie.NewEmptyTrie()) + snapshottedTrie := trieState.Snapshot() mockBlockState := NewMockBlockState(ctrl) mockBlockState.EXPECT().GetHeader(common.Hash{}).Return(&types.Header{ StateRoot: testHash, }, nil) mockBlock := &types.Block{Body: types.Body{}} mockInstance := NewMockRuntimeInstance(ctrl) - mockInstance.EXPECT().SetContextStorage(trieState) + mockInstance.EXPECT().SetContextStorage(snapshottedTrie) mockInstance.EXPECT().ExecuteBlock(mockBlock).Return(nil, nil) + mockInstance.EXPECT().SetContextStorage(nil) mockBlockState.EXPECT().GetRuntime(testParentHash).Return(mockInstance, nil) chainProcessor.blockState = mockBlockState mockStorageState := NewMockStorageState(ctrl) @@ -123,8 +127,8 @@ func Test_chainProcessor_handleBlock(t *testing.T) { mockStorageState.EXPECT().Unlock() chainProcessor.storageState = mockStorageState mockBlockImportHandler := NewMockBlockImportHandler(ctrl) - mockBlockImportHandler.EXPECT().HandleBlockImport(mockBlock, - trieState, false).Return(mockError) + mockBlockImportHandler.EXPECT().HandleBlockImport(mockBlock, snapshottedTrie, false). + Return(mockError) chainProcessor.blockImportHandler = mockBlockImportHandler return }, @@ -138,7 +142,8 @@ func Test_chainProcessor_handleBlock(t *testing.T) { mockBlock := &types.Block{ Body: types.Body{}, // empty slice of extrinsics } - trieState := storage.NewTrieState(nil) + trieState := storage.NewTrieState(trie.NewEmptyTrie()) + snapshottedTrie := trieState.Snapshot() mockBlockState := NewMockBlockState(ctrl) mockHeader := &types.Header{ Number: 0, @@ -148,8 +153,9 @@ func Test_chainProcessor_handleBlock(t *testing.T) { mockBlockState.EXPECT().GetHeader(common.Hash{}).Return(mockHeader, nil) mockInstance := NewMockRuntimeInstance(ctrl) - mockInstance.EXPECT().SetContextStorage(trieState) + mockInstance.EXPECT().SetContextStorage(snapshottedTrie) mockInstance.EXPECT().ExecuteBlock(mockBlock).Return(nil, nil) + mockInstance.EXPECT().SetContextStorage(nil) mockBlockState.EXPECT().GetRuntime(mockHeaderHash).Return(mockInstance, nil) chainProcessor.blockState = mockBlockState mockStorageState := NewMockStorageState(ctrl) @@ -158,7 +164,8 @@ func Test_chainProcessor_handleBlock(t *testing.T) { mockStorageState.EXPECT().TrieState(&trie.EmptyHash).Return(trieState, nil) chainProcessor.storageState = mockStorageState mockBlockImportHandler := NewMockBlockImportHandler(ctrl) - mockBlockImportHandler.EXPECT().HandleBlockImport(mockBlock, trieState, false).Return(nil) + mockBlockImportHandler.EXPECT().HandleBlockImport(mockBlock, snapshottedTrie, false). + Return(nil) chainProcessor.blockImportHandler = mockBlockImportHandler mockTelemetry := NewMockClient(ctrl) mockTelemetry.EXPECT().SendMessage(gomock.Any()) @@ -179,6 +186,8 @@ func Test_chainProcessor_handleBlock(t *testing.T) { Body: types.Body{}, // empty slice of extrinsics } trieState := storage.NewTrieState(nil) + snapshottedTrie := trieState.Snapshot() + mockBlockState := NewMockBlockState(ctrl) mockHeader := &types.Header{ Number: 0, @@ -188,8 +197,9 @@ func Test_chainProcessor_handleBlock(t *testing.T) { mockBlockState.EXPECT().GetHeader(common.Hash{}).Return(mockHeader, nil) mockInstance := NewMockRuntimeInstance(ctrl) - mockInstance.EXPECT().SetContextStorage(trieState) + mockInstance.EXPECT().SetContextStorage(snapshottedTrie) mockInstance.EXPECT().ExecuteBlock(mockBlock).Return(nil, nil) + mockInstance.EXPECT().SetContextStorage(nil) mockBlockState.EXPECT().GetRuntime(mockHeaderHash).Return(mockInstance, nil) chainProcessor.blockState = mockBlockState mockStorageState := NewMockStorageState(ctrl) @@ -198,7 +208,8 @@ func Test_chainProcessor_handleBlock(t *testing.T) { mockStorageState.EXPECT().TrieState(&trie.EmptyHash).Return(trieState, nil) chainProcessor.storageState = mockStorageState mockBlockImportHandler := NewMockBlockImportHandler(ctrl) - mockBlockImportHandler.EXPECT().HandleBlockImport(mockBlock, trieState, true).Return(nil) + mockBlockImportHandler.EXPECT().HandleBlockImport(mockBlock, snapshottedTrie, true). + Return(nil) chainProcessor.blockImportHandler = mockBlockImportHandler mockTelemetry := NewMockClient(ctrl) mockTelemetry.EXPECT().SendMessage(gomock.Any()) @@ -529,12 +540,13 @@ func Test_chainProcessor_processBlockData(t *testing.T) { chainProcessorBuilder: func(ctrl *gomock.Controller) chainProcessor { stateRootHash := common.MustHexToHash("0x03170a2e7597b7b7e3d84c05391d139a62b157e78786d8c082f29dcf4c111314") runtimeHash := common.MustHexToHash("0x7db9db5ed9967b80143100189ba69d9e4deab85ac3570e5df25686cabe32964a") - mockTrieState := storage.NewTrieState(nil) + mockTrieState := storage.NewTrieState(trie.NewEmptyTrie()) mockBlock := &types.Block{Header: types.Header{}, Body: types.Body{}} mockInstance := NewMockRuntimeInstance(ctrl) - mockInstance.EXPECT().SetContextStorage(mockTrieState) + mockInstance.EXPECT().SetContextStorage(mockTrieState.Snapshot()) mockInstance.EXPECT().ExecuteBlock(mockBlock).Return(nil, nil) + mockInstance.EXPECT().SetContextStorage(nil) mockBlockState := NewMockBlockState(ctrl) mockBlockState.EXPECT().HasHeader(common.Hash{}).Return(false, nil) mockBlockState.EXPECT().HasBlockBody(common.Hash{}).Return(false, nil) @@ -557,8 +569,8 @@ func Test_chainProcessor_processBlockData(t *testing.T) { mockChainSync.EXPECT().syncState().Return(bootstrap) mockBlockImportHandler := NewMockBlockImportHandler(ctrl) - mockBlockImportHandler.EXPECT().HandleBlockImport(mockBlock, mockTrieState, false) - + snapshottedTrieState := mockTrieState.Snapshot() + mockBlockImportHandler.EXPECT().HandleBlockImport(mockBlock, snapshottedTrieState, false) mockTelemetry := NewMockClient(ctrl) mockTelemetry.EXPECT().SendMessage(gomock.Any()).AnyTimes() mockFinalityGadget := NewMockFinalityGadget(ctrl) @@ -863,16 +875,18 @@ func Test_chainProcessor_processBlockDataWithHeaderAndBody(t *testing.T) { blockState.EXPECT().GetRuntime(parentHeaderHash). Return(instance, nil) - instance.EXPECT().SetContextStorage(trieState) + snapshottedTrie := trieState.Snapshot() + instance.EXPECT().SetContextStorage(snapshottedTrie) block := &types.Block{ Header: *expectedHeader, Body: types.Body{{2}}, } instance.EXPECT().ExecuteBlock(block).Return(nil, nil) + instance.EXPECT().SetContextStorage(nil) blockImportHandler := NewMockBlockImportHandler(ctrl) const announceImportedBlock = true - blockImportHandler.EXPECT().HandleBlockImport(block, trieState, announceImportedBlock). + blockImportHandler.EXPECT().HandleBlockImport(block, snapshottedTrie, announceImportedBlock). Return(nil) telemetryClient := NewMockClient(ctrl) diff --git a/lib/babe/babe.go b/lib/babe/babe.go index 63c3b0dc742..20d943357c4 100644 --- a/lib/babe/babe.go +++ b/lib/babe/babe.go @@ -529,7 +529,9 @@ func (b *Service) handleSlot(epoch, slotNum uint64, return err } - rt.SetContextStorage(ts) + nextStorage := ts.Snapshot() + rt.SetContextStorage(nextStorage) + // TODO clear nextStorage block, err := b.buildBlock(parent, currentSlot, rt, authorityIndex, preRuntimeDigest) if err != nil { diff --git a/lib/runtime/storage/trie.go b/lib/runtime/storage/trie.go index e35579603c8..24867ff024d 100644 --- a/lib/runtime/storage/trie.go +++ b/lib/runtime/storage/trie.go @@ -36,11 +36,10 @@ func (s *TrieState) Trie() *trie.Trie { return s.t } -// Snapshot creates a new "version" of the trie. The trie before Snapshot is called -// can no longer be modified, all further changes are on a new "version" of the trie. -// It returns the new version of the trie. -func (s *TrieState) Snapshot() *trie.Trie { - return s.t.Snapshot() +// Snapshot creates a new "version" of the trie underlying the trie state. +// See the trie Snapshot method for more information. +func (s *TrieState) Snapshot() *TrieState { + return NewTrieState(s.t.Snapshot()) } // BeginStorageTransaction begins a new nested storage transaction