From 7bb0ee78af10e86358d147ebc5d3efdd2929c95d Mon Sep 17 00:00:00 2001 From: Jim McDonald Date: Mon, 6 Jul 2020 21:52:53 +0100 Subject: [PATCH] Update beacon state locks (#6326) * Rationalise state locking * Rationalise state locking * Merge branch 'state-locks' of github.com:mcdee/prysm into state-locks * Merge branch 'master' into state-locks * Add feature flag * Merge * Merge branch 'master' into state-locks * Merge branch 'master' into state-locks * Update locks * Merge branch 'master' into state-locks * Gazelle * Tidy-ups * Merge branch 'master' into state-locks * Remove commentary to a docs.go file for better presentation on godocs * Add newBeaconStateLocks as a --dev flag * gofmt * Merge branch 'master' into state-locks --- beacon-chain/state/BUILD.bazel | 2 + beacon-chain/state/docs.go | 40 +++ beacon-chain/state/getters.go | 419 ++++++++++++++++++++++++++++++- beacon-chain/state/setters.go | 129 ++++------ beacon-chain/state/state_trie.go | 119 ++++++--- beacon-chain/state/types.go | 3 - shared/featureconfig/config.go | 6 + shared/featureconfig/flags.go | 6 + 8 files changed, 592 insertions(+), 132 deletions(-) create mode 100644 beacon-chain/state/docs.go diff --git a/beacon-chain/state/BUILD.bazel b/beacon-chain/state/BUILD.bazel index a8d1fca1eb..c94e98bdb9 100644 --- a/beacon-chain/state/BUILD.bazel +++ b/beacon-chain/state/BUILD.bazel @@ -5,6 +5,7 @@ go_library( name = "go_default_library", srcs = [ "cloners.go", + "docs.go", "field_trie.go", "getters.go", "setters.go", @@ -27,6 +28,7 @@ go_library( "//beacon-chain/state/stateutil:go_default_library", "//proto/beacon/p2p/v1:go_default_library", "//shared/bytesutil:go_default_library", + "//shared/featureconfig:go_default_library", "//shared/hashutil:go_default_library", "//shared/htrutils:go_default_library", "//shared/params:go_default_library", diff --git a/beacon-chain/state/docs.go b/beacon-chain/state/docs.go new file mode 100644 index 0000000000..6fa18b6081 --- /dev/null +++ b/beacon-chain/state/docs.go @@ -0,0 +1,40 @@ +// Package state defines how the beacon chain state for eth2 +// functions in the running beacon node, using an advanced, +// immutable implementation of the state data structure. +// +// BeaconState getters may be accessed from inside or outside the package. To +// avoid duplicating locks, we have internal and external versions of the +// getter The external function carries out the short-circuit conditions, +// obtains a read lock, then calls the internal function. The internal function +// carries out the short-circuit conditions and returns the required data +// without further locking, allowing it to be used by other package-level +// functions that already hold a lock. Hence the functions look something +// like this: +// +// func (b *BeaconState) Foo() uint64 { +// // Short-circuit conditions. +// if !b.HasInnerState() { +// return 0 +// } +// +// // Read lock. +// b.lock.RLock() +// defer b.lock.RUnlock() +// +// // Internal getter. +// return b.foo() +// } +// +// func (b *BeaconState) foo() uint64 { +// // Short-circuit conditions. +// if !b.HasInnerState() { +// return 0 +// } +// +// return b.state.foo +// } +// +// Although it is technically possible to remove the short-circuit conditions +// from the external function, that would require every read to obtain a lock +// even if the data was not present, leading to potential slowdowns. +package state diff --git a/beacon-chain/state/getters.go b/beacon-chain/state/getters.go index 979c15523f..52e2888248 100644 --- a/beacon-chain/state/getters.go +++ b/beacon-chain/state/getters.go @@ -9,6 +9,7 @@ import ( "github.com/prysmaticlabs/go-bitfield" pbp2p "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" "github.com/prysmaticlabs/prysm/shared/bytesutil" + "github.com/prysmaticlabs/prysm/shared/featureconfig" "github.com/prysmaticlabs/prysm/shared/params" ) @@ -106,6 +107,34 @@ func (b *BeaconState) CloneInnerState() *pbp2p.BeaconState { if b == nil || b.state == nil { return nil } + + if featureconfig.Get().NewBeaconStateLocks { + b.lock.RLock() + defer b.lock.RUnlock() + return &pbp2p.BeaconState{ + GenesisTime: b.genesisTime(), + GenesisValidatorsRoot: b.genesisValidatorRoot(), + Slot: b.slot(), + Fork: b.fork(), + LatestBlockHeader: b.latestBlockHeader(), + BlockRoots: b.blockRoots(), + StateRoots: b.stateRoots(), + HistoricalRoots: b.historicalRoots(), + Eth1Data: b.eth1Data(), + Eth1DataVotes: b.eth1DataVotes(), + Eth1DepositIndex: b.eth1DepositIndex(), + Validators: b.validators(), + Balances: b.balances(), + RandaoMixes: b.randaoMixes(), + Slashings: b.slashings(), + PreviousEpochAttestations: b.previousEpochAttestations(), + CurrentEpochAttestations: b.currentEpochAttestations(), + JustificationBits: b.justificationBits(), + PreviousJustifiedCheckpoint: b.previousJustifiedCheckpoint(), + CurrentJustifiedCheckpoint: b.currentJustifiedCheckpoint(), + FinalizedCheckpoint: b.finalizedCheckpoint(), + } + } return &pbp2p.BeaconState{ GenesisTime: b.GenesisTime(), GenesisValidatorsRoot: b.GenesisValidatorRoot(), @@ -142,6 +171,20 @@ func (b *BeaconState) GenesisTime() uint64 { if !b.HasInnerState() { return 0 } + + b.lock.RLock() + defer b.lock.RUnlock() + + return b.genesisTime() +} + +// genesisTime of the beacon state as a uint64. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) genesisTime() uint64 { + if !b.HasInnerState() { + return 0 + } + return b.state.GenesisTime } @@ -150,7 +193,22 @@ func (b *BeaconState) GenesisValidatorRoot() []byte { if !b.HasInnerState() { return nil } + if b.state.GenesisValidatorsRoot == nil { + return params.BeaconConfig().ZeroHash[:] + } + b.lock.RLock() + defer b.lock.RUnlock() + + return b.genesisValidatorRoot() +} + +// genesisValidatorRoot of the beacon state. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) genesisValidatorRoot() []byte { + if !b.HasInnerState() { + return nil + } if b.state.GenesisValidatorsRoot == nil { return params.BeaconConfig().ZeroHash[:] } @@ -165,6 +223,20 @@ func (b *BeaconState) GenesisUnixTime() time.Time { if !b.HasInnerState() { return time.Unix(0, 0) } + + b.lock.RLock() + defer b.lock.RUnlock() + + return b.genesisUnixTime() +} + +// genesisUnixTime returns the genesis time as time.Time. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) genesisUnixTime() time.Time { + if !b.HasInnerState() { + return time.Unix(0, 0) + } + return time.Unix(int64(b.state.GenesisTime), 0) } @@ -173,9 +245,20 @@ func (b *BeaconState) Slot() uint64 { if !b.HasInnerState() { return 0 } + b.lock.RLock() defer b.lock.RUnlock() + return b.slot() +} + +// slot of the current beacon chain state. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) slot() uint64 { + if !b.HasInnerState() { + return 0 + } + return b.state.Slot } @@ -191,6 +274,19 @@ func (b *BeaconState) Fork() *pbp2p.Fork { b.lock.RLock() defer b.lock.RUnlock() + return b.fork() +} + +// fork version of the beacon chain. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) fork() *pbp2p.Fork { + if !b.HasInnerState() { + return nil + } + if b.state.Fork == nil { + return nil + } + prevVersion := make([]byte, len(b.state.Fork.PreviousVersion)) copy(prevVersion, b.state.Fork.PreviousVersion) currVersion := make([]byte, len(b.state.Fork.CurrentVersion)) @@ -214,6 +310,19 @@ func (b *BeaconState) LatestBlockHeader() *ethpb.BeaconBlockHeader { b.lock.RLock() defer b.lock.RUnlock() + return b.latestBlockHeader() +} + +// latestBlockHeader stored within the beacon state. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) latestBlockHeader() *ethpb.BeaconBlockHeader { + if !b.HasInnerState() { + return nil + } + if b.state.LatestBlockHeader == nil { + return nil + } + hdr := ðpb.BeaconBlockHeader{ Slot: b.state.LatestBlockHeader.Slot, ProposerIndex: b.state.LatestBlockHeader.ProposerIndex, @@ -237,9 +346,20 @@ func (b *BeaconState) ParentRoot() [32]byte { if !b.HasInnerState() { return [32]byte{} } + b.lock.RLock() defer b.lock.RUnlock() + return b.parentRoot() +} + +// parentRoot is a convenience method to access state.LatestBlockRoot.ParentRoot. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) parentRoot() [32]byte { + if !b.HasInnerState() { + return [32]byte{} + } + parentRoot := [32]byte{} copy(parentRoot[:], b.state.LatestBlockHeader.ParentRoot) return parentRoot @@ -250,12 +370,26 @@ func (b *BeaconState) BlockRoots() [][]byte { if !b.HasInnerState() { return nil } - b.lock.RLock() - defer b.lock.RUnlock() - if b.state.BlockRoots == nil { return nil } + + b.lock.RLock() + defer b.lock.RUnlock() + + return b.blockRoots() +} + +// blockRoots kept track of in the beacon state. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) blockRoots() [][]byte { + if !b.HasInnerState() { + return nil + } + if b.state.BlockRoots == nil { + return nil + } + roots := make([][]byte, len(b.state.BlockRoots)) for i, r := range b.state.BlockRoots { tmpRt := make([]byte, len(r)) @@ -278,6 +412,20 @@ func (b *BeaconState) BlockRootAtIndex(idx uint64) ([]byte, error) { b.lock.RLock() defer b.lock.RUnlock() + return b.blockRootAtIndex(idx) +} + +// blockRootAtIndex retrieves a specific block root based on an +// input index value. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) blockRootAtIndex(idx uint64) ([]byte, error) { + if !b.HasInnerState() { + return nil, ErrNilInnerState + } + if b.state.BlockRoots == nil { + return nil, nil + } + if uint64(len(b.state.BlockRoots)) <= idx { return nil, fmt.Errorf("index %d out of range", idx) } @@ -298,6 +446,19 @@ func (b *BeaconState) StateRoots() [][]byte { b.lock.RLock() defer b.lock.RUnlock() + return b.stateRoots() +} + +// StateRoots kept track of in the beacon state. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) stateRoots() [][]byte { + if !b.HasInnerState() { + return nil + } + if b.state.StateRoots == nil { + return nil + } + roots := make([][]byte, len(b.state.StateRoots)) for i, r := range b.state.StateRoots { tmpRt := make([]byte, len(r)) @@ -319,6 +480,19 @@ func (b *BeaconState) HistoricalRoots() [][]byte { b.lock.RLock() defer b.lock.RUnlock() + return b.historicalRoots() +} + +// historicalRoots based on epochs stored in the beacon state. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) historicalRoots() [][]byte { + if !b.HasInnerState() { + return nil + } + if b.state.HistoricalRoots == nil { + return nil + } + roots := make([][]byte, len(b.state.HistoricalRoots)) for i, r := range b.state.HistoricalRoots { tmpRt := make([]byte, len(r)) @@ -333,10 +507,22 @@ func (b *BeaconState) Eth1Data() *ethpb.Eth1Data { if !b.HasInnerState() { return nil } + if b.state.Eth1Data == nil { + return nil + } b.lock.RLock() defer b.lock.RUnlock() + return b.eth1Data() +} + +// eth1Data corresponding to the proof-of-work chain information stored in the beacon state. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) eth1Data() *ethpb.Eth1Data { + if !b.HasInnerState() { + return nil + } if b.state.Eth1Data == nil { return nil } @@ -350,10 +536,23 @@ func (b *BeaconState) Eth1DataVotes() []*ethpb.Eth1Data { if !b.HasInnerState() { return nil } + if b.state.Eth1DataVotes == nil { + return nil + } b.lock.RLock() defer b.lock.RUnlock() + return b.eth1DataVotes() +} + +// eth1DataVotes corresponds to votes from eth2 on the canonical proof-of-work chain +// data retrieved from eth1. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) eth1DataVotes() []*ethpb.Eth1Data { + if !b.HasInnerState() { + return nil + } if b.state.Eth1DataVotes == nil { return nil } @@ -375,6 +574,17 @@ func (b *BeaconState) Eth1DepositIndex() uint64 { b.lock.RLock() defer b.lock.RUnlock() + return b.eth1DepositIndex() +} + +// eth1DepositIndex corresponds to the index of the deposit made to the +// validator deposit contract at the time of this state's eth1 data. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) eth1DepositIndex() uint64 { + if !b.HasInnerState() { + return 0 + } + return b.state.Eth1DepositIndex } @@ -390,6 +600,19 @@ func (b *BeaconState) Validators() []*ethpb.Validator { b.lock.RLock() defer b.lock.RUnlock() + return b.validators() +} + +// validators participating in consensus on the beacon chain. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) validators() []*ethpb.Validator { + if !b.HasInnerState() { + return nil + } + if b.state.Validators == nil { + return nil + } + res := make([]*ethpb.Validator, len(b.state.Validators)) for i := 0; i < len(res); i++ { val := b.state.Validators[i] @@ -427,17 +650,16 @@ func (b *BeaconState) ValidatorAtIndex(idx uint64) (*ethpb.Validator, error) { if !b.HasInnerState() { return nil, ErrNilInnerState } + if b.state.Validators == nil { + return ðpb.Validator{}, nil + } + if uint64(len(b.state.Validators)) <= idx { + return nil, fmt.Errorf("index %d out of range", idx) + } b.lock.RLock() defer b.lock.RUnlock() - if b.state.Validators == nil { - return ðpb.Validator{}, nil - } - - if uint64(len(b.state.Validators)) <= idx { - return nil, fmt.Errorf("index %d out of range", idx) - } val := b.state.Validators[idx] return CopyValidator(val), nil } @@ -523,9 +745,10 @@ func (b *BeaconState) ReadFromEveryValidator(f func(idx int, val *ReadOnlyValida return errors.New("nil validators in state") } b.lock.RLock() - defer b.lock.RUnlock() + validators := b.state.Validators + b.lock.RUnlock() - for i, v := range b.state.Validators { + for i, v := range validators { err := f(i, &ReadOnlyValidator{validator: v}) if err != nil { return err @@ -542,9 +765,23 @@ func (b *BeaconState) Balances() []uint64 { if b.state.Balances == nil { return nil } + b.lock.RLock() defer b.lock.RUnlock() + return b.balances() +} + +// balances of validators participating in consensus on the beacon chain. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) balances() []uint64 { + if !b.HasInnerState() { + return nil + } + if b.state.Balances == nil { + return nil + } + res := make([]uint64, len(b.state.Balances)) copy(res, b.state.Balances) return res @@ -580,6 +817,19 @@ func (b *BeaconState) BalancesLength() int { b.lock.RLock() defer b.lock.RUnlock() + return b.balancesLength() +} + +// balancesLength returns the length of the balances slice. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) balancesLength() int { + if !b.HasInnerState() { + return 0 + } + if b.state.Balances == nil { + return 0 + } + return len(b.state.Balances) } @@ -595,6 +845,19 @@ func (b *BeaconState) RandaoMixes() [][]byte { b.lock.RLock() defer b.lock.RUnlock() + return b.randaoMixes() +} + +// randaoMixes of block proposers on the beacon chain. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) randaoMixes() [][]byte { + if !b.HasInnerState() { + return nil + } + if b.state.RandaoMixes == nil { + return nil + } + mixes := make([][]byte, len(b.state.RandaoMixes)) for i, r := range b.state.RandaoMixes { tmpRt := make([]byte, len(r)) @@ -617,6 +880,20 @@ func (b *BeaconState) RandaoMixAtIndex(idx uint64) ([]byte, error) { b.lock.RLock() defer b.lock.RUnlock() + return b.randaoMixAtIndex(idx) +} + +// randaoMixAtIndex retrieves a specific block root based on an +// input index value. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) randaoMixAtIndex(idx uint64) ([]byte, error) { + if !b.HasInnerState() { + return nil, ErrNilInnerState + } + if b.state.RandaoMixes == nil { + return nil, nil + } + if uint64(len(b.state.RandaoMixes)) <= idx { return nil, fmt.Errorf("index %d out of range", idx) } @@ -637,6 +914,19 @@ func (b *BeaconState) RandaoMixesLength() int { b.lock.RLock() defer b.lock.RUnlock() + return b.randaoMixesLength() +} + +// randaoMixesLength returns the length of the randao mixes slice. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) randaoMixesLength() int { + if !b.HasInnerState() { + return 0 + } + if b.state.RandaoMixes == nil { + return 0 + } + return len(b.state.RandaoMixes) } @@ -652,6 +942,19 @@ func (b *BeaconState) Slashings() []uint64 { b.lock.RLock() defer b.lock.RUnlock() + return b.slashings() +} + +// slashings of validators on the beacon chain. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) slashings() []uint64 { + if !b.HasInnerState() { + return nil + } + if b.state.Slashings == nil { + return nil + } + res := make([]uint64, len(b.state.Slashings)) copy(res, b.state.Slashings) return res @@ -669,6 +972,19 @@ func (b *BeaconState) PreviousEpochAttestations() []*pbp2p.PendingAttestation { b.lock.RLock() defer b.lock.RUnlock() + return b.previousEpochAttestations() +} + +// previousEpochAttestations corresponding to blocks on the beacon chain. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) previousEpochAttestations() []*pbp2p.PendingAttestation { + if !b.HasInnerState() { + return nil + } + if b.state.PreviousEpochAttestations == nil { + return nil + } + res := make([]*pbp2p.PendingAttestation, len(b.state.PreviousEpochAttestations)) for i := 0; i < len(res); i++ { res[i] = CopyPendingAttestation(b.state.PreviousEpochAttestations[i]) @@ -688,6 +1004,19 @@ func (b *BeaconState) CurrentEpochAttestations() []*pbp2p.PendingAttestation { b.lock.RLock() defer b.lock.RUnlock() + return b.currentEpochAttestations() +} + +// currentEpochAttestations corresponding to blocks on the beacon chain. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) currentEpochAttestations() []*pbp2p.PendingAttestation { + if !b.HasInnerState() { + return nil + } + if b.state.CurrentEpochAttestations == nil { + return nil + } + res := make([]*pbp2p.PendingAttestation, len(b.state.CurrentEpochAttestations)) for i := 0; i < len(res); i++ { res[i] = CopyPendingAttestation(b.state.CurrentEpochAttestations[i]) @@ -707,6 +1036,19 @@ func (b *BeaconState) JustificationBits() bitfield.Bitvector4 { b.lock.RLock() defer b.lock.RUnlock() + return b.justificationBits() +} + +// justificationBits marking which epochs have been justified in the beacon chain. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) justificationBits() bitfield.Bitvector4 { + if !b.HasInnerState() { + return nil + } + if b.state.JustificationBits == nil { + return nil + } + res := make([]byte, len(b.state.JustificationBits.Bytes())) copy(res, b.state.JustificationBits.Bytes()) return res @@ -724,6 +1066,19 @@ func (b *BeaconState) PreviousJustifiedCheckpoint() *ethpb.Checkpoint { b.lock.RLock() defer b.lock.RUnlock() + return b.previousJustifiedCheckpoint() +} + +// previousJustifiedCheckpoint denoting an epoch and block root. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) previousJustifiedCheckpoint() *ethpb.Checkpoint { + if !b.HasInnerState() { + return nil + } + if b.state.PreviousJustifiedCheckpoint == nil { + return nil + } + return CopyCheckpoint(b.state.PreviousJustifiedCheckpoint) } @@ -739,6 +1094,19 @@ func (b *BeaconState) CurrentJustifiedCheckpoint() *ethpb.Checkpoint { b.lock.RLock() defer b.lock.RUnlock() + return b.currentJustifiedCheckpoint() +} + +// currentJustifiedCheckpoint denoting an epoch and block root. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) currentJustifiedCheckpoint() *ethpb.Checkpoint { + if !b.HasInnerState() { + return nil + } + if b.state.CurrentJustifiedCheckpoint == nil { + return nil + } + return CopyCheckpoint(b.state.CurrentJustifiedCheckpoint) } @@ -754,6 +1122,19 @@ func (b *BeaconState) FinalizedCheckpoint() *ethpb.Checkpoint { b.lock.RLock() defer b.lock.RUnlock() + return b.finalizedCheckpoint() +} + +// finalizedCheckpoint denoting an epoch and block root. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) finalizedCheckpoint() *ethpb.Checkpoint { + if !b.HasInnerState() { + return nil + } + if b.state.FinalizedCheckpoint == nil { + return nil + } + return CopyCheckpoint(b.state.FinalizedCheckpoint) } @@ -765,8 +1146,22 @@ func (b *BeaconState) FinalizedCheckpointEpoch() uint64 { if b.state.FinalizedCheckpoint == nil { return 0 } + b.lock.RLock() defer b.lock.RUnlock() + return b.finalizedCheckpointEpoch() +} + +// finalizedCheckpointEpoch returns the epoch value of the finalized checkpoint. +// This assumes that a lock is already held on BeaconState. +func (b *BeaconState) finalizedCheckpointEpoch() uint64 { + if !b.HasInnerState() { + return 0 + } + if b.state.FinalizedCheckpoint == nil { + return 0 + } + return b.state.FinalizedCheckpoint.Epoch } diff --git a/beacon-chain/state/setters.go b/beacon-chain/state/setters.go index 1999a75896..bf18118301 100644 --- a/beacon-chain/state/setters.go +++ b/beacon-chain/state/setters.go @@ -122,8 +122,9 @@ func (b *BeaconState) UpdateBlockRootAtIndex(idx uint64, blockRoot [32]byte) err if uint64(len(b.state.BlockRoots)) <= idx { return fmt.Errorf("invalid index provided %d", idx) } + b.lock.Lock() + defer b.lock.Unlock() - b.lock.RLock() r := b.state.BlockRoots if ref := b.sharedFieldReferences[blockRoots]; ref.Refs() > 1 { // Copy elements in underlying array by reference. @@ -132,17 +133,12 @@ func (b *BeaconState) UpdateBlockRootAtIndex(idx uint64, blockRoot [32]byte) err ref.MinusRef() b.sharedFieldReferences[blockRoots] = &reference{refs: 1} } - b.lock.RUnlock() - - // Must secure lock after copy or hit a deadlock. - b.lock.Lock() - defer b.lock.Unlock() r[idx] = blockRoot[:] b.state.BlockRoots = r b.markFieldAsDirty(blockRoots) - b.AddDirtyIndices(blockRoots, []uint64{idx}) + b.addDirtyIndices(blockRoots, []uint64{idx}) return nil } @@ -176,6 +172,10 @@ func (b *BeaconState) UpdateStateRootAtIndex(idx uint64, stateRoot [32]byte) err b.lock.RUnlock() return errors.Errorf("invalid index provided %d", idx) } + b.lock.RUnlock() + + b.lock.Lock() + defer b.lock.Unlock() // Check if we hold the only reference to the shared state roots slice. r := b.state.StateRoots @@ -186,17 +186,12 @@ func (b *BeaconState) UpdateStateRootAtIndex(idx uint64, stateRoot [32]byte) err ref.MinusRef() b.sharedFieldReferences[stateRoots] = &reference{refs: 1} } - b.lock.RUnlock() - - // Must secure lock after copy or hit a deadlock. - b.lock.Lock() - defer b.lock.Unlock() r[idx] = stateRoot[:] b.state.StateRoots = r b.markFieldAsDirty(stateRoots) - b.AddDirtyIndices(stateRoots, []uint64{idx}) + b.addDirtyIndices(stateRoots, []uint64{idx}) return nil } @@ -254,7 +249,9 @@ func (b *BeaconState) AppendEth1DataVotes(val *ethpb.Eth1Data) error { if !b.HasInnerState() { return ErrNilInnerState } - b.lock.RLock() + b.lock.Lock() + defer b.lock.Unlock() + votes := b.state.Eth1DataVotes if b.sharedFieldReferences[eth1DataVotes].Refs() > 1 { // Copy elements in underlying array by reference. @@ -263,14 +260,10 @@ func (b *BeaconState) AppendEth1DataVotes(val *ethpb.Eth1Data) error { b.sharedFieldReferences[eth1DataVotes].MinusRef() b.sharedFieldReferences[eth1DataVotes] = &reference{refs: 1} } - b.lock.RUnlock() - - b.lock.Lock() - defer b.lock.Unlock() b.state.Eth1DataVotes = append(votes, val) b.markFieldAsDirty(eth1DataVotes) - b.AddDirtyIndices(eth1DataVotes, []uint64{uint64(len(b.state.Eth1DataVotes) - 1)}) + b.addDirtyIndices(eth1DataVotes, []uint64{uint64(len(b.state.Eth1DataVotes) - 1)}) return nil } @@ -311,16 +304,16 @@ func (b *BeaconState) ApplyToEveryValidator(f func(idx int, val *ethpb.Validator if !b.HasInnerState() { return ErrNilInnerState } - b.lock.RLock() + b.lock.Lock() v := b.state.Validators if ref := b.sharedFieldReferences[validators]; ref.Refs() > 1 { // Perform a copy since this is a shared reference and we don't want to mutate others. - v = b.Validators() + v = b.validators() ref.MinusRef() b.sharedFieldReferences[validators] = &reference{refs: 1} } - b.lock.RUnlock() + b.lock.Unlock() changedVals := []uint64{} for i, val := range v { changed, err := f(i, val) @@ -337,7 +330,7 @@ func (b *BeaconState) ApplyToEveryValidator(f func(idx int, val *ethpb.Validator b.state.Validators = v b.markFieldAsDirty(validators) - b.AddDirtyIndices(validators, changedVals) + b.addDirtyIndices(validators, changedVals) return nil } @@ -351,25 +344,22 @@ func (b *BeaconState) UpdateValidatorAtIndex(idx uint64, val *ethpb.Validator) e if uint64(len(b.state.Validators)) <= idx { return errors.Errorf("invalid index provided %d", idx) } + b.lock.Lock() + defer b.lock.Unlock() - b.lock.RLock() v := b.state.Validators if ref := b.sharedFieldReferences[validators]; ref.Refs() > 1 { // Perform a copy since this is a shared reference and we don't want to mutate others. - v = b.Validators() + v = b.validators() ref.MinusRef() b.sharedFieldReferences[validators] = &reference{refs: 1} } - b.lock.RUnlock() - - b.lock.Lock() - defer b.lock.Unlock() v[idx] = val b.state.Validators = v b.markFieldAsDirty(validators) - b.AddDirtyIndices(validators, []uint64{idx}) + b.addDirtyIndices(validators, []uint64{idx}) return nil } @@ -413,18 +403,15 @@ func (b *BeaconState) UpdateBalancesAtIndex(idx uint64, val uint64) error { if uint64(len(b.state.Balances)) <= idx { return errors.Errorf("invalid index provided %d", idx) } + b.lock.Lock() + defer b.lock.Unlock() - b.lock.RLock() bals := b.state.Balances if b.sharedFieldReferences[balances].Refs() > 1 { - bals = b.Balances() + bals = b.balances() b.sharedFieldReferences[balances].MinusRef() b.sharedFieldReferences[balances] = &reference{refs: 1} } - b.lock.RUnlock() - - b.lock.Lock() - defer b.lock.Unlock() bals[idx] = val b.state.Balances = bals @@ -459,8 +446,9 @@ func (b *BeaconState) UpdateRandaoMixesAtIndex(idx uint64, val []byte) error { if uint64(len(b.state.RandaoMixes)) <= idx { return errors.Errorf("invalid index provided %d", idx) } + b.lock.Lock() + defer b.lock.Unlock() - b.lock.RLock() mixes := b.state.RandaoMixes if refs := b.sharedFieldReferences[randaoMixes].Refs(); refs > 1 { // Copy elements in underlying array by reference. @@ -469,15 +457,11 @@ func (b *BeaconState) UpdateRandaoMixesAtIndex(idx uint64, val []byte) error { b.sharedFieldReferences[randaoMixes].MinusRef() b.sharedFieldReferences[randaoMixes] = &reference{refs: 1} } - b.lock.RUnlock() - - b.lock.Lock() - defer b.lock.Unlock() mixes[idx] = val b.state.RandaoMixes = mixes b.markFieldAsDirty(randaoMixes) - b.AddDirtyIndices(randaoMixes, []uint64{idx}) + b.addDirtyIndices(randaoMixes, []uint64{idx}) return nil } @@ -508,18 +492,15 @@ func (b *BeaconState) UpdateSlashingsAtIndex(idx uint64, val uint64) error { if uint64(len(b.state.Slashings)) <= idx { return errors.Errorf("invalid index provided %d", idx) } - b.lock.RLock() - s := b.state.Slashings + b.lock.Lock() + defer b.lock.Unlock() + s := b.state.Slashings if b.sharedFieldReferences[slashings].Refs() > 1 { - s = b.Slashings() + s = b.slashings() b.sharedFieldReferences[slashings].MinusRef() b.sharedFieldReferences[slashings] = &reference{refs: 1} } - b.lock.RUnlock() - - b.lock.Lock() - defer b.lock.Unlock() s[idx] = val @@ -571,7 +552,9 @@ func (b *BeaconState) AppendHistoricalRoots(root [32]byte) error { if !b.HasInnerState() { return ErrNilInnerState } - b.lock.RLock() + b.lock.Lock() + defer b.lock.Unlock() + roots := b.state.HistoricalRoots if b.sharedFieldReferences[historicalRoots].Refs() > 1 { roots = make([][]byte, len(b.state.HistoricalRoots)) @@ -579,10 +562,6 @@ func (b *BeaconState) AppendHistoricalRoots(root [32]byte) error { b.sharedFieldReferences[historicalRoots].MinusRef() b.sharedFieldReferences[historicalRoots] = &reference{refs: 1} } - b.lock.RUnlock() - - b.lock.Lock() - defer b.lock.Unlock() b.state.HistoricalRoots = append(roots, root[:]) b.markFieldAsDirty(historicalRoots) @@ -595,7 +574,8 @@ func (b *BeaconState) AppendCurrentEpochAttestations(val *pbp2p.PendingAttestati if !b.HasInnerState() { return ErrNilInnerState } - b.lock.RLock() + b.lock.Lock() + defer b.lock.Unlock() atts := b.state.CurrentEpochAttestations if b.sharedFieldReferences[currentEpochAttestations].Refs() > 1 { @@ -605,10 +585,6 @@ func (b *BeaconState) AppendCurrentEpochAttestations(val *pbp2p.PendingAttestati b.sharedFieldReferences[currentEpochAttestations].MinusRef() b.sharedFieldReferences[currentEpochAttestations] = &reference{refs: 1} } - b.lock.RUnlock() - - b.lock.Lock() - defer b.lock.Unlock() b.state.CurrentEpochAttestations = append(atts, val) b.markFieldAsDirty(currentEpochAttestations) @@ -622,7 +598,9 @@ func (b *BeaconState) AppendPreviousEpochAttestations(val *pbp2p.PendingAttestat if !b.HasInnerState() { return ErrNilInnerState } - b.lock.RLock() + b.lock.Lock() + defer b.lock.Unlock() + atts := b.state.PreviousEpochAttestations if b.sharedFieldReferences[previousEpochAttestations].Refs() > 1 { atts = make([]*pbp2p.PendingAttestation, len(b.state.PreviousEpochAttestations)) @@ -630,14 +608,10 @@ func (b *BeaconState) AppendPreviousEpochAttestations(val *pbp2p.PendingAttestat b.sharedFieldReferences[previousEpochAttestations].MinusRef() b.sharedFieldReferences[previousEpochAttestations] = &reference{refs: 1} } - b.lock.RUnlock() - - b.lock.Lock() - defer b.lock.Unlock() b.state.PreviousEpochAttestations = append(atts, val) b.markFieldAsDirty(previousEpochAttestations) - b.AddDirtyIndices(previousEpochAttestations, []uint64{uint64(len(b.state.PreviousEpochAttestations) - 1)}) + b.addDirtyIndices(previousEpochAttestations, []uint64{uint64(len(b.state.PreviousEpochAttestations) - 1)}) return nil } @@ -648,17 +622,15 @@ func (b *BeaconState) AppendValidator(val *ethpb.Validator) error { if !b.HasInnerState() { return ErrNilInnerState } - b.lock.RLock() + b.lock.Lock() + defer b.lock.Unlock() + vals := b.state.Validators if b.sharedFieldReferences[validators].Refs() > 1 { - vals = b.Validators() + vals = b.validators() b.sharedFieldReferences[validators].MinusRef() b.sharedFieldReferences[validators] = &reference{refs: 1} } - b.lock.RUnlock() - - b.lock.Lock() - defer b.lock.Unlock() // append validator to slice and add // it to the validator map @@ -667,7 +639,7 @@ func (b *BeaconState) AppendValidator(val *ethpb.Validator) error { valMap := coreutils.ValidatorIndexMap(b.state.Validators) b.markFieldAsDirty(validators) - b.AddDirtyIndices(validators, []uint64{valIdx}) + b.addDirtyIndices(validators, []uint64{valIdx}) b.valIdxMap = valMap return nil } @@ -678,18 +650,15 @@ func (b *BeaconState) AppendBalance(bal uint64) error { if !b.HasInnerState() { return ErrNilInnerState } - b.lock.RLock() + b.lock.Lock() + defer b.lock.Unlock() bals := b.state.Balances if b.sharedFieldReferences[balances].Refs() > 1 { - bals = b.Balances() + bals = b.balances() b.sharedFieldReferences[balances].MinusRef() b.sharedFieldReferences[balances] = &reference{refs: 1} } - b.lock.RUnlock() - - b.lock.Lock() - defer b.lock.Unlock() b.state.Balances = append(bals, bal) b.markFieldAsDirty(balances) @@ -791,8 +760,8 @@ func (b *BeaconState) markFieldAsDirty(field fieldIndex) { // do nothing if field already exists } -// AddDirtyIndices adds the relevant dirty field indices, so that they +// addDirtyIndices adds the relevant dirty field indices, so that they // can be recomputed. -func (b *BeaconState) AddDirtyIndices(index fieldIndex, indices []uint64) { +func (b *BeaconState) addDirtyIndices(index fieldIndex, indices []uint64) { b.dirtyIndices[index] = append(b.dirtyIndices[index], indices...) } diff --git a/beacon-chain/state/state_trie.go b/beacon-chain/state/state_trie.go index 1c1feb6c14..c929721f90 100644 --- a/beacon-chain/state/state_trie.go +++ b/beacon-chain/state/state_trie.go @@ -12,6 +12,7 @@ import ( "github.com/prysmaticlabs/prysm/beacon-chain/state/stateutil" pbp2p "github.com/prysmaticlabs/prysm/proto/beacon/p2p/v1" "github.com/prysmaticlabs/prysm/shared/bytesutil" + "github.com/prysmaticlabs/prysm/shared/featureconfig" "github.com/prysmaticlabs/prysm/shared/hashutil" "github.com/prysmaticlabs/prysm/shared/htrutils" "github.com/prysmaticlabs/prysm/shared/params" @@ -72,47 +73,91 @@ func (b *BeaconState) Copy() *BeaconState { if !b.HasInnerState() { return nil } - b.lock.RLock() - defer b.lock.RUnlock() - dst := &BeaconState{ - state: &pbp2p.BeaconState{ - // Primitive types, safe to copy. - GenesisTime: b.state.GenesisTime, - Slot: b.state.Slot, - Eth1DepositIndex: b.state.Eth1DepositIndex, + var dst *BeaconState + if featureconfig.Get().NewBeaconStateLocks { + b.lock.RLock() + defer b.lock.RUnlock() + dst = &BeaconState{ + state: &pbp2p.BeaconState{ + // Primitive types, safe to copy. + GenesisTime: b.state.GenesisTime, + Slot: b.state.Slot, + Eth1DepositIndex: b.state.Eth1DepositIndex, - // Large arrays, infrequently changed, constant size. - RandaoMixes: b.state.RandaoMixes, - StateRoots: b.state.StateRoots, - BlockRoots: b.state.BlockRoots, - PreviousEpochAttestations: b.state.PreviousEpochAttestations, - CurrentEpochAttestations: b.state.CurrentEpochAttestations, - Slashings: b.state.Slashings, - Eth1DataVotes: b.state.Eth1DataVotes, + // Large arrays, infrequently changed, constant size. + RandaoMixes: b.state.RandaoMixes, + StateRoots: b.state.StateRoots, + BlockRoots: b.state.BlockRoots, + PreviousEpochAttestations: b.state.PreviousEpochAttestations, + CurrentEpochAttestations: b.state.CurrentEpochAttestations, + Slashings: b.state.Slashings, + Eth1DataVotes: b.state.Eth1DataVotes, - // Large arrays, increases over time. - Validators: b.state.Validators, - Balances: b.state.Balances, - HistoricalRoots: b.state.HistoricalRoots, + // Large arrays, increases over time. + Validators: b.state.Validators, + Balances: b.state.Balances, + HistoricalRoots: b.state.HistoricalRoots, - // Everything else, too small to be concerned about, constant size. - Fork: b.Fork(), - LatestBlockHeader: b.LatestBlockHeader(), - Eth1Data: b.Eth1Data(), - JustificationBits: b.JustificationBits(), - PreviousJustifiedCheckpoint: b.PreviousJustifiedCheckpoint(), - CurrentJustifiedCheckpoint: b.CurrentJustifiedCheckpoint(), - FinalizedCheckpoint: b.FinalizedCheckpoint(), - GenesisValidatorsRoot: b.GenesisValidatorRoot(), - }, - dirtyFields: make(map[fieldIndex]interface{}, 21), - dirtyIndices: make(map[fieldIndex][]uint64, 21), - rebuildTrie: make(map[fieldIndex]bool, 21), - sharedFieldReferences: make(map[fieldIndex]*reference, 10), - stateFieldLeaves: make(map[fieldIndex]*FieldTrie, 21), + // Everything else, too small to be concerned about, constant size. + Fork: b.fork(), + LatestBlockHeader: b.latestBlockHeader(), + Eth1Data: b.eth1Data(), + JustificationBits: b.justificationBits(), + PreviousJustifiedCheckpoint: b.previousJustifiedCheckpoint(), + CurrentJustifiedCheckpoint: b.currentJustifiedCheckpoint(), + FinalizedCheckpoint: b.finalizedCheckpoint(), + GenesisValidatorsRoot: b.genesisValidatorRoot(), + }, + dirtyFields: make(map[fieldIndex]interface{}, 21), + dirtyIndices: make(map[fieldIndex][]uint64, 21), + rebuildTrie: make(map[fieldIndex]bool, 21), + sharedFieldReferences: make(map[fieldIndex]*reference, 10), + stateFieldLeaves: make(map[fieldIndex]*FieldTrie, 21), - // Copy on write validator index map. - valIdxMap: b.valIdxMap, + // Copy on write validator index map. + valIdxMap: b.valIdxMap, + } + } else { + dst = &BeaconState{ + state: &pbp2p.BeaconState{ + // Primitive types, safe to copy. + GenesisTime: b.state.GenesisTime, + Slot: b.state.Slot, + Eth1DepositIndex: b.state.Eth1DepositIndex, + + // Large arrays, infrequently changed, constant size. + RandaoMixes: b.state.RandaoMixes, + StateRoots: b.state.StateRoots, + BlockRoots: b.state.BlockRoots, + PreviousEpochAttestations: b.state.PreviousEpochAttestations, + CurrentEpochAttestations: b.state.CurrentEpochAttestations, + Slashings: b.state.Slashings, + Eth1DataVotes: b.state.Eth1DataVotes, + + // Large arrays, increases over time. + Validators: b.state.Validators, + Balances: b.state.Balances, + HistoricalRoots: b.state.HistoricalRoots, + + // Everything else, too small to be concerned about, constant size. + Fork: b.Fork(), + LatestBlockHeader: b.LatestBlockHeader(), + Eth1Data: b.Eth1Data(), + JustificationBits: b.JustificationBits(), + PreviousJustifiedCheckpoint: b.PreviousJustifiedCheckpoint(), + CurrentJustifiedCheckpoint: b.CurrentJustifiedCheckpoint(), + FinalizedCheckpoint: b.FinalizedCheckpoint(), + GenesisValidatorsRoot: b.GenesisValidatorRoot(), + }, + dirtyFields: make(map[fieldIndex]interface{}, 21), + dirtyIndices: make(map[fieldIndex][]uint64, 21), + rebuildTrie: make(map[fieldIndex]bool, 21), + sharedFieldReferences: make(map[fieldIndex]*reference, 10), + stateFieldLeaves: make(map[fieldIndex]*FieldTrie, 21), + + // Copy on write validator index map. + valIdxMap: b.valIdxMap, + } } for field, ref := range b.sharedFieldReferences { diff --git a/beacon-chain/state/types.go b/beacon-chain/state/types.go index 391c0c3c7d..07cfdd7647 100644 --- a/beacon-chain/state/types.go +++ b/beacon-chain/state/types.go @@ -1,6 +1,3 @@ -// Package state defines how the beacon chain state for eth2 -// functions in the running beacon node, using an advanced, -// immutable implementation of the state data structure. package state import ( diff --git a/shared/featureconfig/config.go b/shared/featureconfig/config.go index c04e5e84ff..5abe535a98 100644 --- a/shared/featureconfig/config.go +++ b/shared/featureconfig/config.go @@ -30,6 +30,8 @@ var log = logrus.WithField("prefix", "flags") // Flags is a struct to represent which features the client will perform on runtime. type Flags struct { + // State locks + NewBeaconStateLocks bool // NewStateLocks for updated beacon state locking. // Testnet Flags. AltonaTestnet bool // AltonaTestnet defines the flag through which we can enable the node to run on the altona testnet. // Feature related flags. @@ -216,6 +218,10 @@ func ConfigureBeaconChain(ctx *cli.Context) { cfg.DisableGRPCConnectionLogs = true } cfg.AttestationAggregationStrategy = ctx.String(attestationAggregationStrategy.Name) + if ctx.Bool(newBeaconStateLocks.Name) { + log.Warn("Using new beacon state locks") + cfg.NewBeaconStateLocks = true + } if ctx.Bool(forceMaxCoverAttestationAggregation.Name) { log.Warn("Forcing max_cover strategy on attestation aggregation") cfg.AttestationAggregationStrategy = "max_cover" diff --git a/shared/featureconfig/flags.go b/shared/featureconfig/flags.go index ab1e775109..1c1f89df90 100644 --- a/shared/featureconfig/flags.go +++ b/shared/featureconfig/flags.go @@ -136,6 +136,10 @@ var ( Usage: "Which strategy to use when aggregating attestations, one of: naive, max_cover.", Value: "naive", } + newBeaconStateLocks = &cli.BoolFlag{ + Name: "new-beacon-state-locks", + Usage: "Enable new beacon state locking", + } forceMaxCoverAttestationAggregation = &cli.BoolFlag{ Name: "attestation-aggregation-force-maxcover", Usage: "When enabled, forces --attestation-aggregation-strategy=max_cover setting.", @@ -150,6 +154,7 @@ var ( var devModeFlags = []cli.Flag{ initSyncVerifyEverythingFlag, forceMaxCoverAttestationAggregation, + newBeaconStateLocks, } // Deprecated flags list. @@ -581,6 +586,7 @@ var BeaconChainFlags = append(deprecatedFlags, []cli.Flag{ disableReduceAttesterStateCopy, disableGRPCConnectionLogging, attestationAggregationStrategy, + newBeaconStateLocks, forceMaxCoverAttestationAggregation, altonaTestnet, }...)