diff --git a/beacon-chain/blockchain/BUILD.bazel b/beacon-chain/blockchain/BUILD.bazel index be7bd5e925..954192e719 100644 --- a/beacon-chain/blockchain/BUILD.bazel +++ b/beacon-chain/blockchain/BUILD.bazel @@ -38,8 +38,16 @@ go_library( ], ) -go_test( +test_suite( name = "go_default_test", + tests = [ + ":go_raceoff_test", + ":go_raceon_test", + ], +) + +go_test( + name = "go_raceoff_test", size = "medium", srcs = [ "chain_info_test.go", @@ -73,3 +81,38 @@ go_test( "@org_golang_x_net//context:go_default_library", ], ) + +go_test( + name = "go_raceon_test", + srcs = [ + "chain_info_norace_test.go", + "service_norace_test.go", + ], + embed = [":go_default_library"], + race = "on", + tags = ["race_on"], + deps = [ + "//beacon-chain/cache/depositcache:go_default_library", + "//beacon-chain/core/blocks:go_default_library", + "//beacon-chain/core/helpers:go_default_library", + "//beacon-chain/core/state:go_default_library", + "//beacon-chain/db:go_default_library", + "//beacon-chain/db/testing:go_default_library", + "//beacon-chain/p2p:go_default_library", + "//beacon-chain/powchain:go_default_library", + "//proto/beacon/p2p/v1:go_default_library", + "//proto/eth/v1alpha1:go_default_library", + "//shared/bytesutil:go_default_library", + "//shared/event:go_default_library", + "//shared/params:go_default_library", + "//shared/testutil:go_default_library", + "@com_github_ethereum_go_ethereum//:go_default_library", + "@com_github_ethereum_go_ethereum//common:go_default_library", + "@com_github_ethereum_go_ethereum//core/types:go_default_library", + "@com_github_gogo_protobuf//proto:go_default_library", + "@com_github_prysmaticlabs_go_ssz//:go_default_library", + "@com_github_sirupsen_logrus//:go_default_library", + "@com_github_sirupsen_logrus//hooks/test:go_default_library", + "@org_golang_x_net//context:go_default_library", + ], +) diff --git a/beacon-chain/blockchain/chain_info.go b/beacon-chain/blockchain/chain_info.go index c8370cac5d..c279164c5c 100644 --- a/beacon-chain/blockchain/chain_info.go +++ b/beacon-chain/blockchain/chain_info.go @@ -55,13 +55,16 @@ func (s *Service) FinalizedCheckpt() *ethpb.Checkpoint { // HeadSlot returns the slot of the head of the chain. func (s *Service) HeadSlot() uint64 { + s.headLock.RLock() + defer s.headLock.RUnlock() + return s.headSlot } // HeadRoot returns the root of the head of the chain. func (s *Service) HeadRoot() []byte { - s.canonicalRootsLock.RLock() - defer s.canonicalRootsLock.RUnlock() + s.headLock.RLock() + defer s.headLock.RUnlock() root := s.canonicalRoots[s.headSlot] if len(root) != 0 { @@ -73,18 +76,24 @@ func (s *Service) HeadRoot() []byte { // HeadBlock returns the head block of the chain. func (s *Service) HeadBlock() *ethpb.BeaconBlock { + s.headLock.RLock() + defer s.headLock.RUnlock() + return proto.Clone(s.headBlock).(*ethpb.BeaconBlock) } // HeadState returns the head state of the chain. func (s *Service) HeadState() *pb.BeaconState { + s.headLock.RLock() + defer s.headLock.RUnlock() + return proto.Clone(s.headState).(*pb.BeaconState) } // CanonicalRoot returns the canonical root of a given slot. func (s *Service) CanonicalRoot(slot uint64) []byte { - s.canonicalRootsLock.RLock() - defer s.canonicalRootsLock.RUnlock() + s.headLock.RLock() + defer s.headLock.RUnlock() return s.canonicalRoots[slot] } diff --git a/beacon-chain/blockchain/chain_info_norace_test.go b/beacon-chain/blockchain/chain_info_norace_test.go new file mode 100644 index 0000000000..fd971c5561 --- /dev/null +++ b/beacon-chain/blockchain/chain_info_norace_test.go @@ -0,0 +1,77 @@ +package blockchain + +import ( + "context" + "testing" + + testDB "github.com/prysmaticlabs/prysm/beacon-chain/db/testing" + ethpb "github.com/prysmaticlabs/prysm/proto/eth/v1alpha1" +) + +func TestHeadSlot_DataRace(t *testing.T) { + db := testDB.SetupDB(t) + defer testDB.TeardownDB(t, db) + s := &Service{ + beaconDB: db, + canonicalRoots: make(map[uint64][]byte), + } + go func() { + s.saveHead( + context.Background(), + ðpb.BeaconBlock{Slot: 777}, + [32]byte{}, + ) + }() + s.HeadSlot() +} + +func TestHeadRoot_DataRace(t *testing.T) { + db := testDB.SetupDB(t) + defer testDB.TeardownDB(t, db) + s := &Service{ + beaconDB: db, + canonicalRoots: make(map[uint64][]byte), + } + go func() { + s.saveHead( + context.Background(), + ðpb.BeaconBlock{Slot: 777}, + [32]byte{}, + ) + }() + s.HeadRoot() +} + +func TestHeadBlock_DataRace(t *testing.T) { + db := testDB.SetupDB(t) + defer testDB.TeardownDB(t, db) + s := &Service{ + beaconDB: db, + canonicalRoots: make(map[uint64][]byte), + } + go func() { + s.saveHead( + context.Background(), + ðpb.BeaconBlock{Slot: 777}, + [32]byte{}, + ) + }() + s.HeadBlock() +} + +func TestHeadState_DataRace(t *testing.T) { + db := testDB.SetupDB(t) + defer testDB.TeardownDB(t, db) + s := &Service{ + beaconDB: db, + canonicalRoots: make(map[uint64][]byte), + } + go func() { + s.saveHead( + context.Background(), + ðpb.BeaconBlock{Slot: 777}, + [32]byte{}, + ) + }() + s.HeadState() +} diff --git a/beacon-chain/blockchain/receive_attestation.go b/beacon-chain/blockchain/receive_attestation.go index dbbe010326..31bde10780 100644 --- a/beacon-chain/blockchain/receive_attestation.go +++ b/beacon-chain/blockchain/receive_attestation.go @@ -6,7 +6,7 @@ import ( "encoding/hex" "github.com/pkg/errors" - "github.com/prysmaticlabs/go-ssz" + ssz "github.com/prysmaticlabs/go-ssz" "github.com/prysmaticlabs/prysm/beacon-chain/core/helpers" ethpb "github.com/prysmaticlabs/prysm/proto/eth/v1alpha1" "github.com/prysmaticlabs/prysm/shared/bytesutil" @@ -89,6 +89,8 @@ func (s *Service) ReceiveAttestationNoPubsub(ctx context.Context, att *ethpb.Att // Skip checking for competing attestation's target roots at epoch boundary. if !helpers.IsEpochStart(attSlot) { + s.headLock.RLock() + defer s.headLock.RUnlock() targetRoot, err := helpers.BlockRoot(s.headState, att.Data.Target.Epoch) if err != nil { return errors.Wrapf(err, "could not get target root for epoch %d", att.Data.Target.Epoch) diff --git a/beacon-chain/blockchain/service.go b/beacon-chain/blockchain/service.go index a0be5796a4..f687c7e438 100644 --- a/beacon-chain/blockchain/service.go +++ b/beacon-chain/blockchain/service.go @@ -12,7 +12,7 @@ import ( "time" "github.com/pkg/errors" - "github.com/prysmaticlabs/go-ssz" + ssz "github.com/prysmaticlabs/go-ssz" "github.com/prysmaticlabs/prysm/beacon-chain/blockchain/forkchoice" "github.com/prysmaticlabs/prysm/beacon-chain/cache/depositcache" "github.com/prysmaticlabs/prysm/beacon-chain/core/blocks" @@ -61,7 +61,7 @@ type Service struct { headBlock *ethpb.BeaconBlock headState *pb.BeaconState canonicalRoots map[uint64][]byte - canonicalRootsLock sync.RWMutex + headLock sync.RWMutex } // Config options for the service. @@ -206,11 +206,12 @@ func (s *Service) HeadUpdatedFeed() *event.Feed { // This gets called to update canonical root mapping. func (s *Service) saveHead(ctx context.Context, b *ethpb.BeaconBlock, r [32]byte) error { + s.headLock.Lock() + defer s.headLock.Unlock() + s.headSlot = b.Slot - s.canonicalRootsLock.Lock() s.canonicalRoots[b.Slot] = r[:] - defer s.canonicalRootsLock.Unlock() if err := s.beaconDB.SaveHeadBlockRoot(ctx, r); err != nil { return errors.Wrap(err, "could not save head root in DB") @@ -242,6 +243,9 @@ func (s *Service) saveGenesisValidators(ctx context.Context, state *pb.BeaconSta // This gets called when beacon chain is first initialized to save genesis data (state, block, and more) in db func (s *Service) saveGenesisData(ctx context.Context, genesisState *pb.BeaconState) error { + s.headLock.Lock() + defer s.headLock.Unlock() + stateRoot, err := ssz.HashTreeRoot(genesisState) if err != nil { return errors.Wrap(err, "could not tree hash genesis state") @@ -286,6 +290,9 @@ func (s *Service) saveGenesisData(ctx context.Context, genesisState *pb.BeaconSt // This gets called to initialize chain info variables using the head stored in DB func (s *Service) initializeChainInfo(ctx context.Context) error { + s.headLock.Lock() + defer s.headLock.Unlock() + headBlock, err := s.beaconDB.HeadBlock(ctx) if err != nil { return errors.Wrap(err, "could not get head block in db") diff --git a/beacon-chain/blockchain/service_norace_test.go b/beacon-chain/blockchain/service_norace_test.go new file mode 100644 index 0000000000..062ae72ee8 --- /dev/null +++ b/beacon-chain/blockchain/service_norace_test.go @@ -0,0 +1,37 @@ +package blockchain + +import ( + "context" + "io/ioutil" + "testing" + + testDB "github.com/prysmaticlabs/prysm/beacon-chain/db/testing" + ethpb "github.com/prysmaticlabs/prysm/proto/eth/v1alpha1" + "github.com/sirupsen/logrus" +) + +func init() { + logrus.SetLevel(logrus.DebugLevel) + logrus.SetOutput(ioutil.Discard) +} + +func TestChainService_SaveHead_DataRace(t *testing.T) { + db := testDB.SetupDB(t) + defer testDB.TeardownDB(t, db) + s := &Service{ + beaconDB: db, + canonicalRoots: make(map[uint64][]byte), + } + go func() { + s.saveHead( + context.Background(), + ðpb.BeaconBlock{Slot: 777}, + [32]byte{}, + ) + }() + s.saveHead( + context.Background(), + ðpb.BeaconBlock{Slot: 888}, + [32]byte{}, + ) +} diff --git a/beacon-chain/blockchain/service_test.go b/beacon-chain/blockchain/service_test.go index ed82acbb64..8cb5310a9a 100644 --- a/beacon-chain/blockchain/service_test.go +++ b/beacon-chain/blockchain/service_test.go @@ -11,11 +11,11 @@ import ( "testing" "time" - "github.com/ethereum/go-ethereum" + ethereum "github.com/ethereum/go-ethereum" "github.com/ethereum/go-ethereum/common" gethTypes "github.com/ethereum/go-ethereum/core/types" "github.com/gogo/protobuf/proto" - "github.com/prysmaticlabs/go-ssz" + ssz "github.com/prysmaticlabs/go-ssz" "github.com/prysmaticlabs/prysm/beacon-chain/cache/depositcache" b "github.com/prysmaticlabs/prysm/beacon-chain/core/blocks" "github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"