From c3adde3b32921d12aad56621917a145c700e672b Mon Sep 17 00:00:00 2001 From: terence tsao Date: Tue, 23 Jun 2020 16:08:08 -0700 Subject: [PATCH] QSP10, QSP11 - Add proper locking (#6350) * Proper locking based on audit feedbacks * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Preston's feedback * Merge branch 'qsp10-11' of github.com:prysmaticlabs/prysm into qsp10-11 * Unlock in invalid index branch * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge branch 'master' into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 * Merge refs/heads/master into qsp10-11 --- beacon-chain/forkchoice/protoarray/store.go | 3 +++ beacon-chain/state/getters.go | 29 ++++++++++++++++++--- beacon-chain/state/setters.go | 3 ++- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/beacon-chain/forkchoice/protoarray/store.go b/beacon-chain/forkchoice/protoarray/store.go index 1b08b4021540..9e596e647f97 100644 --- a/beacon-chain/forkchoice/protoarray/store.go +++ b/beacon-chain/forkchoice/protoarray/store.go @@ -101,6 +101,9 @@ func (f *ForkChoice) Prune(ctx context.Context, finalizedRoot [32]byte) error { // Nodes returns the copied list of block nodes in the fork choice store. func (f *ForkChoice) Nodes() []*Node { + f.store.nodeIndicesLock.RLock() + defer f.store.nodeIndicesLock.RUnlock() + cpy := make([]*Node, len(f.store.Nodes)) copy(cpy, f.store.Nodes) return cpy diff --git a/beacon-chain/state/getters.go b/beacon-chain/state/getters.go index 7b4990e33eb1..76c94936657e 100644 --- a/beacon-chain/state/getters.go +++ b/beacon-chain/state/getters.go @@ -237,6 +237,9 @@ func (b *BeaconState) ParentRoot() [32]byte { if !b.HasInnerState() { return [32]byte{} } + b.lock.RLock() + defer b.lock.RUnlock() + parentRoot := [32]byte{} copy(parentRoot[:], b.state.LatestBlockHeader.ParentRoot) return parentRoot @@ -330,9 +333,14 @@ func (b *BeaconState) Eth1Data() *ethpb.Eth1Data { if !b.HasInnerState() { return nil } + + b.lock.RLock() + defer b.lock.RUnlock() + if b.state.Eth1Data == nil { return nil } + return CopyETH1Data(b.state.Eth1Data) } @@ -342,9 +350,14 @@ func (b *BeaconState) Eth1DataVotes() []*ethpb.Eth1Data { if !b.HasInnerState() { return nil } + + b.lock.RLock() + defer b.lock.RUnlock() + if b.state.Eth1DataVotes == nil { return nil } + res := make([]*ethpb.Eth1Data, len(b.state.Eth1DataVotes)) for i := 0; i < len(res); i++ { res[i] = CopyETH1Data(b.state.Eth1DataVotes[i]) @@ -358,6 +371,10 @@ func (b *BeaconState) Eth1DepositIndex() uint64 { if !b.HasInnerState() { return 0 } + + b.lock.RLock() + defer b.lock.RUnlock() + return b.state.Eth1DepositIndex } @@ -410,16 +427,17 @@ func (b *BeaconState) ValidatorAtIndex(idx uint64) (*ethpb.Validator, error) { if !b.HasInnerState() { return nil, ErrNilInnerState } + + 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) } - - b.lock.RLock() - defer b.lock.RUnlock() - val := b.state.Validators[idx] pubKey := make([]byte, len(val.PublicKey)) copy(pubKey, val.PublicKey) @@ -502,6 +520,9 @@ func (b *BeaconState) NumValidators() int { if !b.HasInnerState() { return 0 } + b.lock.RLock() + defer b.lock.RUnlock() + return len(b.state.Validators) } diff --git a/beacon-chain/state/setters.go b/beacon-chain/state/setters.go index b0661651e5cf..c5108262186c 100644 --- a/beacon-chain/state/setters.go +++ b/beacon-chain/state/setters.go @@ -155,11 +155,12 @@ func (b *BeaconState) UpdateStateRootAtIndex(idx uint64, stateRoot [32]byte) err if !b.HasInnerState() { return ErrNilInnerState } + b.lock.RLock() if len(b.state.StateRoots) <= int(idx) { + b.lock.RUnlock() return errors.Errorf("invalid index provided %d", idx) } - b.lock.RLock() // Check if we hold the only reference to the shared state roots slice. r := b.state.StateRoots if ref := b.sharedFieldReferences[stateRoots]; ref.Refs() > 1 {