From c0e396392368c3f7dc030587580e3572f918ebda Mon Sep 17 00:00:00 2001 From: rkapka Date: Thu, 4 Jul 2024 13:24:28 +0200 Subject: [PATCH] review --- .../validator/proposer_attestations_test.go | 106 ++++++++++++++++++ .../electra/forkchoice/forkchoice_test.go | 1 - .../electra/forkchoice/forkchoice_test.go | 1 - validator/client/attest.go | 28 ++--- 4 files changed, 116 insertions(+), 20 deletions(-) diff --git a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations_test.go b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations_test.go index f3705c671b10..fbbf62d7992d 100644 --- a/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations_test.go +++ b/beacon-chain/rpc/prysm/v1alpha1/validator/proposer_attestations_test.go @@ -2,10 +2,13 @@ package validator import ( "bytes" + "context" "sort" "testing" "github.com/prysmaticlabs/go-bitfield" + "github.com/prysmaticlabs/prysm/v5/beacon-chain/operations/attestations" + "github.com/prysmaticlabs/prysm/v5/config/params" "github.com/prysmaticlabs/prysm/v5/consensus-types/primitives" ethpb "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1" "github.com/prysmaticlabs/prysm/v5/testing/assert" @@ -423,3 +426,106 @@ func TestProposer_ProposerAtts_dedup(t *testing.T) { }) } } + +func Test_packAttestations(t *testing.T) { + ctx := context.Background() + phase0Att := ðpb.Attestation{ + AggregationBits: bitfield.Bitlist{0b11111}, + Data: ðpb.AttestationData{ + BeaconBlockRoot: make([]byte, 32), + Source: ðpb.Checkpoint{ + Epoch: 0, + Root: make([]byte, 32), + }, + Target: ðpb.Checkpoint{ + Epoch: 0, + Root: make([]byte, 32), + }, + }, + Signature: make([]byte, 96), + } + cb := primitives.NewAttestationCommitteeBits() + cb.SetBitAt(0, true) + electraAtt := ðpb.AttestationElectra{ + AggregationBits: bitfield.Bitlist{0b11111}, + CommitteeBits: cb, + Data: ðpb.AttestationData{ + BeaconBlockRoot: make([]byte, 32), + Source: ðpb.Checkpoint{ + Epoch: 0, + Root: make([]byte, 32), + }, + Target: ðpb.Checkpoint{ + Epoch: 0, + Root: make([]byte, 32), + }, + }, + Signature: make([]byte, 96), + } + pool := attestations.NewPool() + require.NoError(t, pool.SaveAggregatedAttestations([]ethpb.Att{phase0Att, electraAtt})) + s := &Server{AttPool: pool} + + t.Run("Phase 0", func(t *testing.T) { + st, _ := util.DeterministicGenesisState(t, 64) + require.NoError(t, st.SetSlot(1)) + + atts, err := s.packAttestations(ctx, st, 0) + require.NoError(t, err) + require.Equal(t, 1, len(atts)) + assert.DeepEqual(t, phase0Att, atts[0]) + }) + t.Run("Electra", func(t *testing.T) { + params.SetupTestConfigCleanup(t) + cfg := params.BeaconConfig().Copy() + cfg.ElectraForkEpoch = 1 + params.OverrideBeaconConfig(cfg) + + st, _ := util.DeterministicGenesisStateElectra(t, 64) + require.NoError(t, st.SetSlot(params.BeaconConfig().SlotsPerEpoch+1)) + + atts, err := s.packAttestations(ctx, st, params.BeaconConfig().SlotsPerEpoch) + require.NoError(t, err) + require.Equal(t, 1, len(atts)) + assert.DeepEqual(t, electraAtt, atts[0]) + }) +} + +func Test_limitToMaxAttestations(t *testing.T) { + t.Run("Phase 0", func(t *testing.T) { + atts := make([]ethpb.Att, params.BeaconConfig().MaxAttestations+1) + for i := range atts { + atts[i] = ðpb.Attestation{} + } + + // 1 less than limit + pAtts := proposerAtts(atts[:len(atts)-3]) + assert.Equal(t, len(pAtts), len(pAtts.limitToMaxAttestations())) + + // equal to limit + pAtts = atts[:len(atts)-2] + assert.Equal(t, len(pAtts), len(pAtts.limitToMaxAttestations())) + + // 1 more than limit + pAtts = atts + assert.Equal(t, len(pAtts)-1, len(pAtts.limitToMaxAttestations())) + }) + t.Run("Electra", func(t *testing.T) { + atts := make([]ethpb.Att, params.BeaconConfig().MaxAttestationsElectra+1) + for i := range atts { + atts[i] = ðpb.AttestationElectra{} + } + + // 1 less than limit + pAtts := proposerAtts(atts[:len(atts)-3]) + assert.Equal(t, len(pAtts), len(pAtts.limitToMaxAttestations())) + + // equal to limit + pAtts = atts[:len(atts)-2] + assert.Equal(t, len(pAtts), len(pAtts.limitToMaxAttestations())) + + // 1 more than limit + pAtts = atts + assert.Equal(t, len(pAtts)-1, len(pAtts.limitToMaxAttestations())) + }) +} diff --git a/testing/spectest/mainnet/electra/forkchoice/forkchoice_test.go b/testing/spectest/mainnet/electra/forkchoice/forkchoice_test.go index e46b5557a8a3..50f73e858f6f 100644 --- a/testing/spectest/mainnet/electra/forkchoice/forkchoice_test.go +++ b/testing/spectest/mainnet/electra/forkchoice/forkchoice_test.go @@ -8,6 +8,5 @@ import ( ) func TestMainnet_Electra_Forkchoice(t *testing.T) { - t.Skip("TODO: Electra") forkchoice.Run(t, "mainnet", version.Electra) } diff --git a/testing/spectest/minimal/electra/forkchoice/forkchoice_test.go b/testing/spectest/minimal/electra/forkchoice/forkchoice_test.go index aa04917b5ae6..19c0acd78ad4 100644 --- a/testing/spectest/minimal/electra/forkchoice/forkchoice_test.go +++ b/testing/spectest/minimal/electra/forkchoice/forkchoice_test.go @@ -8,6 +8,5 @@ import ( ) func TestMinimal_Electra_Forkchoice(t *testing.T) { - t.Skip("TODO: Electra") forkchoice.Run(t, "minimal", version.Electra) } diff --git a/validator/client/attest.go b/validator/client/attest.go index fd0ce2506a30..024bfaa0e813 100644 --- a/validator/client/attest.go +++ b/validator/client/attest.go @@ -193,26 +193,18 @@ func (v *validator) SubmitAttestation(ctx context.Context, slot primitives.Slot, return } + span.AddAttributes( + trace.Int64Attribute("slot", int64(slot)), // lint:ignore uintcast -- This conversion is OK for tracing. + trace.StringAttribute("attestationHash", fmt.Sprintf("%#x", attResp.AttestationDataRoot)), + trace.StringAttribute("blockRoot", fmt.Sprintf("%#x", data.BeaconBlockRoot)), + trace.Int64Attribute("justifiedEpoch", int64(data.Source.Epoch)), + trace.Int64Attribute("targetEpoch", int64(data.Target.Epoch)), + trace.StringAttribute("aggregationBitfield", fmt.Sprintf("%#x", aggregationBitfield)), + ) if postElectra { - span.AddAttributes( - trace.Int64Attribute("slot", int64(slot)), // lint:ignore uintcast -- This conversion is OK for tracing. - trace.StringAttribute("attestationHash", fmt.Sprintf("%#x", attResp.AttestationDataRoot)), - trace.StringAttribute("committeeBitfield", fmt.Sprintf("%#x", committeeBits)), - trace.StringAttribute("blockRoot", fmt.Sprintf("%#x", data.BeaconBlockRoot)), - trace.Int64Attribute("justifiedEpoch", int64(data.Source.Epoch)), - trace.Int64Attribute("targetEpoch", int64(data.Target.Epoch)), - trace.StringAttribute("aggregationBitfield", fmt.Sprintf("%#x", aggregationBitfield)), - ) + span.AddAttributes(trace.StringAttribute("committeeBitfield", fmt.Sprintf("%#x", committeeBits))) } else { - span.AddAttributes( - trace.Int64Attribute("slot", int64(slot)), // lint:ignore uintcast -- This conversion is OK for tracing. - trace.StringAttribute("attestationHash", fmt.Sprintf("%#x", attResp.AttestationDataRoot)), - trace.Int64Attribute("committeeIndex", int64(data.CommitteeIndex)), - trace.StringAttribute("blockRoot", fmt.Sprintf("%#x", data.BeaconBlockRoot)), - trace.Int64Attribute("justifiedEpoch", int64(data.Source.Epoch)), - trace.Int64Attribute("targetEpoch", int64(data.Target.Epoch)), - trace.StringAttribute("aggregationBitfield", fmt.Sprintf("%#x", aggregationBitfield)), - ) + span.AddAttributes(trace.Int64Attribute("committeeIndex", int64(data.CommitteeIndex))) } if v.emitAccountMetrics {