From 0f0d480dbc61d212c81127e0945b3cdd53040edb Mon Sep 17 00:00:00 2001 From: terencechain Date: Mon, 3 Oct 2022 10:53:17 -0700 Subject: [PATCH] More efficient way of computing skip slot cache key (#11441) * More efficient way of computing skip slot cache key * Gazelle * Add defensive check * Fix test setup * Disable skip slot cache * Fix rpc tests for dependent root --- .../head_sync_committee_info_test.go | 2 ++ beacon-chain/blockchain/testing/mock.go | 5 ++- beacon-chain/core/transition/BUILD.bazel | 1 - .../core/transition/skip_slot_cache.go | 19 +++++++----- .../core/transition/skip_slot_cache_test.go | 31 +++++++++++++++++++ beacon-chain/core/transition/transition.go | 2 +- beacon-chain/rpc/eth/beacon/pool_test.go | 1 + beacon-chain/rpc/eth/validator/validator.go | 1 + .../rpc/eth/validator/validator_test.go | 6 ++++ .../validator/proposer_bellatrix_test.go | 2 ++ .../prysm/v1alpha1/validator/proposer_test.go | 2 ++ 11 files changed, 62 insertions(+), 10 deletions(-) diff --git a/beacon-chain/blockchain/head_sync_committee_info_test.go b/beacon-chain/blockchain/head_sync_committee_info_test.go index 5982c1d83..af2e67c82 100644 --- a/beacon-chain/blockchain/head_sync_committee_info_test.go +++ b/beacon-chain/blockchain/head_sync_committee_info_test.go @@ -6,6 +6,7 @@ import ( "github.com/prysmaticlabs/prysm/v3/beacon-chain/cache" "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/signing" + "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/transition" dbtest "github.com/prysmaticlabs/prysm/v3/beacon-chain/db/testing" doublylinkedtree "github.com/prysmaticlabs/prysm/v3/beacon-chain/forkchoice/doubly-linked-tree" "github.com/prysmaticlabs/prysm/v3/beacon-chain/state/stategen" @@ -53,6 +54,7 @@ func TestService_HeadDomainFetcher_Errors(t *testing.T) { } func TestService_HeadSyncCommitteeIndices(t *testing.T) { + transition.SkipSlotCache.Disable() s, _ := util.DeterministicGenesisStateAltair(t, params.BeaconConfig().TargetCommitteeSize) c := &Service{} c.head = &head{state: s} diff --git a/beacon-chain/blockchain/testing/mock.go b/beacon-chain/blockchain/testing/mock.go index 13d2746b2..a1a4df2cc 100644 --- a/beacon-chain/blockchain/testing/mock.go +++ b/beacon-chain/blockchain/testing/mock.go @@ -281,7 +281,10 @@ func (s *ChainService) HeadBlock(context.Context) (interfaces.SignedBeaconBlock, // HeadState mocks HeadState method in chain service. func (s *ChainService) HeadState(context.Context) (state.BeaconState, error) { - return s.State, nil + if s.State == nil { + return nil, nil + } + return s.State.Copy(), nil } // CurrentFork mocks HeadState method in chain service. diff --git a/beacon-chain/core/transition/BUILD.bazel b/beacon-chain/core/transition/BUILD.bazel index 1ae2f19e1..953b2f661 100644 --- a/beacon-chain/core/transition/BUILD.bazel +++ b/beacon-chain/core/transition/BUILD.bazel @@ -40,7 +40,6 @@ go_library( "//consensus-types/interfaces:go_default_library", "//consensus-types/primitives:go_default_library", "//crypto/bls:go_default_library", - "//crypto/hash:go_default_library", "//encoding/bytesutil:go_default_library", "//math:go_default_library", "//monitoring/tracing:go_default_library", diff --git a/beacon-chain/core/transition/skip_slot_cache.go b/beacon-chain/core/transition/skip_slot_cache.go index 51bcaa4ed..1fc753e24 100644 --- a/beacon-chain/core/transition/skip_slot_cache.go +++ b/beacon-chain/core/transition/skip_slot_cache.go @@ -6,7 +6,6 @@ import ( "github.com/prysmaticlabs/prysm/v3/beacon-chain/cache" "github.com/prysmaticlabs/prysm/v3/beacon-chain/state" - "github.com/prysmaticlabs/prysm/v3/crypto/hash" "github.com/prysmaticlabs/prysm/v3/encoding/bytesutil" ) @@ -16,17 +15,23 @@ import ( // reasonable amount of time. var SkipSlotCache = cache.NewSkipSlotCache() -// The key for skip slot cache is mixed between state root and state slot. +// SkipSlotCacheKey is the key for skip slot cache is mixed between state root and state slot. // state root is in the mix to defend against different forks with same skip slots // to hit the same cache. We don't want beacon states mixed up between different chains. -func cacheKey(_ context.Context, state state.ReadOnlyBeaconState) ([32]byte, error) { +// [0:24] represents the state root +// [24:32] represents the state slot +func SkipSlotCacheKey(_ context.Context, state state.ReadOnlyBeaconState) ([32]byte, error) { bh := state.LatestBlockHeader() if bh == nil { return [32]byte{}, errors.New("block head in state can't be nil") } - r, err := bh.HashTreeRoot() - if err != nil { - return [32]byte{}, err + sr := bh.StateRoot + if len(sr) != 32 { + return [32]byte{}, errors.New("invalid state root in latest block header") } - return hash.Hash(append(bytesutil.Bytes32(uint64(state.Slot())), r[:]...)), nil + + var b [8]byte + copy(b[:], bytesutil.SlotToBytesBigEndian(state.Slot())) + sr = append(sr[:24], b[:]...) + return bytesutil.ToBytes32(sr), nil } diff --git a/beacon-chain/core/transition/skip_slot_cache_test.go b/beacon-chain/core/transition/skip_slot_cache_test.go index 92066792f..f46e14035 100644 --- a/beacon-chain/core/transition/skip_slot_cache_test.go +++ b/beacon-chain/core/transition/skip_slot_cache_test.go @@ -10,6 +10,8 @@ import ( state_native "github.com/prysmaticlabs/prysm/v3/beacon-chain/state/state-native" "github.com/prysmaticlabs/prysm/v3/config/params" "github.com/prysmaticlabs/prysm/v3/consensus-types/blocks" + "github.com/prysmaticlabs/prysm/v3/encoding/bytesutil" + ethpb "github.com/prysmaticlabs/prysm/v3/proto/prysm/v1alpha1" "github.com/prysmaticlabs/prysm/v3/runtime/version" "github.com/prysmaticlabs/prysm/v3/testing/assert" "github.com/prysmaticlabs/prysm/v3/testing/require" @@ -80,6 +82,8 @@ func TestSkipSlotCache_ConcurrentMixup(t *testing.T) { require.NoError(t, err) s1, err = transition.ExecuteStateTransition(context.Background(), originalState.Copy(), wsb) require.NoError(t, err, "Could not run state transition") + s1, err = transition.ProcessSlot(context.Background(), s1) + require.NoError(t, err, "Could not process slot") } { @@ -93,6 +97,8 @@ func TestSkipSlotCache_ConcurrentMixup(t *testing.T) { require.NoError(t, err) s0, err = transition.ExecuteStateTransition(context.Background(), originalState.Copy(), wsb) require.NoError(t, err, "Could not run state transition") + s0, err = transition.ProcessSlot(context.Background(), s0) + require.NoError(t, err, "Could not process slot") } r1, err := s1.HashTreeRoot(context.Background()) @@ -167,3 +173,28 @@ func TestSkipSlotCache_ConcurrentMixup(t *testing.T) { // Wait for all transitions to finish wg.Wait() } + +func TestSkipSlotCacheKey(t *testing.T) { + st, _ := util.DeterministicGenesisState(t, 100) + require.NoError(t, st.SetSlot(1)) + k, err := transition.SkipSlotCacheKey(context.Background(), st) + require.NoError(t, err) + require.Equal(t, [32]byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, k) + + require.NoError(t, st.SetLatestBlockHeader(ðpb.BeaconBlockHeader{ + StateRoot: bytesutil.PadTo([]byte{1, 2, 3, 4}, 32), + })) + k, err = transition.SkipSlotCacheKey(context.Background(), st) + require.NoError(t, err) + require.Equal(t, [32]byte{1, 2, 3, 4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}, k) +} + +func BenchmarkCacheKey(b *testing.B) { + st, _ := util.DeterministicGenesisState(b, 100) + for i := 0; i < b.N; i++ { + _, err := transition.SkipSlotCacheKey(context.Background(), st) + require.NoError(b, err) + } +} diff --git a/beacon-chain/core/transition/transition.go b/beacon-chain/core/transition/transition.go index e4c233394..e9732321b 100644 --- a/beacon-chain/core/transition/transition.go +++ b/beacon-chain/core/transition/transition.go @@ -200,7 +200,7 @@ func ProcessSlots(ctx context.Context, state state.BeaconState, slot types.Slot) } highestSlot := state.Slot() - key, err := cacheKey(ctx, state) + key, err := SkipSlotCacheKey(ctx, state) if err != nil { return nil, err } diff --git a/beacon-chain/rpc/eth/beacon/pool_test.go b/beacon-chain/rpc/eth/beacon/pool_test.go index c9c9cfb24..969e87001 100644 --- a/beacon-chain/rpc/eth/beacon/pool_test.go +++ b/beacon-chain/rpc/eth/beacon/pool_test.go @@ -446,6 +446,7 @@ func TestSubmitAttesterSlashing_Ok(t *testing.T) { } func TestSubmitAttesterSlashing_AcrossFork(t *testing.T) { + transition.SkipSlotCache.Disable() ctx := context.Background() params.SetupTestConfigCleanup(t) diff --git a/beacon-chain/rpc/eth/validator/validator.go b/beacon-chain/rpc/eth/validator/validator.go index cce248537..71c2f1448 100644 --- a/beacon-chain/rpc/eth/validator/validator.go +++ b/beacon-chain/rpc/eth/validator/validator.go @@ -973,6 +973,7 @@ func attestationDependentRoot(s state.BeaconState, epoch types.Epoch) ([]byte, e } dependentRootSlot = prevEpochStartSlot.Sub(1) } + root, err := helpers.BlockRootAtSlot(s, dependentRootSlot) if err != nil { return nil, errors.Wrap(err, "could not get block root") diff --git a/beacon-chain/rpc/eth/validator/validator_test.go b/beacon-chain/rpc/eth/validator/validator_test.go index ee1a774c6..5fc05477d 100644 --- a/beacon-chain/rpc/eth/validator/validator_test.go +++ b/beacon-chain/rpc/eth/validator/validator_test.go @@ -176,6 +176,9 @@ func TestGetAttesterDuties(t *testing.T) { } resp, err := vs.GetAttesterDuties(ctx, req) require.NoError(t, err) + + bs, err = transition.ProcessSlotsIfPossible(ctx, bs, params.BeaconConfig().SlotsPerEpoch*2) + require.NoError(t, err) assert.DeepEqual(t, bs.BlockRoots()[31], resp.DependentRoot) require.Equal(t, 1, len(resp.Data)) duty := resp.Data[0] @@ -359,6 +362,9 @@ func TestGetProposerDuties(t *testing.T) { } resp, err := vs.GetProposerDuties(ctx, req) require.NoError(t, err) + + bs, err = transition.ProcessSlotsIfPossible(ctx, bs, params.BeaconConfig().SlotsPerEpoch*2) + require.NoError(t, err) assert.DeepEqual(t, bs.BlockRoots()[31], resp.DependentRoot) assert.Equal(t, 32, len(resp.Data)) // We expect a proposer duty for slot 74. diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go index 7a6fe175a..42aa1c862 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_bellatrix_test.go @@ -15,6 +15,7 @@ import ( "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/helpers" "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/signing" prysmtime "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/time" + "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/transition" dbTest "github.com/prysmaticlabs/prysm/v3/beacon-chain/db/testing" mockExecution "github.com/prysmaticlabs/prysm/v3/beacon-chain/execution/testing" doublylinkedtree "github.com/prysmaticlabs/prysm/v3/beacon-chain/forkchoice/doubly-linked-tree" @@ -528,6 +529,7 @@ func TestServer_getAndBuildHeaderBlock(t *testing.T) { } func TestServer_GetBellatrixBeaconBlock_HappyCase(t *testing.T) { + transition.SkipSlotCache.Disable() db := dbTest.SetupDB(t) ctx := context.Background() hook := logTest.NewGlobal() diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go index ad40f57ee..d731647a8 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_test.go @@ -19,6 +19,7 @@ import ( "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/helpers" "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/signing" coretime "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/time" + "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/transition" dbutil "github.com/prysmaticlabs/prysm/v3/beacon-chain/db/testing" mockExecution "github.com/prysmaticlabs/prysm/v3/beacon-chain/execution/testing" doublylinkedtree "github.com/prysmaticlabs/prysm/v3/beacon-chain/forkchoice/doubly-linked-tree" @@ -2089,6 +2090,7 @@ func TestProposer_GetBeaconBlock_PostForkEpoch(t *testing.T) { } func TestProposer_GetBeaconBlock_BellatrixEpoch(t *testing.T) { + transition.SkipSlotCache.Disable() db := dbutil.SetupDB(t) ctx := context.Background() hook := logTest.NewGlobal()