From d2b39e96970a81e27120484a80263380d39f6661 Mon Sep 17 00:00:00 2001 From: Potuz Date: Mon, 29 Aug 2022 21:48:25 -0300 Subject: [PATCH] Defensive pull tips, doubly-linked-tree (#11175) * Defensive pull tips, doubly-linked-tree * feature flag * gaz Co-authored-by: terencechain Co-authored-by: prylabs-bulldozer[bot] <58059840+prylabs-bulldozer[bot]@users.noreply.github.com> --- .../forkchoice/doubly-linked-tree/BUILD.bazel | 1 + .../doubly-linked-tree/forkchoice.go | 4 +- .../doubly-linked-tree/forkchoice_test.go | 2 +- .../forkchoice/doubly-linked-tree/node.go | 21 +++++++---- .../doubly-linked-tree/node_test.go | 14 +++---- .../forkchoice/doubly-linked-tree/store.go | 6 +-- .../unrealized_justification_test.go | 37 +++++++++++++------ config/features/config.go | 8 +++- config/features/flags.go | 6 +++ time/slots/slottime.go | 2 +- 10 files changed, 69 insertions(+), 32 deletions(-) diff --git a/beacon-chain/forkchoice/doubly-linked-tree/BUILD.bazel b/beacon-chain/forkchoice/doubly-linked-tree/BUILD.bazel index 00c308e2e..383be406b 100644 --- a/beacon-chain/forkchoice/doubly-linked-tree/BUILD.bazel +++ b/beacon-chain/forkchoice/doubly-linked-tree/BUILD.bazel @@ -63,6 +63,7 @@ go_test( "//beacon-chain/forkchoice/types:go_default_library", "//beacon-chain/state:go_default_library", "//beacon-chain/state/v3:go_default_library", + "//config/features:go_default_library", "//config/params:go_default_library", "//consensus-types/blocks:go_default_library", "//consensus-types/primitives:go_default_library", diff --git a/beacon-chain/forkchoice/doubly-linked-tree/forkchoice.go b/beacon-chain/forkchoice/doubly-linked-tree/forkchoice.go index d6bfdd442..1b785cf12 100644 --- a/beacon-chain/forkchoice/doubly-linked-tree/forkchoice.go +++ b/beacon-chain/forkchoice/doubly-linked-tree/forkchoice.go @@ -3,6 +3,7 @@ package doublylinkedtree import ( "context" "fmt" + "time" "github.com/pkg/errors" "github.com/prysmaticlabs/prysm/v3/beacon-chain/core/blocks" @@ -83,7 +84,8 @@ func (f *ForkChoice) Head( jc := f.JustifiedCheckpoint() fc := f.FinalizedCheckpoint() - if err := f.store.treeRootNode.updateBestDescendant(ctx, jc.Epoch, fc.Epoch); err != nil { + currentEpoch := slots.EpochsSinceGenesis(time.Unix(int64(f.store.genesisTime), 0)) + if err := f.store.treeRootNode.updateBestDescendant(ctx, jc.Epoch, fc.Epoch, currentEpoch); err != nil { return [32]byte{}, errors.Wrap(err, "could not update best descendant") } return f.store.head(ctx) diff --git a/beacon-chain/forkchoice/doubly-linked-tree/forkchoice_test.go b/beacon-chain/forkchoice/doubly-linked-tree/forkchoice_test.go index e50265c19..925315903 100644 --- a/beacon-chain/forkchoice/doubly-linked-tree/forkchoice_test.go +++ b/beacon-chain/forkchoice/doubly-linked-tree/forkchoice_test.go @@ -208,7 +208,7 @@ func TestForkChoice_IsCanonicalReorg(t *testing.T) { require.Equal(t, uint64(10), f.store.nodeByRoot[[32]byte{'1'}].weight) require.Equal(t, uint64(0), f.store.nodeByRoot[[32]byte{'2'}].weight) - require.NoError(t, f.store.treeRootNode.updateBestDescendant(ctx, 1, 1)) + require.NoError(t, f.store.treeRootNode.updateBestDescendant(ctx, 1, 1, 1)) require.DeepEqual(t, [32]byte{'3'}, f.store.treeRootNode.bestDescendant.root) f.store.nodesLock.Unlock() diff --git a/beacon-chain/forkchoice/doubly-linked-tree/node.go b/beacon-chain/forkchoice/doubly-linked-tree/node.go index 54d6513c6..ba17de1c4 100644 --- a/beacon-chain/forkchoice/doubly-linked-tree/node.go +++ b/beacon-chain/forkchoice/doubly-linked-tree/node.go @@ -5,6 +5,7 @@ import ( "context" "github.com/pkg/errors" + "github.com/prysmaticlabs/prysm/v3/config/features" "github.com/prysmaticlabs/prysm/v3/config/params" types "github.com/prysmaticlabs/prysm/v3/consensus-types/primitives" v1 "github.com/prysmaticlabs/prysm/v3/proto/eth/v1" @@ -43,7 +44,7 @@ func (n *Node) applyWeightChanges(ctx context.Context) error { // updateBestDescendant updates the best descendant of this node and its // children. This function assumes the caller has a lock on Store.nodesLock -func (n *Node) updateBestDescendant(ctx context.Context, justifiedEpoch, finalizedEpoch types.Epoch) error { +func (n *Node) updateBestDescendant(ctx context.Context, justifiedEpoch, finalizedEpoch, currentEpoch types.Epoch) error { if ctx.Err() != nil { return ctx.Err() } @@ -59,10 +60,10 @@ func (n *Node) updateBestDescendant(ctx context.Context, justifiedEpoch, finaliz if child == nil { return errors.Wrap(ErrNilNode, "could not update best descendant") } - if err := child.updateBestDescendant(ctx, justifiedEpoch, finalizedEpoch); err != nil { + if err := child.updateBestDescendant(ctx, justifiedEpoch, finalizedEpoch, currentEpoch); err != nil { return err } - childLeadsToViableHead := child.leadsToViableHead(justifiedEpoch, finalizedEpoch) + childLeadsToViableHead := child.leadsToViableHead(justifiedEpoch, finalizedEpoch, currentEpoch) if childLeadsToViableHead && !hasViableDescendant { // The child leads to a viable head, but the current // parent's best child doesn't. @@ -97,18 +98,24 @@ func (n *Node) updateBestDescendant(ctx context.Context, justifiedEpoch, finaliz // viableForHead returns true if the node is viable to head. // Any node with different finalized or justified epoch than // the ones in fork choice store should not be viable to head. -func (n *Node) viableForHead(justifiedEpoch, finalizedEpoch types.Epoch) bool { +func (n *Node) viableForHead(justifiedEpoch, finalizedEpoch, currentEpoch types.Epoch) bool { justified := justifiedEpoch == n.justifiedEpoch || justifiedEpoch == 0 finalized := finalizedEpoch == n.finalizedEpoch || finalizedEpoch == 0 + if features.Get().EnableDefensivePull && !justified && justifiedEpoch+1 == currentEpoch { + if n.unrealizedJustifiedEpoch+1 >= currentEpoch { + justified = true + finalized = true + } + } return justified && finalized } -func (n *Node) leadsToViableHead(justifiedEpoch, finalizedEpoch types.Epoch) bool { +func (n *Node) leadsToViableHead(justifiedEpoch, finalizedEpoch, currentEpoch types.Epoch) bool { if n.bestDescendant == nil { - return n.viableForHead(justifiedEpoch, finalizedEpoch) + return n.viableForHead(justifiedEpoch, finalizedEpoch, currentEpoch) } - return n.bestDescendant.viableForHead(justifiedEpoch, finalizedEpoch) + return n.bestDescendant.viableForHead(justifiedEpoch, finalizedEpoch, currentEpoch) } // setNodeAndParentValidated sets the current node and all the ancestors as validated (i.e. non-optimistic). diff --git a/beacon-chain/forkchoice/doubly-linked-tree/node_test.go b/beacon-chain/forkchoice/doubly-linked-tree/node_test.go index 09c00c670..989faa919 100644 --- a/beacon-chain/forkchoice/doubly-linked-tree/node_test.go +++ b/beacon-chain/forkchoice/doubly-linked-tree/node_test.go @@ -114,7 +114,7 @@ func TestNode_UpdateBestDescendant_HigherWeightChild(t *testing.T) { s := f.store s.nodeByRoot[indexToHash(1)].weight = 100 s.nodeByRoot[indexToHash(2)].weight = 200 - assert.NoError(t, s.treeRootNode.updateBestDescendant(ctx, 1, 1)) + assert.NoError(t, s.treeRootNode.updateBestDescendant(ctx, 1, 1, 1)) assert.Equal(t, 2, len(s.treeRootNode.children)) assert.Equal(t, s.treeRootNode.children[1], s.treeRootNode.bestDescendant) @@ -134,7 +134,7 @@ func TestNode_UpdateBestDescendant_LowerWeightChild(t *testing.T) { s := f.store s.nodeByRoot[indexToHash(1)].weight = 200 s.nodeByRoot[indexToHash(2)].weight = 100 - assert.NoError(t, s.treeRootNode.updateBestDescendant(ctx, 1, 1)) + assert.NoError(t, s.treeRootNode.updateBestDescendant(ctx, 1, 1, 1)) assert.Equal(t, 2, len(s.treeRootNode.children)) assert.Equal(t, s.treeRootNode.children[0], s.treeRootNode.bestDescendant) @@ -174,7 +174,7 @@ func TestNode_ViableForHead(t *testing.T) { {&Node{finalizedEpoch: 3, justifiedEpoch: 4}, 4, 3, true}, } for _, tc := range tests { - got := tc.n.viableForHead(tc.justifiedEpoch, tc.finalizedEpoch) + got := tc.n.viableForHead(tc.justifiedEpoch, tc.finalizedEpoch, 5) assert.Equal(t, tc.want, got) } } @@ -198,10 +198,10 @@ func TestNode_LeadsToViableHead(t *testing.T) { require.NoError(t, err) require.NoError(t, f.InsertNode(ctx, state, blkRoot)) - require.Equal(t, true, f.store.treeRootNode.leadsToViableHead(4, 3)) - require.Equal(t, true, f.store.nodeByRoot[indexToHash(5)].leadsToViableHead(4, 3)) - require.Equal(t, false, f.store.nodeByRoot[indexToHash(2)].leadsToViableHead(4, 3)) - require.Equal(t, false, f.store.nodeByRoot[indexToHash(4)].leadsToViableHead(4, 3)) + require.Equal(t, true, f.store.treeRootNode.leadsToViableHead(4, 3, 5)) + require.Equal(t, true, f.store.nodeByRoot[indexToHash(5)].leadsToViableHead(4, 3, 5)) + require.Equal(t, false, f.store.nodeByRoot[indexToHash(2)].leadsToViableHead(4, 3, 5)) + require.Equal(t, false, f.store.nodeByRoot[indexToHash(4)].leadsToViableHead(4, 3, 5)) } func TestNode_SetFullyValidated(t *testing.T) { diff --git a/beacon-chain/forkchoice/doubly-linked-tree/store.go b/beacon-chain/forkchoice/doubly-linked-tree/store.go index 72570658b..a1593dbfd 100644 --- a/beacon-chain/forkchoice/doubly-linked-tree/store.go +++ b/beacon-chain/forkchoice/doubly-linked-tree/store.go @@ -95,8 +95,8 @@ func (s *Store) head(ctx context.Context) ([32]byte, error) { if bestDescendant == nil { bestDescendant = justifiedNode } - - if !bestDescendant.viableForHead(s.justifiedCheckpoint.Epoch, s.finalizedCheckpoint.Epoch) { + currentEpoch := slots.EpochsSinceGenesis(time.Unix(int64(s.genesisTime), 0)) + if !bestDescendant.viableForHead(s.justifiedCheckpoint.Epoch, s.finalizedCheckpoint.Epoch, currentEpoch) { s.allTipsAreInvalid = true return [32]byte{}, fmt.Errorf("head at slot %d with weight %d is not eligible, finalizedEpoch, justified Epoch %d, %d != %d, %d", bestDescendant.slot, bestDescendant.weight/10e9, bestDescendant.finalizedEpoch, bestDescendant.justifiedEpoch, s.finalizedCheckpoint.Epoch, s.justifiedCheckpoint.Epoch) @@ -175,7 +175,7 @@ func (s *Store) insert(ctx context.Context, jEpoch := s.justifiedCheckpoint.Epoch fEpoch := s.finalizedCheckpoint.Epoch s.checkpointsLock.RUnlock() - if err := s.treeRootNode.updateBestDescendant(ctx, jEpoch, fEpoch); err != nil { + if err := s.treeRootNode.updateBestDescendant(ctx, jEpoch, fEpoch, slots.ToEpoch(currentSlot)); err != nil { return n, err } } diff --git a/beacon-chain/forkchoice/doubly-linked-tree/unrealized_justification_test.go b/beacon-chain/forkchoice/doubly-linked-tree/unrealized_justification_test.go index 5aca4b042..8e78f58c3 100644 --- a/beacon-chain/forkchoice/doubly-linked-tree/unrealized_justification_test.go +++ b/beacon-chain/forkchoice/doubly-linked-tree/unrealized_justification_test.go @@ -5,6 +5,7 @@ import ( "testing" forkchoicetypes "github.com/prysmaticlabs/prysm/v3/beacon-chain/forkchoice/types" + "github.com/prysmaticlabs/prysm/v3/config/features" "github.com/prysmaticlabs/prysm/v3/config/params" types "github.com/prysmaticlabs/prysm/v3/consensus-types/primitives" "github.com/prysmaticlabs/prysm/v3/testing/require" @@ -198,31 +199,36 @@ func TestStore_NoDeadLock(t *testing.T) { // D justifies and comes late. // func TestStore_ForkNextEpoch(t *testing.T) { + resetCfg := features.InitWithReset(&features.Flags{ + EnableDefensivePull: true, + }) + defer resetCfg() + f := setup(0, 0) ctx := context.Background() // Epoch 1 blocks (D does not arrive) - state, blkRoot, err := prepareForkchoiceState(ctx, 100, [32]byte{'a'}, params.BeaconConfig().ZeroHash, [32]byte{'A'}, 0, 0) + state, blkRoot, err := prepareForkchoiceState(ctx, 92, [32]byte{'a'}, params.BeaconConfig().ZeroHash, [32]byte{'A'}, 0, 0) require.NoError(t, err) require.NoError(t, f.InsertNode(ctx, state, blkRoot)) - state, blkRoot, err = prepareForkchoiceState(ctx, 101, [32]byte{'b'}, [32]byte{'a'}, [32]byte{'B'}, 0, 0) + state, blkRoot, err = prepareForkchoiceState(ctx, 93, [32]byte{'b'}, [32]byte{'a'}, [32]byte{'B'}, 0, 0) require.NoError(t, err) require.NoError(t, f.InsertNode(ctx, state, blkRoot)) - state, blkRoot, err = prepareForkchoiceState(ctx, 102, [32]byte{'c'}, [32]byte{'b'}, [32]byte{'C'}, 0, 0) + state, blkRoot, err = prepareForkchoiceState(ctx, 94, [32]byte{'c'}, [32]byte{'b'}, [32]byte{'C'}, 0, 0) require.NoError(t, err) require.NoError(t, f.InsertNode(ctx, state, blkRoot)) // Epoch 2 blocks - state, blkRoot, err = prepareForkchoiceState(ctx, 104, [32]byte{'e'}, [32]byte{'c'}, [32]byte{'E'}, 0, 0) + state, blkRoot, err = prepareForkchoiceState(ctx, 96, [32]byte{'e'}, [32]byte{'c'}, [32]byte{'E'}, 0, 0) require.NoError(t, err) require.NoError(t, f.InsertNode(ctx, state, blkRoot)) - state, blkRoot, err = prepareForkchoiceState(ctx, 105, [32]byte{'f'}, [32]byte{'e'}, [32]byte{'F'}, 0, 0) + state, blkRoot, err = prepareForkchoiceState(ctx, 97, [32]byte{'f'}, [32]byte{'e'}, [32]byte{'F'}, 0, 0) require.NoError(t, err) require.NoError(t, f.InsertNode(ctx, state, blkRoot)) - state, blkRoot, err = prepareForkchoiceState(ctx, 106, [32]byte{'g'}, [32]byte{'f'}, [32]byte{'G'}, 0, 0) + state, blkRoot, err = prepareForkchoiceState(ctx, 98, [32]byte{'g'}, [32]byte{'f'}, [32]byte{'G'}, 0, 0) require.NoError(t, err) require.NoError(t, f.InsertNode(ctx, state, blkRoot)) - state, blkRoot, err = prepareForkchoiceState(ctx, 107, [32]byte{'h'}, [32]byte{'g'}, [32]byte{'H'}, 0, 0) + state, blkRoot, err = prepareForkchoiceState(ctx, 99, [32]byte{'h'}, [32]byte{'g'}, [32]byte{'H'}, 0, 0) require.NoError(t, err) require.NoError(t, f.InsertNode(ctx, state, blkRoot)) @@ -234,16 +240,25 @@ func TestStore_ForkNextEpoch(t *testing.T) { require.Equal(t, types.Epoch(0), f.JustifiedCheckpoint().Epoch) // D arrives late, D is head - state, blkRoot, err = prepareForkchoiceState(ctx, 103, [32]byte{'d'}, [32]byte{'c'}, [32]byte{'D'}, 0, 0) + state, blkRoot, err = prepareForkchoiceState(ctx, 95, [32]byte{'d'}, [32]byte{'c'}, [32]byte{'D'}, 0, 0) require.NoError(t, err) require.NoError(t, f.InsertNode(ctx, state, blkRoot)) - require.NoError(t, f.store.setUnrealizedJustifiedEpoch([32]byte{'d'}, 1)) - f.store.unrealizedJustifiedCheckpoint = &forkchoicetypes.Checkpoint{Epoch: 1} + require.NoError(t, f.store.setUnrealizedJustifiedEpoch([32]byte{'d'}, 2)) + f.store.unrealizedJustifiedCheckpoint = &forkchoicetypes.Checkpoint{Epoch: 2} f.updateUnrealizedCheckpoints() headRoot, err = f.Head(ctx, []uint64{100}) require.NoError(t, err) require.Equal(t, [32]byte{'d'}, headRoot) - require.Equal(t, types.Epoch(1), f.JustifiedCheckpoint().Epoch) + require.Equal(t, types.Epoch(2), f.JustifiedCheckpoint().Epoch) + require.Equal(t, uint64(0), f.store.nodeByRoot[[32]byte{'d'}].weight) + require.Equal(t, uint64(100), f.store.nodeByRoot[[32]byte{'h'}].weight) + // Set current epoch to 3, and H's unrealized checkpoint. Check it's head + driftGenesisTime(f, 99, 0) + require.NoError(t, f.store.setUnrealizedJustifiedEpoch([32]byte{'h'}, 2)) + headRoot, err = f.Head(ctx, []uint64{100}) + require.NoError(t, err) + require.Equal(t, [32]byte{'h'}, headRoot) + require.Equal(t, types.Epoch(2), f.JustifiedCheckpoint().Epoch) require.Equal(t, uint64(0), f.store.nodeByRoot[[32]byte{'d'}].weight) require.Equal(t, uint64(100), f.store.nodeByRoot[[32]byte{'h'}].weight) } diff --git a/config/features/config.go b/config/features/config.go index 0a9ffcf42..3de915cce 100644 --- a/config/features/config.go +++ b/config/features/config.go @@ -61,7 +61,8 @@ type Flags struct { EnableSlashingProtectionPruning bool EnableNativeState bool // EnableNativeState defines whether the beacon state will be represented as a pure Go struct or a Go struct that wraps a proto struct. - DisablePullTips bool // DisablePullTips enables experimental disabling of boundary checks. + DisablePullTips bool // DisablePullTips disables experimental disabling of boundary checks. + EnableDefensivePull bool // EnableDefensivePull enables exerimental back boundary checks. EnableVectorizedHTR bool // EnableVectorizedHTR specifies whether the beacon state will use the optimized sha256 routines. DisableForkchoiceDoublyLinkedTree bool // DisableForkChoiceDoublyLinkedTree specifies whether fork choice store will use a doubly linked tree. EnableBatchGossipAggregation bool // EnableBatchGossipAggregation specifies whether to further aggregate our gossip batches before verifying them. @@ -209,6 +210,11 @@ func ConfigureBeaconChain(ctx *cli.Context) error { logEnabled(disablePullTips) cfg.DisablePullTips = true } + if ctx.Bool(enableDefensivePull.Name) { + logEnabled(enableDefensivePull) + cfg.EnableDefensivePull = true + } + if ctx.Bool(disableVecHTR.Name) { logEnabled(disableVecHTR) } else { diff --git a/config/features/flags.go b/config/features/flags.go index ac4e63b30..46bad2dc3 100644 --- a/config/features/flags.go +++ b/config/features/flags.go @@ -97,6 +97,11 @@ var ( Name: "experimental-enable-boundary-checks", Usage: "Experimental enable of boundary checks, useful for debugging, may cause bad votes.", } + enableDefensivePull = &cli.BoolFlag{ + Name: "enable-back-pull", + Usage: "Experimental enable of past boundary checks, useful for debugging, may cause bad votes.", + Hidden: true, + } disableVecHTR = &cli.BoolFlag{ Name: "disable-vectorized-htr", Usage: "Disables the new go sha256 library which utilizes optimized routines for merkle trees", @@ -163,6 +168,7 @@ var BeaconChainFlags = append(deprecatedBeaconFlags, append(deprecatedFlags, []c disableGossipBatchAggregation, EnableOnlyBlindedBeaconBlocks, enableStartupOptimistic, + enableDefensivePull, }...)...) // E2EBeaconChainFlags contains a list of the beacon chain feature flags to be tested in E2E. diff --git a/time/slots/slottime.go b/time/slots/slottime.go index 5d16d3e23..03d6673aa 100644 --- a/time/slots/slottime.go +++ b/time/slots/slottime.go @@ -33,7 +33,7 @@ func SinceGenesis(genesis time.Time) types.Slot { return types.Slot(uint64(prysmTime.Since(genesis).Seconds()) / params.BeaconConfig().SecondsPerSlot) } -// EpochsSinceGenesis returns the number of slots since +// EpochsSinceGenesis returns the number of epochs since // the provided genesis time. func EpochsSinceGenesis(genesis time.Time) types.Epoch { return types.Epoch(SinceGenesis(genesis) / params.BeaconConfig().SlotsPerEpoch)