From ab6e9d66863f005e8be030cbfcdd56e573cfdc8c Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Wed, 26 Apr 2023 14:47:05 -0500 Subject: [PATCH 1/8] fix races in committee cache tests --- beacon-chain/cache/active_balance.go | 13 ++++++-- beacon-chain/cache/committee.go | 15 ++++++--- beacon-chain/cache/proposer_indices.go | 13 ++++++-- beacon-chain/cache/sync_committee.go | 32 ++++++++++++++++--- beacon-chain/core/helpers/BUILD.bazel | 2 ++ beacon-chain/core/helpers/beacon_committee.go | 8 ++--- beacon-chain/core/helpers/main_test.go | 13 ++++++++ .../core/helpers/rewards_penalties_test.go | 1 + beacon-chain/core/helpers/sync_committee.go | 4 +-- .../core/helpers/sync_committee_test.go | 6 +++- 10 files changed, 84 insertions(+), 23 deletions(-) create mode 100644 beacon-chain/core/helpers/main_test.go diff --git a/beacon-chain/cache/active_balance.go b/beacon-chain/cache/active_balance.go index 9fc5a8c2e7ad..72c57d2740dc 100644 --- a/beacon-chain/cache/active_balance.go +++ b/beacon-chain/cache/active_balance.go @@ -42,9 +42,16 @@ type BalanceCache struct { // NewEffectiveBalanceCache creates a new effective balance cache for storing/accessing total balance by epoch. func NewEffectiveBalanceCache() *BalanceCache { - return &BalanceCache{ - cache: lruwrpr.New(maxBalanceCacheSize), - } + c := &BalanceCache{} + c.Clear() + return c +} + +// Clear resets the SyncCommitteeCache to its initial state +func (c *BalanceCache) Clear() { + c.lock.Lock() + defer c.lock.Unlock() + c.cache = lruwrpr.New(maxBalanceCacheSize) } // AddTotalEffectiveBalance adds a new total effective balance entry for current balance for state `st` into the cache. diff --git a/beacon-chain/cache/committee.go b/beacon-chain/cache/committee.go index 6fa8a624ba40..8e625f2c7fd5 100644 --- a/beacon-chain/cache/committee.go +++ b/beacon-chain/cache/committee.go @@ -56,10 +56,17 @@ func committeeKeyFn(obj interface{}) (string, error) { // NewCommitteesCache creates a new committee cache for storing/accessing shuffled indices of a committee. func NewCommitteesCache() *CommitteeCache { - return &CommitteeCache{ - CommitteeCache: lruwrpr.New(maxCommitteesCacheSize), - inProgress: make(map[string]bool), - } + cc := &CommitteeCache{} + cc.Clear() + return cc +} + +// Clear resets the CommitteeCache to its initial state +func (c *CommitteeCache) Clear() { + c.lock.Lock() + defer c.lock.Unlock() + c.CommitteeCache = lruwrpr.New(maxCommitteesCacheSize) + c.inProgress = make(map[string]bool) } // Committee fetches the shuffled indices by slot and committee index. Every list of indices diff --git a/beacon-chain/cache/proposer_indices.go b/beacon-chain/cache/proposer_indices.go index 1676d56daf79..dead14534a7c 100644 --- a/beacon-chain/cache/proposer_indices.go +++ b/beacon-chain/cache/proposer_indices.go @@ -46,9 +46,16 @@ func proposerIndicesKeyFn(obj interface{}) (string, error) { // NewProposerIndicesCache creates a new proposer indices cache for storing/accessing proposer index assignments of an epoch. func NewProposerIndicesCache() *ProposerIndicesCache { - return &ProposerIndicesCache{ - proposerIndicesCache: cache.NewFIFO(proposerIndicesKeyFn), - } + c := &ProposerIndicesCache{} + c.Clear() + return c +} + +// Clear resets the ProposerIndicesCache to its initial state +func (c *ProposerIndicesCache) Clear() { + c.lock.Lock() + defer c.lock.Unlock() + c.proposerIndicesCache = cache.NewFIFO(proposerIndicesKeyFn) } // AddProposerIndices adds ProposerIndices object to the cache. diff --git a/beacon-chain/cache/sync_committee.go b/beacon-chain/cache/sync_committee.go index c332be74d9a7..3d37694b7a7c 100644 --- a/beacon-chain/cache/sync_committee.go +++ b/beacon-chain/cache/sync_committee.go @@ -4,7 +4,9 @@ package cache import ( "sync" + "sync/atomic" + "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "github.com/prysmaticlabs/prysm/v4/beacon-chain/state" @@ -31,8 +33,9 @@ var ( // SyncCommitteeCache utilizes a FIFO cache to sufficiently cache validator position within sync committee. // It is thread safe with concurrent read write. type SyncCommitteeCache struct { - cache *cache.FIFO - lock sync.RWMutex + cache *cache.FIFO + lock sync.RWMutex + cleared *atomic.Uint64 } // Index position of all validators in sync committee where `currentSyncCommitteeRoot` is the @@ -51,9 +54,21 @@ type positionInCommittee struct { // NewSyncCommittee initializes and returns a new SyncCommitteeCache. func NewSyncCommittee() *SyncCommitteeCache { - return &SyncCommitteeCache{ - cache: cache.NewFIFO(keyFn), - } + c := &SyncCommitteeCache{cleared: &atomic.Uint64{}} + c.Clear() + return c +} + +// Clear resets the SyncCommitteeCache to its initial state +func (s *SyncCommitteeCache) Clear() { + s.lock.Lock() + defer s.lock.Unlock() + s.cleared.Add(1) + s.cache = cache.NewFIFO(keyFn) +} + +func (s *SyncCommitteeCache) ListKeys() []string { + return s.cache.ListKeys() } // CurrentPeriodIndexPosition returns current period index position of a validator index with respect with @@ -123,6 +138,10 @@ func (s *SyncCommitteeCache) idxPositionInCommittee( // current epoch and next epoch. This should be called when `current_sync_committee` and `next_sync_committee` // change and that happens every `EPOCHS_PER_SYNC_COMMITTEE_PERIOD`. func (s *SyncCommitteeCache) UpdatePositionsInCommittee(syncCommitteeBoundaryRoot [32]byte, st state.BeaconState) error { + // since we call UpdatePositionsInCommittee asynchronously, keep track of the cache value + // seen at the beginning of the routine and compare at the end before updating. If the underlying value has been + // cycled (new address), don't update it. + clearCount := s.cleared.Load() csc, err := st.CurrentSyncCommittee() if err != nil { return err @@ -162,6 +181,9 @@ func (s *SyncCommitteeCache) UpdatePositionsInCommittee(syncCommitteeBoundaryRoo s.lock.Lock() defer s.lock.Unlock() + if clearCount != s.cleared.Load() { + return errors.New("cache rotated during async committee update operation") + } if err := s.cache.Add(&syncCommitteeIndexPosition{ currentSyncCommitteeRoot: syncCommitteeBoundaryRoot, diff --git a/beacon-chain/core/helpers/BUILD.bazel b/beacon-chain/core/helpers/BUILD.bazel index b6283bd29d24..5f440cbd0981 100644 --- a/beacon-chain/core/helpers/BUILD.bazel +++ b/beacon-chain/core/helpers/BUILD.bazel @@ -48,6 +48,7 @@ go_test( "attestation_test.go", "beacon_committee_test.go", "block_test.go", + "main_test.go", "randao_test.go", "rewards_penalties_test.go", "shuffle_test.go", @@ -77,4 +78,5 @@ go_test( "//time/slots:go_default_library", "@com_github_prysmaticlabs_go_bitfield//:go_default_library", ], + race = "on", ) diff --git a/beacon-chain/core/helpers/beacon_committee.go b/beacon-chain/core/helpers/beacon_committee.go index 85a85cc6fb3e..c5eafaf5529e 100644 --- a/beacon-chain/core/helpers/beacon_committee.go +++ b/beacon-chain/core/helpers/beacon_committee.go @@ -382,10 +382,10 @@ func UpdateProposerIndicesInCache(ctx context.Context, state state.ReadOnlyBeaco // ClearCache clears the beacon committee cache and sync committee cache. func ClearCache() { - committeeCache = cache.NewCommitteesCache() - proposerIndicesCache = cache.NewProposerIndicesCache() - syncCommitteeCache = cache.NewSyncCommittee() - balanceCache = cache.NewEffectiveBalanceCache() + committeeCache.Clear() + proposerIndicesCache.Clear() + syncCommitteeCache.Clear() + balanceCache.Clear() } // computeCommittee returns the requested shuffled committee out of the total committees using diff --git a/beacon-chain/core/helpers/main_test.go b/beacon-chain/core/helpers/main_test.go new file mode 100644 index 000000000000..9232793a4533 --- /dev/null +++ b/beacon-chain/core/helpers/main_test.go @@ -0,0 +1,13 @@ +package helpers + +import ( + "os" + "testing" +) + +// run ClearCache before each test to prevent cross-test side effects +func TestMain(m *testing.M) { + ClearCache() + code := m.Run() + os.Exit(code) +} diff --git a/beacon-chain/core/helpers/rewards_penalties_test.go b/beacon-chain/core/helpers/rewards_penalties_test.go index baeec1e2f344..ad421c31a432 100644 --- a/beacon-chain/core/helpers/rewards_penalties_test.go +++ b/beacon-chain/core/helpers/rewards_penalties_test.go @@ -75,6 +75,7 @@ func TestTotalActiveBalance(t *testing.T) { } func TestTotalActiveBal_ReturnMin(t *testing.T) { + ClearCache() tests := []struct { vCount int }{ diff --git a/beacon-chain/core/helpers/sync_committee.go b/beacon-chain/core/helpers/sync_committee.go index 23cfb2d3a9d5..4fe5b123d391 100644 --- a/beacon-chain/core/helpers/sync_committee.go +++ b/beacon-chain/core/helpers/sync_committee.go @@ -25,9 +25,7 @@ var ( // along with the sync committee root. // 1. Checks if the public key exists in the sync committee cache // 2. If 1 fails, checks if the public key exists in the input current sync committee object -func IsCurrentPeriodSyncCommittee( - st state.BeaconState, valIdx primitives.ValidatorIndex, -) (bool, error) { +func IsCurrentPeriodSyncCommittee(st state.BeaconState, valIdx primitives.ValidatorIndex) (bool, error) { root, err := syncPeriodBoundaryRoot(st) if err != nil { return false, err diff --git a/beacon-chain/core/helpers/sync_committee_test.go b/beacon-chain/core/helpers/sync_committee_test.go index c3860d5de849..f4201ec94141 100644 --- a/beacon-chain/core/helpers/sync_committee_test.go +++ b/beacon-chain/core/helpers/sync_committee_test.go @@ -17,6 +17,7 @@ import ( ) func TestIsCurrentEpochSyncCommittee_UsingCache(t *testing.T) { + ClearCache() validators := make([]*ethpb.Validator, params.BeaconConfig().SyncCommitteeSize) syncCommittee := ðpb.SyncCommittee{ AggregatePubkey: bytesutil.PadTo([]byte{}, params.BeaconConfig().BLSPubkeyLength), @@ -47,6 +48,7 @@ func TestIsCurrentEpochSyncCommittee_UsingCache(t *testing.T) { } func TestIsCurrentEpochSyncCommittee_UsingCommittee(t *testing.T) { + ClearCache() validators := make([]*ethpb.Validator, params.BeaconConfig().SyncCommitteeSize) syncCommittee := ðpb.SyncCommittee{ AggregatePubkey: bytesutil.PadTo([]byte{}, params.BeaconConfig().BLSPubkeyLength), @@ -73,6 +75,8 @@ func TestIsCurrentEpochSyncCommittee_UsingCommittee(t *testing.T) { } func TestIsCurrentEpochSyncCommittee_DoesNotExist(t *testing.T) { + ClearCache() + defer ClearCache() validators := make([]*ethpb.Validator, params.BeaconConfig().SyncCommitteeSize) syncCommittee := ðpb.SyncCommittee{ AggregatePubkey: bytesutil.PadTo([]byte{}, params.BeaconConfig().BLSPubkeyLength), @@ -211,6 +215,7 @@ func TestCurrentEpochSyncSubcommitteeIndices_UsingCache(t *testing.T) { } func TestCurrentEpochSyncSubcommitteeIndices_UsingCommittee(t *testing.T) { + ClearCache() validators := make([]*ethpb.Validator, params.BeaconConfig().SyncCommitteeSize) syncCommittee := ðpb.SyncCommittee{ AggregatePubkey: bytesutil.PadTo([]byte{}, params.BeaconConfig().BLSPubkeyLength), @@ -230,7 +235,6 @@ func TestCurrentEpochSyncSubcommitteeIndices_UsingCommittee(t *testing.T) { require.NoError(t, err) require.NoError(t, state.SetCurrentSyncCommittee(syncCommittee)) require.NoError(t, state.SetNextSyncCommittee(syncCommittee)) - root, err := syncPeriodBoundaryRoot(state) require.NoError(t, err) From 0ba28f3c74821157531a45f38a32ab97286f108e Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Wed, 26 Apr 2023 14:50:41 -0500 Subject: [PATCH 2/8] lint --- beacon-chain/core/helpers/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon-chain/core/helpers/BUILD.bazel b/beacon-chain/core/helpers/BUILD.bazel index 5f440cbd0981..8f9ca1439b61 100644 --- a/beacon-chain/core/helpers/BUILD.bazel +++ b/beacon-chain/core/helpers/BUILD.bazel @@ -57,6 +57,7 @@ go_test( "weak_subjectivity_test.go", ], embed = [":go_default_library"], + race = "on", shard_count = 2, deps = [ "//beacon-chain/cache:go_default_library", @@ -78,5 +79,4 @@ go_test( "//time/slots:go_default_library", "@com_github_prysmaticlabs_go_bitfield//:go_default_library", ], - race = "on", ) From 77dfd1fbc25a02a32488b6f85a4a4cedd1ddd98f Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Wed, 26 Apr 2023 15:20:01 -0500 Subject: [PATCH 3/8] gratuitous defer ClearCache if ClearCache --- beacon-chain/cache/sync_committee.go | 4 ---- beacon-chain/core/helpers/BUILD.bazel | 1 - .../core/helpers/beacon_committee_test.go | 10 ++++++++-- beacon-chain/core/helpers/main_test.go | 13 ------------- .../core/helpers/rewards_penalties_test.go | 3 +++ .../core/helpers/sync_committee_test.go | 18 +++++++++++++----- beacon-chain/core/helpers/validators_test.go | 5 +++++ 7 files changed, 29 insertions(+), 25 deletions(-) delete mode 100644 beacon-chain/core/helpers/main_test.go diff --git a/beacon-chain/cache/sync_committee.go b/beacon-chain/cache/sync_committee.go index 3d37694b7a7c..a34a19bb1271 100644 --- a/beacon-chain/cache/sync_committee.go +++ b/beacon-chain/cache/sync_committee.go @@ -67,10 +67,6 @@ func (s *SyncCommitteeCache) Clear() { s.cache = cache.NewFIFO(keyFn) } -func (s *SyncCommitteeCache) ListKeys() []string { - return s.cache.ListKeys() -} - // CurrentPeriodIndexPosition returns current period index position of a validator index with respect with // sync committee. If the input validator index has no assignment, an empty list will be returned. // If the input root does not exist in cache, `ErrNonExistingSyncCommitteeKey` is returned. diff --git a/beacon-chain/core/helpers/BUILD.bazel b/beacon-chain/core/helpers/BUILD.bazel index 8f9ca1439b61..8d984936b4e8 100644 --- a/beacon-chain/core/helpers/BUILD.bazel +++ b/beacon-chain/core/helpers/BUILD.bazel @@ -48,7 +48,6 @@ go_test( "attestation_test.go", "beacon_committee_test.go", "block_test.go", - "main_test.go", "randao_test.go", "rewards_penalties_test.go", "shuffle_test.go", diff --git a/beacon-chain/core/helpers/beacon_committee_test.go b/beacon-chain/core/helpers/beacon_committee_test.go index 5b0051ecc2d7..c28115063868 100644 --- a/beacon-chain/core/helpers/beacon_committee_test.go +++ b/beacon-chain/core/helpers/beacon_committee_test.go @@ -91,6 +91,7 @@ func TestVerifyBitfieldLength_OK(t *testing.T) { func TestCommitteeAssignments_CannotRetrieveFutureEpoch(t *testing.T) { ClearCache() + defer ClearCache() epoch := primitives.Epoch(1) state, err := state_native.InitializeFromProtoPhase0(ðpb.BeaconState{ Slot: 0, // Epoch 0. @@ -101,6 +102,8 @@ func TestCommitteeAssignments_CannotRetrieveFutureEpoch(t *testing.T) { } func TestCommitteeAssignments_NoProposerForSlot0(t *testing.T) { + ClearCache() + defer ClearCache() validators := make([]*ethpb.Validator, 4*params.BeaconConfig().SlotsPerEpoch) for i := 0; i < len(validators); i++ { var activationEpoch primitives.Epoch @@ -118,7 +121,6 @@ func TestCommitteeAssignments_NoProposerForSlot0(t *testing.T) { RandaoMixes: make([][]byte, params.BeaconConfig().EpochsPerHistoricalVector), }) require.NoError(t, err) - ClearCache() _, proposerIndexToSlots, err := CommitteeAssignments(context.Background(), state, 0) require.NoError(t, err, "Failed to determine CommitteeAssignments") for _, ss := range proposerIndexToSlots { @@ -188,6 +190,7 @@ func TestCommitteeAssignments_CanRetrieve(t *testing.T) { }, } + defer ClearCache() for i, tt := range tests { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { ClearCache() @@ -255,6 +258,8 @@ func TestCommitteeAssignments_CannotRetrieveOlderThanSlotsPerHistoricalRoot(t *t } func TestCommitteeAssignments_EverySlotHasMin1Proposer(t *testing.T) { + ClearCache() + defer ClearCache() // Initialize test with 256 validators, each slot and each index gets 4 validators. validators := make([]*ethpb.Validator, 4*params.BeaconConfig().SlotsPerEpoch) for i := 0; i < len(validators); i++ { @@ -269,7 +274,6 @@ func TestCommitteeAssignments_EverySlotHasMin1Proposer(t *testing.T) { RandaoMixes: make([][]byte, params.BeaconConfig().EpochsPerHistoricalVector), }) require.NoError(t, err) - ClearCache() epoch := primitives.Epoch(1) _, proposerIndexToSlots, err := CommitteeAssignments(context.Background(), state, epoch) require.NoError(t, err, "Failed to determine CommitteeAssignments") @@ -376,6 +380,7 @@ func TestVerifyAttestationBitfieldLengths_OK(t *testing.T) { }, } + defer ClearCache() for i, tt := range tests { ClearCache() require.NoError(t, state.SetSlot(tt.stateSlot)) @@ -390,6 +395,7 @@ func TestVerifyAttestationBitfieldLengths_OK(t *testing.T) { func TestUpdateCommitteeCache_CanUpdate(t *testing.T) { ClearCache() + defer ClearCache() validatorCount := params.BeaconConfig().MinGenesisActiveValidatorCount validators := make([]*ethpb.Validator, validatorCount) indices := make([]primitives.ValidatorIndex, validatorCount) diff --git a/beacon-chain/core/helpers/main_test.go b/beacon-chain/core/helpers/main_test.go deleted file mode 100644 index 9232793a4533..000000000000 --- a/beacon-chain/core/helpers/main_test.go +++ /dev/null @@ -1,13 +0,0 @@ -package helpers - -import ( - "os" - "testing" -) - -// run ClearCache before each test to prevent cross-test side effects -func TestMain(m *testing.M) { - ClearCache() - code := m.Run() - os.Exit(code) -} diff --git a/beacon-chain/core/helpers/rewards_penalties_test.go b/beacon-chain/core/helpers/rewards_penalties_test.go index ad421c31a432..24927f5b5ab7 100644 --- a/beacon-chain/core/helpers/rewards_penalties_test.go +++ b/beacon-chain/core/helpers/rewards_penalties_test.go @@ -76,6 +76,7 @@ func TestTotalActiveBalance(t *testing.T) { func TestTotalActiveBal_ReturnMin(t *testing.T) { ClearCache() + defer ClearCache() tests := []struct { vCount int }{ @@ -97,6 +98,8 @@ func TestTotalActiveBal_ReturnMin(t *testing.T) { } func TestTotalActiveBalance_WithCache(t *testing.T) { + ClearCache() + defer ClearCache() tests := []struct { vCount int wantCount int diff --git a/beacon-chain/core/helpers/sync_committee_test.go b/beacon-chain/core/helpers/sync_committee_test.go index f4201ec94141..287e1eba1c85 100644 --- a/beacon-chain/core/helpers/sync_committee_test.go +++ b/beacon-chain/core/helpers/sync_committee_test.go @@ -18,6 +18,7 @@ import ( func TestIsCurrentEpochSyncCommittee_UsingCache(t *testing.T) { ClearCache() + defer ClearCache() validators := make([]*ethpb.Validator, params.BeaconConfig().SyncCommitteeSize) syncCommittee := ðpb.SyncCommittee{ AggregatePubkey: bytesutil.PadTo([]byte{}, params.BeaconConfig().BLSPubkeyLength), @@ -38,7 +39,6 @@ func TestIsCurrentEpochSyncCommittee_UsingCache(t *testing.T) { require.NoError(t, state.SetCurrentSyncCommittee(syncCommittee)) require.NoError(t, state.SetNextSyncCommittee(syncCommittee)) - ClearCache() r := [32]byte{'a'} require.NoError(t, err, syncCommitteeCache.UpdatePositionsInCommittee(r, state)) @@ -49,6 +49,7 @@ func TestIsCurrentEpochSyncCommittee_UsingCache(t *testing.T) { func TestIsCurrentEpochSyncCommittee_UsingCommittee(t *testing.T) { ClearCache() + defer ClearCache() validators := make([]*ethpb.Validator, params.BeaconConfig().SyncCommitteeSize) syncCommittee := ðpb.SyncCommittee{ AggregatePubkey: bytesutil.PadTo([]byte{}, params.BeaconConfig().BLSPubkeyLength), @@ -103,6 +104,8 @@ func TestIsCurrentEpochSyncCommittee_DoesNotExist(t *testing.T) { } func TestIsNextEpochSyncCommittee_UsingCache(t *testing.T) { + ClearCache() + defer ClearCache() validators := make([]*ethpb.Validator, params.BeaconConfig().SyncCommitteeSize) syncCommittee := ðpb.SyncCommittee{ AggregatePubkey: bytesutil.PadTo([]byte{}, params.BeaconConfig().BLSPubkeyLength), @@ -123,7 +126,6 @@ func TestIsNextEpochSyncCommittee_UsingCache(t *testing.T) { require.NoError(t, state.SetCurrentSyncCommittee(syncCommittee)) require.NoError(t, state.SetNextSyncCommittee(syncCommittee)) - ClearCache() r := [32]byte{'a'} require.NoError(t, err, syncCommitteeCache.UpdatePositionsInCommittee(r, state)) @@ -185,6 +187,8 @@ func TestIsNextEpochSyncCommittee_DoesNotExist(t *testing.T) { } func TestCurrentEpochSyncSubcommitteeIndices_UsingCache(t *testing.T) { + ClearCache() + defer ClearCache() validators := make([]*ethpb.Validator, params.BeaconConfig().SyncCommitteeSize) syncCommittee := ðpb.SyncCommittee{ AggregatePubkey: bytesutil.PadTo([]byte{}, params.BeaconConfig().BLSPubkeyLength), @@ -205,7 +209,6 @@ func TestCurrentEpochSyncSubcommitteeIndices_UsingCache(t *testing.T) { require.NoError(t, state.SetCurrentSyncCommittee(syncCommittee)) require.NoError(t, state.SetNextSyncCommittee(syncCommittee)) - ClearCache() r := [32]byte{'a'} require.NoError(t, err, syncCommitteeCache.UpdatePositionsInCommittee(r, state)) @@ -216,6 +219,7 @@ func TestCurrentEpochSyncSubcommitteeIndices_UsingCache(t *testing.T) { func TestCurrentEpochSyncSubcommitteeIndices_UsingCommittee(t *testing.T) { ClearCache() + defer ClearCache() validators := make([]*ethpb.Validator, params.BeaconConfig().SyncCommitteeSize) syncCommittee := ðpb.SyncCommittee{ AggregatePubkey: bytesutil.PadTo([]byte{}, params.BeaconConfig().BLSPubkeyLength), @@ -256,6 +260,7 @@ func TestCurrentEpochSyncSubcommitteeIndices_UsingCommittee(t *testing.T) { func TestCurrentEpochSyncSubcommitteeIndices_DoesNotExist(t *testing.T) { ClearCache() + defer ClearCache() validators := make([]*ethpb.Validator, params.BeaconConfig().SyncCommitteeSize) syncCommittee := ðpb.SyncCommittee{ AggregatePubkey: bytesutil.PadTo([]byte{}, params.BeaconConfig().BLSPubkeyLength), @@ -282,6 +287,8 @@ func TestCurrentEpochSyncSubcommitteeIndices_DoesNotExist(t *testing.T) { } func TestNextEpochSyncSubcommitteeIndices_UsingCache(t *testing.T) { + ClearCache() + defer ClearCache() validators := make([]*ethpb.Validator, params.BeaconConfig().SyncCommitteeSize) syncCommittee := ðpb.SyncCommittee{ AggregatePubkey: bytesutil.PadTo([]byte{}, params.BeaconConfig().BLSPubkeyLength), @@ -302,7 +309,6 @@ func TestNextEpochSyncSubcommitteeIndices_UsingCache(t *testing.T) { require.NoError(t, state.SetCurrentSyncCommittee(syncCommittee)) require.NoError(t, state.SetNextSyncCommittee(syncCommittee)) - ClearCache() r := [32]byte{'a'} require.NoError(t, err, syncCommitteeCache.UpdatePositionsInCommittee(r, state)) @@ -339,6 +345,7 @@ func TestNextEpochSyncSubcommitteeIndices_UsingCommittee(t *testing.T) { func TestNextEpochSyncSubcommitteeIndices_DoesNotExist(t *testing.T) { ClearCache() + defer ClearCache() validators := make([]*ethpb.Validator, params.BeaconConfig().SyncCommitteeSize) syncCommittee := ðpb.SyncCommittee{ AggregatePubkey: bytesutil.PadTo([]byte{}, params.BeaconConfig().BLSPubkeyLength), @@ -391,6 +398,8 @@ func TestUpdateSyncCommitteeCache_BadRoot(t *testing.T) { } func TestIsCurrentEpochSyncCommittee_SameBlockRoot(t *testing.T) { + ClearCache() + defer ClearCache() validators := make([]*ethpb.Validator, params.BeaconConfig().SyncCommitteeSize) syncCommittee := ðpb.SyncCommittee{ AggregatePubkey: bytesutil.PadTo([]byte{}, params.BeaconConfig().BLSPubkeyLength), @@ -416,7 +425,6 @@ func TestIsCurrentEpochSyncCommittee_SameBlockRoot(t *testing.T) { require.NoError(t, state.SetCurrentSyncCommittee(syncCommittee)) require.NoError(t, state.SetNextSyncCommittee(syncCommittee)) - ClearCache() comIdxs, err := CurrentPeriodSyncSubcommitteeIndices(state, 200) require.NoError(t, err) diff --git a/beacon-chain/core/helpers/validators_test.go b/beacon-chain/core/helpers/validators_test.go index ed890814819a..d17766912b67 100644 --- a/beacon-chain/core/helpers/validators_test.go +++ b/beacon-chain/core/helpers/validators_test.go @@ -179,6 +179,7 @@ func TestIsSlashableValidator_OK(t *testing.T) { func TestBeaconProposerIndex_OK(t *testing.T) { params.SetupTestConfigCleanup(t) ClearCache() + defer ClearCache() c := params.BeaconConfig() c.MinGenesisActiveValidatorCount = 16384 params.OverrideBeaconConfig(c) @@ -222,6 +223,7 @@ func TestBeaconProposerIndex_OK(t *testing.T) { }, } + defer ClearCache() for _, tt := range tests { ClearCache() require.NoError(t, state.SetSlot(tt.slot)) @@ -234,6 +236,7 @@ func TestBeaconProposerIndex_OK(t *testing.T) { func TestBeaconProposerIndex_BadState(t *testing.T) { params.SetupTestConfigCleanup(t) ClearCache() + defer ClearCache() c := params.BeaconConfig() c.MinGenesisActiveValidatorCount = 16384 params.OverrideBeaconConfig(c) @@ -345,6 +348,7 @@ func TestChurnLimit_OK(t *testing.T) { {validatorCount: 1000000, wantedChurn: 15 /* validatorCount/churnLimitQuotient */}, {validatorCount: 2000000, wantedChurn: 30 /* validatorCount/churnLimitQuotient */}, } + defer ClearCache() for _, test := range tests { ClearCache() @@ -516,6 +520,7 @@ func TestActiveValidatorIndices(t *testing.T) { want: []primitives.ValidatorIndex{0, 2, 3}, }, } + defer ClearCache() for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { s, err := state_native.InitializeFromProtoPhase0(tt.args.state) From f9894be88de4d5899cfc5f293e54230e13d25d9e Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Wed, 26 Apr 2023 15:53:48 -0500 Subject: [PATCH 4/8] log warning to avoid failed block processing --- beacon-chain/cache/sync_committee.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/beacon-chain/cache/sync_committee.go b/beacon-chain/cache/sync_committee.go index a34a19bb1271..b3507dca40e9 100644 --- a/beacon-chain/cache/sync_committee.go +++ b/beacon-chain/cache/sync_committee.go @@ -6,12 +6,12 @@ import ( "sync" "sync/atomic" - "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "github.com/prysmaticlabs/prysm/v4/beacon-chain/state" "github.com/prysmaticlabs/prysm/v4/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v4/encoding/bytesutil" + log "github.com/sirupsen/logrus" "k8s.io/client-go/tools/cache" ) @@ -178,7 +178,8 @@ func (s *SyncCommitteeCache) UpdatePositionsInCommittee(syncCommitteeBoundaryRoo s.lock.Lock() defer s.lock.Unlock() if clearCount != s.cleared.Load() { - return errors.New("cache rotated during async committee update operation") + log.Warn("cache rotated during async committee update operation - abandoning cache update") + return nil } if err := s.cache.Add(&syncCommitteeIndexPosition{ From ad79d3c02e699ce24ea3c912ec6a992bdfe1693e Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Wed, 26 Apr 2023 16:11:29 -0500 Subject: [PATCH 5/8] gaz --- beacon-chain/cache/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/beacon-chain/cache/BUILD.bazel b/beacon-chain/cache/BUILD.bazel index 0f8ef9d9c594..ebc364da24ef 100644 --- a/beacon-chain/cache/BUILD.bazel +++ b/beacon-chain/cache/BUILD.bazel @@ -47,6 +47,7 @@ go_library( "@com_github_pkg_errors//:go_default_library", "@com_github_prometheus_client_golang//prometheus:go_default_library", "@com_github_prometheus_client_golang//prometheus/promauto:go_default_library", + "@com_github_sirupsen_logrus//:go_default_library", "@io_k8s_client_go//tools/cache:go_default_library", "@io_opencensus_go//trace:go_default_library", ], From a845dfa61385a6dc6db28ba55cbcd59fc9111f13 Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Wed, 26 Apr 2023 19:17:13 -0500 Subject: [PATCH 6/8] add Clear to cache stubs --- beacon-chain/cache/active_balance_disabled.go | 9 ++++----- beacon-chain/cache/committee_disabled.go | 5 +++++ beacon-chain/cache/proposer_indices_disabled.go | 4 ++++ beacon-chain/cache/sync_committee_disabled.go | 5 +++++ 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/beacon-chain/cache/active_balance_disabled.go b/beacon-chain/cache/active_balance_disabled.go index 10dc772b184e..0366fa773ea8 100644 --- a/beacon-chain/cache/active_balance_disabled.go +++ b/beacon-chain/cache/active_balance_disabled.go @@ -3,16 +3,11 @@ package cache import ( - "sync" - - lru "github.com/hashicorp/golang-lru" "github.com/prysmaticlabs/prysm/v4/beacon-chain/state" ) // FakeBalanceCache is a fake struct with 1 LRU cache for looking up balance by epoch. type FakeBalanceCache struct { - cache *lru.Cache - lock sync.RWMutex } // NewEffectiveBalanceCache creates a new effective balance cache for storing/accessing total balance by epoch. @@ -29,3 +24,7 @@ func (c *FakeBalanceCache) AddTotalEffectiveBalance(st state.ReadOnlyBeaconState func (c *FakeBalanceCache) Get(st state.ReadOnlyBeaconState) (uint64, error) { return 0, nil } + +// Clear is a stub. +func (c *FakeBalanceCache) Clear() { +} diff --git a/beacon-chain/cache/committee_disabled.go b/beacon-chain/cache/committee_disabled.go index b984df68f16e..d1baee034bb6 100644 --- a/beacon-chain/cache/committee_disabled.go +++ b/beacon-chain/cache/committee_disabled.go @@ -69,3 +69,8 @@ func (c *FakeCommitteeCache) MarkInProgress(seed [32]byte) error { func (c *FakeCommitteeCache) MarkNotInProgress(seed [32]byte) error { return nil } + +// Clear is a stub. +func (c *FakeCommitteeCache) Clear() { + return nil +} diff --git a/beacon-chain/cache/proposer_indices_disabled.go b/beacon-chain/cache/proposer_indices_disabled.go index 17c2609a57d2..72db16c19e70 100644 --- a/beacon-chain/cache/proposer_indices_disabled.go +++ b/beacon-chain/cache/proposer_indices_disabled.go @@ -33,3 +33,7 @@ func (c *FakeProposerIndicesCache) HasProposerIndices(r [32]byte) (bool, error) func (c *FakeProposerIndicesCache) Len() int { return 0 } + +// Clear is a stub. +func (c *FakeProposerIndicesCache) Clear() int { +} diff --git a/beacon-chain/cache/sync_committee_disabled.go b/beacon-chain/cache/sync_committee_disabled.go index 3cde4de06e43..0eecc319e4e7 100644 --- a/beacon-chain/cache/sync_committee_disabled.go +++ b/beacon-chain/cache/sync_committee_disabled.go @@ -30,3 +30,8 @@ func (s *FakeSyncCommitteeCache) NextPeriodIndexPosition(root [32]byte, valIdx p func (s *FakeSyncCommitteeCache) UpdatePositionsInCommittee(syncCommitteeBoundaryRoot [32]byte, state state.BeaconState) error { return nil } + +// Clear -- fake. +func (s *FakeSyncCommitteeCache) Clear() { + return nil +} From 3f12e180333be286767ba5b9f806fadfbe99fbbe Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Wed, 26 Apr 2023 19:23:10 -0500 Subject: [PATCH 7/8] fix Clear mistakes --- beacon-chain/cache/active_balance_disabled.go | 1 + beacon-chain/cache/proposer_indices_disabled.go | 2 +- beacon-chain/cache/sync_committee_disabled.go | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/beacon-chain/cache/active_balance_disabled.go b/beacon-chain/cache/active_balance_disabled.go index 0366fa773ea8..8704e49f90b9 100644 --- a/beacon-chain/cache/active_balance_disabled.go +++ b/beacon-chain/cache/active_balance_disabled.go @@ -27,4 +27,5 @@ func (c *FakeBalanceCache) Get(st state.ReadOnlyBeaconState) (uint64, error) { // Clear is a stub. func (c *FakeBalanceCache) Clear() { + return } diff --git a/beacon-chain/cache/proposer_indices_disabled.go b/beacon-chain/cache/proposer_indices_disabled.go index 72db16c19e70..1d35d7059596 100644 --- a/beacon-chain/cache/proposer_indices_disabled.go +++ b/beacon-chain/cache/proposer_indices_disabled.go @@ -35,5 +35,5 @@ func (c *FakeProposerIndicesCache) Len() int { } // Clear is a stub. -func (c *FakeProposerIndicesCache) Clear() int { +func (c *FakeProposerIndicesCache) Clear() { } diff --git a/beacon-chain/cache/sync_committee_disabled.go b/beacon-chain/cache/sync_committee_disabled.go index 0eecc319e4e7..623decc38778 100644 --- a/beacon-chain/cache/sync_committee_disabled.go +++ b/beacon-chain/cache/sync_committee_disabled.go @@ -33,5 +33,5 @@ func (s *FakeSyncCommitteeCache) UpdatePositionsInCommittee(syncCommitteeBoundar // Clear -- fake. func (s *FakeSyncCommitteeCache) Clear() { - return nil + return } From bf83ab94592f5804929aea36578a11fe23cd48b4 Mon Sep 17 00:00:00 2001 From: Kasey Kirkham Date: Wed, 26 Apr 2023 19:32:22 -0500 Subject: [PATCH 8/8] last fake cache fix --- beacon-chain/cache/committee_disabled.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon-chain/cache/committee_disabled.go b/beacon-chain/cache/committee_disabled.go index d1baee034bb6..2056f07bdc68 100644 --- a/beacon-chain/cache/committee_disabled.go +++ b/beacon-chain/cache/committee_disabled.go @@ -72,5 +72,5 @@ func (c *FakeCommitteeCache) MarkNotInProgress(seed [32]byte) error { // Clear is a stub. func (c *FakeCommitteeCache) Clear() { - return nil + return }