From 991c7871635f334e35ad84fa99b86f3b21fd1d81 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Wed, 27 Apr 2022 21:00:18 +0530 Subject: [PATCH 1/7] fix(dot/core): fix the race condition in TrieState fix the race condition in TrieState(nil) and GetRuntime(nil) Fixes #2402 --- dot/core/service.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dot/core/service.go b/dot/core/service.go index bd41a71af5..00fe81556e 100644 --- a/dot/core/service.go +++ b/dot/core/service.go @@ -495,12 +495,14 @@ func (s *Service) HandleSubmittedExtrinsic(ext types.Extrinsic) error { return nil } - ts, err := s.storageState.TrieState(nil) + bestBlockHash := s.blockState.BestBlockHash() + + ts, err := s.storageState.TrieState(&bestBlockHash) if err != nil { return err } - rt, err := s.blockState.GetRuntime(nil) + rt, err := s.blockState.GetRuntime(&bestBlockHash) if err != nil { logger.Critical("failed to get runtime") return err From 6526908ebe605eb377740d32c423dcf5ac6d1ed4 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Thu, 28 Apr 2022 17:57:26 +0530 Subject: [PATCH 2/7] fixed tests --- dot/core/service.go | 4 ++++ dot/core/service_test.go | 36 +++++++++++++++++++++++++----------- dot/state/storage.go | 1 + 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/dot/core/service.go b/dot/core/service.go index 00fe81556e..f0486d2288 100644 --- a/dot/core/service.go +++ b/dot/core/service.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "errors" + "fmt" "sync" "github.com/ChainSafe/gossamer/dot/network" @@ -495,6 +496,9 @@ func (s *Service) HandleSubmittedExtrinsic(ext types.Extrinsic) error { return nil } + if s.blockState == nil { + fmt.Println("\nmeh") + } bestBlockHash := s.blockState.BestBlockHash() ts, err := s.storageState.TrieState(&bestBlockHash) diff --git a/dot/core/service_test.go b/dot/core/service_test.go index 3a178bfc83..e10c958a37 100644 --- a/dot/core/service_test.go +++ b/dot/core/service_test.go @@ -1068,10 +1068,13 @@ func TestServiceHandleSubmittedExtrinsic(t *testing.T) { t.Parallel() ctrl := gomock.NewController(t) mockStorageState := NewMockStorageState(ctrl) - mockStorageState.EXPECT().TrieState(nil).Return(nil, errDummyErr) + mockBlockState := NewMockBlockState(ctrl) + mockBlockState.EXPECT().BestBlockHash().Return(common.Hash{}) + mockStorageState.EXPECT().TrieState(&common.Hash{}).Return(nil, errDummyErr) mockTxnState := NewMockTransactionState(ctrl) mockTxnState.EXPECT().Exists(nil) service := &Service{ + blockState: mockBlockState, storageState: mockStorageState, transactionState: mockTxnState, net: NewMockNetwork(ctrl), @@ -1082,10 +1085,14 @@ func TestServiceHandleSubmittedExtrinsic(t *testing.T) { t.Run("get runtime err", func(t *testing.T) { t.Parallel() ctrl := gomock.NewController(t) - mockStorageState := NewMockStorageState(ctrl) - mockStorageState.EXPECT().TrieState(nil).Return(&rtstorage.TrieState{}, nil) + mockBlockState := NewMockBlockState(ctrl) - mockBlockState.EXPECT().GetRuntime(nil).Return(nil, errDummyErr) + mockBlockState.EXPECT().BestBlockHash().Return(common.Hash{}) + mockBlockState.EXPECT().GetRuntime(&common.Hash{}).Return(nil, errDummyErr) + + mockStorageState := NewMockStorageState(ctrl) + mockStorageState.EXPECT().TrieState(&common.Hash{}).Return(&rtstorage.TrieState{}, nil) + mockTxnState := NewMockTransactionState(ctrl) mockTxnState.EXPECT().Exists(nil).MaxTimes(2) service := &Service{ @@ -1100,13 +1107,16 @@ func TestServiceHandleSubmittedExtrinsic(t *testing.T) { t.Run("validate txn err", func(t *testing.T) { t.Parallel() ctrl := gomock.NewController(t) + mockBlockState := NewMockBlockState(ctrl) + mockBlockState.EXPECT().BestBlockHash().Return(common.Hash{}) + runtimeMockErr := new(mocksruntime.Instance) + mockBlockState.EXPECT().GetRuntime(&common.Hash{}).Return(runtimeMockErr, nil).MaxTimes(2) + mockStorageState := NewMockStorageState(ctrl) - mockStorageState.EXPECT().TrieState(nil).Return(&rtstorage.TrieState{}, nil) + mockStorageState.EXPECT().TrieState(&common.Hash{}).Return(&rtstorage.TrieState{}, nil) mockTxnState := NewMockTransactionState(ctrl) mockTxnState.EXPECT().Exists(types.Extrinsic{}) - runtimeMockErr := new(mocksruntime.Instance) - mockBlockState := NewMockBlockState(ctrl) - mockBlockState.EXPECT().GetRuntime(nil).Return(runtimeMockErr, nil).MaxTimes(2) + runtimeMockErr.On("SetContextStorage", &rtstorage.TrieState{}) runtimeMockErr.On("ValidateTransaction", externalExt).Return(nil, errDummyErr) service := &Service{ @@ -1121,14 +1131,18 @@ func TestServiceHandleSubmittedExtrinsic(t *testing.T) { t.Run("happy path", func(t *testing.T) { t.Parallel() ctrl := gomock.NewController(t) - mockStorageState := NewMockStorageState(ctrl) - mockStorageState.EXPECT().TrieState(nil).Return(&rtstorage.TrieState{}, nil) + runtimeMock := new(mocksruntime.Instance) mockBlockState := NewMockBlockState(ctrl) - mockBlockState.EXPECT().GetRuntime(nil).Return(runtimeMock, nil).MaxTimes(2) + mockBlockState.EXPECT().BestBlockHash().Return(common.Hash{}) + mockBlockState.EXPECT().GetRuntime(&common.Hash{}).Return(runtimeMock, nil).MaxTimes(2) runtimeMock.On("SetContextStorage", &rtstorage.TrieState{}) runtimeMock.On("ValidateTransaction", externalExt). Return(&transaction.Validity{Propagate: true}, nil) + + mockStorageState := NewMockStorageState(ctrl) + mockStorageState.EXPECT().TrieState(&common.Hash{}).Return(&rtstorage.TrieState{}, nil) + mockTxnState := NewMockTransactionState(ctrl) mockTxnState.EXPECT().Exists(types.Extrinsic{}).MaxTimes(2) mockTxnState.EXPECT().AddToPool(transaction.NewValidTransaction(ext, &transaction.Validity{Propagate: true})) diff --git a/dot/state/storage.go b/dot/state/storage.go index b4781b204c..2daf6b3215 100644 --- a/dot/state/storage.go +++ b/dot/state/storage.go @@ -114,6 +114,7 @@ func (s *StorageState) TrieState(root *common.Hash) (*rtstorage.TrieState, error return nil, err } root = &sr + fmt.Printf("root: %s\n", root.String()) } t := s.tries.get(*root) From aae04ae50633adf47b501365ef96a077ef884e65 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Thu, 28 Apr 2022 17:58:49 +0530 Subject: [PATCH 3/7] clean up --- dot/core/service.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/dot/core/service.go b/dot/core/service.go index f0486d2288..00fe81556e 100644 --- a/dot/core/service.go +++ b/dot/core/service.go @@ -7,7 +7,6 @@ import ( "bytes" "context" "errors" - "fmt" "sync" "github.com/ChainSafe/gossamer/dot/network" @@ -496,9 +495,6 @@ func (s *Service) HandleSubmittedExtrinsic(ext types.Extrinsic) error { return nil } - if s.blockState == nil { - fmt.Println("\nmeh") - } bestBlockHash := s.blockState.BestBlockHash() ts, err := s.storageState.TrieState(&bestBlockHash) From 7fcffe48ce40cce844f04a4546678d8f09e61f17 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Thu, 28 Apr 2022 18:00:06 +0530 Subject: [PATCH 4/7] little more clean up --- dot/state/storage.go | 1 - 1 file changed, 1 deletion(-) diff --git a/dot/state/storage.go b/dot/state/storage.go index 2daf6b3215..b4781b204c 100644 --- a/dot/state/storage.go +++ b/dot/state/storage.go @@ -114,7 +114,6 @@ func (s *StorageState) TrieState(root *common.Hash) (*rtstorage.TrieState, error return nil, err } root = &sr - fmt.Printf("root: %s\n", root.String()) } t := s.tries.get(*root) From df4c69c7af90244690664c78a8bd4e193794bea3 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Tue, 3 May 2022 11:22:28 +0530 Subject: [PATCH 5/7] temp --- dot/rpc/websocket_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dot/rpc/websocket_test.go b/dot/rpc/websocket_test.go index 9a0f1f12a4..0aceefb8a9 100644 --- a/dot/rpc/websocket_test.go +++ b/dot/rpc/websocket_test.go @@ -110,6 +110,6 @@ func TestHTTPServer_ServeHTTP(t *testing.T) { _, message, err := c.ReadMessage() require.NoError(t, err) - require.Equal(t, item.expected, message) + require.Equal(t, string(item.expected), string(message)) } } From 8046eb0f6c473e35289f7c6e26842745dfe59971 Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Tue, 3 May 2022 15:57:48 +0530 Subject: [PATCH 6/7] fixing a test --- dot/core/service.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/dot/core/service.go b/dot/core/service.go index 00fe81556e..b26c0ae71f 100644 --- a/dot/core/service.go +++ b/dot/core/service.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "errors" + "fmt" "sync" "github.com/ChainSafe/gossamer/dot/network" @@ -497,9 +498,14 @@ func (s *Service) HandleSubmittedExtrinsic(ext types.Extrinsic) error { bestBlockHash := s.blockState.BestBlockHash() - ts, err := s.storageState.TrieState(&bestBlockHash) + stateRoot, err := s.storageState.GetStateRootFromBlock(&bestBlockHash) if err != nil { - return err + return fmt.Errorf("could not get state root from block %s: %w", bestBlockHash, err) + } + + ts, err := s.storageState.TrieState(stateRoot) + if err != nil { + return fmt.Errorf("could not get trie state: %w", err) } rt, err := s.blockState.GetRuntime(&bestBlockHash) From 6ff8bd0915d6149f344169a984b76df27a0183aa Mon Sep 17 00:00:00 2001 From: Kishan Sagathiya Date: Tue, 3 May 2022 16:57:14 +0530 Subject: [PATCH 7/7] fixing more tests --- dot/core/service.go | 2 +- dot/core/service_test.go | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/dot/core/service.go b/dot/core/service.go index b26c0ae71f..e4bc6d846a 100644 --- a/dot/core/service.go +++ b/dot/core/service.go @@ -505,7 +505,7 @@ func (s *Service) HandleSubmittedExtrinsic(ext types.Extrinsic) error { ts, err := s.storageState.TrieState(stateRoot) if err != nil { - return fmt.Errorf("could not get trie state: %w", err) + return err } rt, err := s.blockState.GetRuntime(&bestBlockHash) diff --git a/dot/core/service_test.go b/dot/core/service_test.go index e10c958a37..8f2b847fcd 100644 --- a/dot/core/service_test.go +++ b/dot/core/service_test.go @@ -1068,9 +1068,11 @@ func TestServiceHandleSubmittedExtrinsic(t *testing.T) { t.Parallel() ctrl := gomock.NewController(t) mockStorageState := NewMockStorageState(ctrl) + mockStorageState.EXPECT().TrieState(&common.Hash{}).Return(nil, errDummyErr) + mockStorageState.EXPECT().GetStateRootFromBlock(&common.Hash{}).Return(&common.Hash{}, nil) + mockBlockState := NewMockBlockState(ctrl) mockBlockState.EXPECT().BestBlockHash().Return(common.Hash{}) - mockStorageState.EXPECT().TrieState(&common.Hash{}).Return(nil, errDummyErr) mockTxnState := NewMockTransactionState(ctrl) mockTxnState.EXPECT().Exists(nil) service := &Service{ @@ -1092,6 +1094,7 @@ func TestServiceHandleSubmittedExtrinsic(t *testing.T) { mockStorageState := NewMockStorageState(ctrl) mockStorageState.EXPECT().TrieState(&common.Hash{}).Return(&rtstorage.TrieState{}, nil) + mockStorageState.EXPECT().GetStateRootFromBlock(&common.Hash{}).Return(&common.Hash{}, nil) mockTxnState := NewMockTransactionState(ctrl) mockTxnState.EXPECT().Exists(nil).MaxTimes(2) @@ -1114,6 +1117,8 @@ func TestServiceHandleSubmittedExtrinsic(t *testing.T) { mockStorageState := NewMockStorageState(ctrl) mockStorageState.EXPECT().TrieState(&common.Hash{}).Return(&rtstorage.TrieState{}, nil) + mockStorageState.EXPECT().GetStateRootFromBlock(&common.Hash{}).Return(&common.Hash{}, nil) + mockTxnState := NewMockTransactionState(ctrl) mockTxnState.EXPECT().Exists(types.Extrinsic{}) @@ -1142,6 +1147,7 @@ func TestServiceHandleSubmittedExtrinsic(t *testing.T) { mockStorageState := NewMockStorageState(ctrl) mockStorageState.EXPECT().TrieState(&common.Hash{}).Return(&rtstorage.TrieState{}, nil) + mockStorageState.EXPECT().GetStateRootFromBlock(&common.Hash{}).Return(&common.Hash{}, nil) mockTxnState := NewMockTransactionState(ctrl) mockTxnState.EXPECT().Exists(types.Extrinsic{}).MaxTimes(2)