diff --git a/beacon-chain/verification/blob_test.go b/beacon-chain/verification/blob_test.go index 441dda9a54..d73919568b 100644 --- a/beacon-chain/verification/blob_test.go +++ b/beacon-chain/verification/blob_test.go @@ -687,6 +687,12 @@ func sbrNotFound(t *testing.T, expectedRoot [32]byte) *mockStateByRooter { }} } +func sbrReturnsState(st state.BeaconState) *mockStateByRooter { + return &mockStateByRooter{sbr: func(_ context.Context, _ [32]byte) (state.BeaconState, error) { + return st, nil + }} +} + func sbrForValOverride(idx primitives.ValidatorIndex, val *ethpb.Validator) *mockStateByRooter { return sbrForValOverrideWithT(nil, idx, val) } diff --git a/beacon-chain/verification/data_column.go b/beacon-chain/verification/data_column.go index 6fed317c9c..de0f8a33bd 100644 --- a/beacon-chain/verification/data_column.go +++ b/beacon-chain/verification/data_column.go @@ -11,12 +11,10 @@ import ( "github.com/OffchainLabs/prysm/v7/beacon-chain/core/helpers" "github.com/OffchainLabs/prysm/v7/beacon-chain/core/peerdas" "github.com/OffchainLabs/prysm/v7/beacon-chain/core/transition" - forkchoicetypes "github.com/OffchainLabs/prysm/v7/beacon-chain/forkchoice/types" "github.com/OffchainLabs/prysm/v7/beacon-chain/state" fieldparams "github.com/OffchainLabs/prysm/v7/config/fieldparams" "github.com/OffchainLabs/prysm/v7/config/params" "github.com/OffchainLabs/prysm/v7/consensus-types/blocks" - "github.com/OffchainLabs/prysm/v7/consensus-types/primitives" "github.com/OffchainLabs/prysm/v7/encoding/bytesutil" "github.com/OffchainLabs/prysm/v7/runtime/logging" "github.com/OffchainLabs/prysm/v7/time/slots" @@ -484,88 +482,19 @@ func (dv *RODataColumnsVerifier) SidecarProposerExpected(ctx context.Context) (e defer dv.recordResult(RequireSidecarProposerExpected, &err) - type slotParentRoot struct { - slot primitives.Slot - parentRoot [fieldparams.RootLength]byte - } - - targetRootBySlotParentRoot := make(map[slotParentRoot][fieldparams.RootLength]byte) - - var targetRootFromCache = func(slot primitives.Slot, parentRoot [fieldparams.RootLength]byte) ([fieldparams.RootLength]byte, error) { - // Use cached values if available. - slotParentRoot := slotParentRoot{slot: slot, parentRoot: parentRoot} - if root, ok := targetRootBySlotParentRoot[slotParentRoot]; ok { - return root, nil - } - - // Compute the epoch of the data column slot. - dataColumnEpoch := slots.ToEpoch(slot) - if dataColumnEpoch > 0 { - dataColumnEpoch = dataColumnEpoch - 1 - } - - // Compute the target root for the epoch. - targetRoot, err := dv.fc.TargetRootForEpoch(parentRoot, dataColumnEpoch) - if err != nil { - return [fieldparams.RootLength]byte{}, columnErrBuilder(errors.Wrap(err, "target root from epoch")) - } - - // Store the target root in the cache. - targetRootBySlotParentRoot[slotParentRoot] = targetRoot - - return targetRoot, nil - } - for _, dataColumn := range dv.dataColumns { - // Extract the slot of the data column. dataColumnSlot := dataColumn.Slot() - // Extract the root of the parent block corresponding to the data column. - parentRoot := dataColumn.ParentRoot() - - // Compute the target root for the data column. - targetRoot, err := targetRootFromCache(dataColumnSlot, parentRoot) + // Get the verifying state, it is guaranteed to have the correct proposer in the lookahead. + verifyingState, err := dv.getVerifyingState(ctx, dataColumn) if err != nil { - return columnErrBuilder(errors.Wrap(err, "target root")) + return columnErrBuilder(errors.Wrap(err, "verifying state")) } - // Compute the epoch of the data column slot. - dataColumnEpoch := slots.ToEpoch(dataColumnSlot) - if dataColumnEpoch > 0 { - dataColumnEpoch = dataColumnEpoch - 1 - } - - // Create a checkpoint for the target root. - checkpoint := &forkchoicetypes.Checkpoint{Root: targetRoot, Epoch: dataColumnEpoch} - - // Try to extract the proposer index from the data column in the cache. - idx, cached := dv.pc.Proposer(checkpoint, dataColumnSlot) - - if !cached { - parentRoot := dataColumn.ParentRoot() - // Ensure the expensive index computation is only performed once for - // concurrent requests for the same signature data. - idxAny, err, _ := dv.sg.Do(concatRootSlot(parentRoot, dataColumnSlot), func() (any, error) { - verifyingState, err := dv.getVerifyingState(ctx, dataColumn) - if err != nil { - return nil, columnErrBuilder(errors.Wrap(err, "verifying state")) - } - - idx, err = helpers.BeaconProposerIndexAtSlot(ctx, verifyingState, dataColumnSlot) - if err != nil { - return nil, columnErrBuilder(errors.Wrap(err, "compute proposer")) - } - - return idx, nil - }) - if err != nil { - return err - } - - var ok bool - if idx, ok = idxAny.(primitives.ValidatorIndex); !ok { - return columnErrBuilder(errors.New("type assertion to ValidatorIndex failed")) - } + // Use proposer lookahead directly + idx, err := helpers.BeaconProposerIndexAtSlot(ctx, verifyingState, dataColumnSlot) + if err != nil { + return columnErrBuilder(errors.Wrap(err, "proposer from lookahead")) } if idx != dataColumn.ProposerIndex() { @@ -626,7 +555,3 @@ func inclusionProofKey(c blocks.RODataColumn) ([32]byte, error) { return sha256.Sum256(unhashedKey), nil } - -func concatRootSlot(root [fieldparams.RootLength]byte, slot primitives.Slot) string { - return string(root[:]) + fmt.Sprintf("%d", slot) -} diff --git a/beacon-chain/verification/data_column_test.go b/beacon-chain/verification/data_column_test.go index ab943520c9..05f1f72adf 100644 --- a/beacon-chain/verification/data_column_test.go +++ b/beacon-chain/verification/data_column_test.go @@ -2,7 +2,6 @@ package verification import ( "reflect" - "sync" "testing" "time" @@ -795,87 +794,90 @@ func TestDataColumnsSidecarProposerExpected(t *testing.T) { blobCount = 1 ) - parentRoot := [fieldparams.RootLength]byte{} - columns := GenerateTestDataColumns(t, parentRoot, columnSlot, blobCount) - firstColumn := columns[0] ctx := t.Context() - testCases := []struct { - name string - stateByRooter StateByRooter - proposerCache proposerCache - columns []blocks.RODataColumn - error string - }{ - { - name: "Cached, matches", - stateByRooter: nil, - proposerCache: &mockProposerCache{ - ProposerCB: pcReturnsIdx(firstColumn.ProposerIndex()), - }, - columns: columns, - }, - { - name: "Cached, does not match", - stateByRooter: nil, - proposerCache: &mockProposerCache{ - ProposerCB: pcReturnsIdx(firstColumn.ProposerIndex() + 1), - }, - columns: columns, - error: errSidecarUnexpectedProposer.Error(), - }, - { - name: "Not cached, state lookup failure", - stateByRooter: sbrNotFound(t, firstColumn.ParentRoot()), - proposerCache: &mockProposerCache{ - ProposerCB: pcReturnsNotFound(), - }, - columns: columns, - error: "verifying state", - }, - } + parentRoot := [fieldparams.RootLength]byte{} - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - initializer := Initializer{ - shared: &sharedResources{ - sr: tc.stateByRooter, - pc: tc.proposerCache, - hsp: &mockHeadStateProvider{}, - fc: &mockForkchoicer{ - TargetRootForEpochCB: fcReturnsTargetRoot([fieldparams.RootLength]byte{}), - }, + // Create a Fulu state to get the expected proposer from the lookahead. + fuluState, _ := util.DeterministicGenesisStateFulu(t, 32) + expectedProposer, err := fuluState.ProposerLookahead() + require.NoError(t, err) + expectedProposerIdx := primitives.ValidatorIndex(expectedProposer[columnSlot]) + + // Generate data columns with the expected proposer index. + matchingColumns := generateTestDataColumnsWithProposer(t, parentRoot, columnSlot, blobCount, expectedProposerIdx) + // Generate data columns with wrong proposer index. + wrongColumns := generateTestDataColumnsWithProposer(t, parentRoot, columnSlot, blobCount, expectedProposerIdx+1) + + t.Run("Proposer matches", func(t *testing.T) { + initializer := Initializer{ + shared: &sharedResources{ + sr: sbrReturnsState(fuluState), + hsp: &mockHeadStateProvider{ + headRoot: parentRoot[:], + headSlot: columnSlot, // Same epoch so HeadStateReadOnly is used + headStateReadOnly: fuluState, }, - } + fc: &mockForkchoicer{}, + }, + } - verifier := initializer.NewDataColumnsVerifier(tc.columns, GossipDataColumnSidecarRequirements) - var wg sync.WaitGroup + verifier := initializer.NewDataColumnsVerifier(matchingColumns, GossipDataColumnSidecarRequirements) + err := verifier.SidecarProposerExpected(ctx) + require.NoError(t, err) + require.Equal(t, true, verifier.results.executed(RequireSidecarProposerExpected)) + require.NoError(t, verifier.results.result(RequireSidecarProposerExpected)) + }) - var err1, err2 error - wg.Go(func() { - err1 = verifier.SidecarProposerExpected(ctx) - }) - wg.Go(func() { - err2 = verifier.SidecarProposerExpected(ctx) - }) - wg.Wait() + t.Run("Proposer does not match", func(t *testing.T) { + initializer := Initializer{ + shared: &sharedResources{ + sr: sbrReturnsState(fuluState), + hsp: &mockHeadStateProvider{ + headRoot: parentRoot[:], + headSlot: columnSlot, // Same epoch so HeadStateReadOnly is used + headStateReadOnly: fuluState, + }, + fc: &mockForkchoicer{}, + }, + } - require.Equal(t, true, verifier.results.executed(RequireSidecarProposerExpected)) + verifier := initializer.NewDataColumnsVerifier(wrongColumns, GossipDataColumnSidecarRequirements) + err := verifier.SidecarProposerExpected(ctx) + require.ErrorContains(t, errSidecarUnexpectedProposer.Error(), err) + require.Equal(t, true, verifier.results.executed(RequireSidecarProposerExpected)) + require.NotNil(t, verifier.results.result(RequireSidecarProposerExpected)) + }) - if len(tc.error) > 0 { - require.ErrorContains(t, tc.error, err1) - require.ErrorContains(t, tc.error, err2) - require.NotNil(t, verifier.results.result(RequireSidecarProposerExpected)) - return - } + t.Run("State lookup failure", func(t *testing.T) { + columns := GenerateTestDataColumns(t, parentRoot, columnSlot, blobCount) + initializer := Initializer{ + shared: &sharedResources{ + sr: sbrNotFound(t, columns[0].ParentRoot()), + hsp: &mockHeadStateProvider{}, + fc: &mockForkchoicer{}, + }, + } - require.NoError(t, err1) - require.NoError(t, err2) - require.NoError(t, verifier.results.result(RequireSidecarProposerExpected)) + verifier := initializer.NewDataColumnsVerifier(columns, GossipDataColumnSidecarRequirements) + err := verifier.SidecarProposerExpected(ctx) + require.ErrorContains(t, "verifying state", err) + require.Equal(t, true, verifier.results.executed(RequireSidecarProposerExpected)) + require.NotNil(t, verifier.results.result(RequireSidecarProposerExpected)) + }) +} - err := verifier.SidecarProposerExpected(ctx) - require.NoError(t, err) - }) +func generateTestDataColumnsWithProposer(t *testing.T, parent [fieldparams.RootLength]byte, slot primitives.Slot, blobCount int, proposer primitives.ValidatorIndex) []blocks.RODataColumn { + roBlock, roBlobs := util.GenerateTestDenebBlockWithSidecar(t, parent, slot, blobCount, util.WithProposer(proposer)) + blobs := make([]kzg.Blob, 0, len(roBlobs)) + for i := range roBlobs { + blobs = append(blobs, kzg.Blob(roBlobs[i].Blob)) } + + cellsPerBlob, proofsPerBlob := util.GenerateCellsAndProofs(t, blobs) + roDataColumnSidecars, err := peerdas.DataColumnSidecars(cellsPerBlob, proofsPerBlob, peerdas.PopulateFromBlock(roBlock)) + require.NoError(t, err) + + return roDataColumnSidecars } func TestColumnRequirementSatisfaction(t *testing.T) { @@ -922,12 +924,3 @@ func TestColumnRequirementSatisfaction(t *testing.T) { require.NoError(t, err) } -func TestConcatRootSlot(t *testing.T) { - root := [fieldparams.RootLength]byte{1, 2, 3} - const slot = primitives.Slot(3210) - - const expected = "\x01\x02\x03\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x003210" - - actual := concatRootSlot(root, slot) - require.Equal(t, expected, actual) -} diff --git a/changelog/potuz_dcs_pc_removal.md b/changelog/potuz_dcs_pc_removal.md new file mode 100644 index 0000000000..96fd9b8864 --- /dev/null +++ b/changelog/potuz_dcs_pc_removal.md @@ -0,0 +1,2 @@ +### Changed +- Use lookahead to validate data column sidecar proposer index. diff --git a/testing/util/deneb.go b/testing/util/deneb.go index 787fbce359..762d2fef65 100644 --- a/testing/util/deneb.go +++ b/testing/util/deneb.go @@ -44,6 +44,13 @@ func WithProposerSigning(idx primitives.ValidatorIndex, sk bls.SecretKey, valRoo } } +// WithProposer sets the proposer index for the generated block without signing. +func WithProposer(idx primitives.ValidatorIndex) DenebBlockGeneratorOption { + return func(g *denebBlockGenerator) { + g.proposer = idx + } +} + func WithPayloadSetter(p *enginev1.ExecutionPayloadDeneb) DenebBlockGeneratorOption { return func(g *denebBlockGenerator) { g.payload = p