diff --git a/beacon-chain/blockchain/process_attestation.go b/beacon-chain/blockchain/process_attestation.go index 793fd47fd1..41071f2831 100644 --- a/beacon-chain/blockchain/process_attestation.go +++ b/beacon-chain/blockchain/process_attestation.go @@ -5,6 +5,7 @@ import ( "time" "github.com/OffchainLabs/prysm/v6/beacon-chain/core/helpers" + "github.com/OffchainLabs/prysm/v6/config/params" "github.com/OffchainLabs/prysm/v6/encoding/bytesutil" "github.com/OffchainLabs/prysm/v6/monitoring/tracing/trace" ethpb "github.com/OffchainLabs/prysm/v6/proto/prysm/v1alpha1" @@ -88,7 +89,7 @@ func (s *Service) OnAttestation(ctx context.Context, a ethpb.Att, disparity time if err != nil { return err } - if err := attestation.IsValidAttestationIndices(ctx, indexedAtt); err != nil { + if err := attestation.IsValidAttestationIndices(ctx, indexedAtt, params.BeaconConfig().MaxValidatorsPerCommittee, params.BeaconConfig().MaxCommitteesPerSlot); err != nil { return err } diff --git a/beacon-chain/core/blocks/attestation.go b/beacon-chain/core/blocks/attestation.go index 75a0b76693..10c12407e1 100644 --- a/beacon-chain/core/blocks/attestation.go +++ b/beacon-chain/core/blocks/attestation.go @@ -178,7 +178,7 @@ func VerifyAttestationNoVerifySignature( } } - return attestation.IsValidAttestationIndices(ctx, indexedAtt) + return attestation.IsValidAttestationIndices(ctx, indexedAtt, params.BeaconConfig().MaxValidatorsPerCommittee, params.BeaconConfig().MaxCommitteesPerSlot) } // ProcessAttestationNoVerifySignature processes the attestation without verifying the attestation signature. This @@ -243,7 +243,7 @@ func VerifyIndexedAttestation(ctx context.Context, beaconState state.ReadOnlyBea ctx, span := trace.StartSpan(ctx, "core.VerifyIndexedAttestation") defer span.End() - if err := attestation.IsValidAttestationIndices(ctx, indexedAtt); err != nil { + if err := attestation.IsValidAttestationIndices(ctx, indexedAtt, params.BeaconConfig().MaxValidatorsPerCommittee, params.BeaconConfig().MaxCommitteesPerSlot); err != nil { return err } domain, err := signing.Domain( diff --git a/beacon-chain/core/blocks/signature.go b/beacon-chain/core/blocks/signature.go index af053a3782..2dffeadbf3 100644 --- a/beacon-chain/core/blocks/signature.go +++ b/beacon-chain/core/blocks/signature.go @@ -200,7 +200,7 @@ func createAttestationSignatureBatch( if err != nil { return nil, err } - if err := attestation.IsValidAttestationIndices(ctx, ia); err != nil { + if err := attestation.IsValidAttestationIndices(ctx, ia, params.BeaconConfig().MaxValidatorsPerCommittee, params.BeaconConfig().MaxCommitteesPerSlot); err != nil { return nil, err } indices := ia.GetAttestingIndices() diff --git a/changelog/kasey_decouple-proto-params.md b/changelog/kasey_decouple-proto-params.md new file mode 100644 index 0000000000..3720f25f2f --- /dev/null +++ b/changelog/kasey_decouple-proto-params.md @@ -0,0 +1,2 @@ +### Ignored +- remove usages of params from the proto package so that the params package can access proto types. diff --git a/config/params/BUILD.bazel b/config/params/BUILD.bazel index ef0199ab92..d2d39e5fc7 100644 --- a/config/params/BUILD.bazel +++ b/config/params/BUILD.bazel @@ -29,6 +29,7 @@ go_library( "//consensus-types/primitives:go_default_library", "//encoding/bytesutil:go_default_library", "//math:go_default_library", + "//proto/engine/v1:go_default_library", "//runtime/version:go_default_library", "@com_github_ethereum_go_ethereum//common:go_default_library", "@com_github_ethereum_go_ethereum//params:go_default_library", diff --git a/config/params/config.go b/config/params/config.go index 549f930096..a191ad397b 100644 --- a/config/params/config.go +++ b/config/params/config.go @@ -9,6 +9,7 @@ import ( fieldparams "github.com/OffchainLabs/prysm/v6/config/fieldparams" "github.com/OffchainLabs/prysm/v6/consensus-types/primitives" "github.com/OffchainLabs/prysm/v6/encoding/bytesutil" + enginev1 "github.com/OffchainLabs/prysm/v6/proto/engine/v1" "github.com/OffchainLabs/prysm/v6/runtime/version" "github.com/ethereum/go-ethereum/common" ) @@ -315,6 +316,14 @@ type BeaconChainConfig struct { DeprecatedMaxBlobsPerBlockFulu int `yaml:"MAX_BLOBS_PER_BLOCK_FULU" spec:"true"` } +func (b *BeaconChainConfig) ExecutionRequestLimits() enginev1.ExecutionRequestLimits { + return enginev1.ExecutionRequestLimits{ + Deposits: b.MaxDepositRequestsPerPayload, + Withdrawals: b.MaxWithdrawalsPerPayload, + Consolidations: b.MaxConsolidationsRequestsPerPayload, + } +} + type BlobScheduleEntry struct { Epoch primitives.Epoch `yaml:"EPOCH"` MaxBlobsPerBlock uint64 `yaml:"MAX_BLOBS_PER_BLOCK"` diff --git a/consensus-types/blocks/get_payload.go b/consensus-types/blocks/get_payload.go index 056d083a03..f6a749860f 100644 --- a/consensus-types/blocks/get_payload.go +++ b/consensus-types/blocks/get_payload.go @@ -1,6 +1,7 @@ package blocks import ( + "github.com/OffchainLabs/prysm/v6/config/params" "github.com/OffchainLabs/prysm/v6/consensus-types/interfaces" "github.com/OffchainLabs/prysm/v6/consensus-types/primitives" pb "github.com/OffchainLabs/prysm/v6/proto/engine/v1" @@ -33,7 +34,7 @@ type shouldOverrideBuilderGetter interface { } type executionRequestsGetter interface { - GetDecodedExecutionRequests() (*pb.ExecutionRequests, error) + GetDecodedExecutionRequests(pb.ExecutionRequestLimits) (*pb.ExecutionRequests, error) } func NewGetPayloadResponse(msg proto.Message) (*GetPayloadResponse, error) { @@ -63,7 +64,7 @@ func NewGetPayloadResponse(msg proto.Message) (*GetPayloadResponse, error) { } r.ExecutionData = ed if hasExecutionRequests { - requests, err := executionRequestsGetter.GetDecodedExecutionRequests() + requests, err := executionRequestsGetter.GetDecodedExecutionRequests(params.BeaconConfig().ExecutionRequestLimits()) if err != nil { return nil, err } diff --git a/proto/engine/v1/BUILD.bazel b/proto/engine/v1/BUILD.bazel index 6008410a43..c053fe57b0 100644 --- a/proto/engine/v1/BUILD.bazel +++ b/proto/engine/v1/BUILD.bazel @@ -89,7 +89,6 @@ go_library( visibility = ["//visibility:public"], deps = [ "//config/fieldparams:go_default_library", - "//config/params:go_default_library", "//consensus-types/primitives:go_default_library", "//encoding/bytesutil:go_default_library", "//proto/eth/ext:go_default_library", diff --git a/proto/engine/v1/electra.go b/proto/engine/v1/electra.go index c4a9978498..88bbb3b9c7 100644 --- a/proto/engine/v1/electra.go +++ b/proto/engine/v1/electra.go @@ -3,7 +3,6 @@ package enginev1 import ( "fmt" - "github.com/OffchainLabs/prysm/v6/config/params" "github.com/ethereum/go-ethereum/common/hexutil" "github.com/pkg/errors" ) @@ -23,7 +22,14 @@ const ( ConsolidationRequestType ) -func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequests, error) { +// ExecutionRequestConfig ensures that we don't mix up the execution request params +type ExecutionRequestLimits struct { + Deposits uint64 + Withdrawals uint64 + Consolidations uint64 +} + +func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests(limits ExecutionRequestLimits) (*ExecutionRequests, error) { requests := &ExecutionRequests{} var prevTypeNum *uint8 for i := range ebe.ExecutionRequests { @@ -37,19 +43,19 @@ func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequ prevTypeNum = &requestType switch requestType { case DepositRequestType: - drs, err := unmarshalDeposits(requestListInSSZBytes) + drs, err := unmarshalDeposits(requestListInSSZBytes, limits.Deposits) if err != nil { return nil, err } requests.Deposits = drs case WithdrawalRequestType: - wrs, err := unmarshalWithdrawals(requestListInSSZBytes) + wrs, err := unmarshalWithdrawals(requestListInSSZBytes, limits.Withdrawals) if err != nil { return nil, err } requests.Withdrawals = wrs case ConsolidationRequestType: - crs, err := unmarshalConsolidations(requestListInSSZBytes) + crs, err := unmarshalConsolidations(requestListInSSZBytes, limits.Consolidations) if err != nil { return nil, err } @@ -61,33 +67,33 @@ func (ebe *ExecutionBundleElectra) GetDecodedExecutionRequests() (*ExecutionRequ return requests, nil } -func unmarshalDeposits(requestListInSSZBytes []byte) ([]*DepositRequest, error) { +func unmarshalDeposits(requestListInSSZBytes []byte, maxDepositRequests uint64) ([]*DepositRequest, error) { if len(requestListInSSZBytes) < drSize { return nil, fmt.Errorf("invalid deposit requests SSZ size, got %d expected at least %d", len(requestListInSSZBytes), drSize) } - maxSSZsize := uint64(drSize) * params.BeaconConfig().MaxDepositRequestsPerPayload + maxSSZsize := uint64(drSize) * maxDepositRequests if uint64(len(requestListInSSZBytes)) > maxSSZsize { return nil, fmt.Errorf("invalid deposit requests SSZ size, requests should not be more than the max per payload, got %d max %d", len(requestListInSSZBytes), maxSSZsize) } return unmarshalItems(requestListInSSZBytes, drSize, func() *DepositRequest { return &DepositRequest{} }) } -func unmarshalWithdrawals(requestListInSSZBytes []byte) ([]*WithdrawalRequest, error) { +func unmarshalWithdrawals(requestListInSSZBytes []byte, maxWithdrawals uint64) ([]*WithdrawalRequest, error) { if len(requestListInSSZBytes) < wrSize { return nil, fmt.Errorf("invalid withdrawal requests SSZ size, got %d expected at least %d", len(requestListInSSZBytes), wrSize) } - maxSSZsize := uint64(wrSize) * params.BeaconConfig().MaxWithdrawalRequestsPerPayload + maxSSZsize := uint64(wrSize) * maxWithdrawals if uint64(len(requestListInSSZBytes)) > maxSSZsize { return nil, fmt.Errorf("invalid withdrawal requests SSZ size, requests should not be more than the max per payload, got %d max %d", len(requestListInSSZBytes), maxSSZsize) } return unmarshalItems(requestListInSSZBytes, wrSize, func() *WithdrawalRequest { return &WithdrawalRequest{} }) } -func unmarshalConsolidations(requestListInSSZBytes []byte) ([]*ConsolidationRequest, error) { +func unmarshalConsolidations(requestListInSSZBytes []byte, maxConsolidations uint64) ([]*ConsolidationRequest, error) { if len(requestListInSSZBytes) < crSize { return nil, fmt.Errorf("invalid consolidation requests SSZ size, got %d expected at least %d", len(requestListInSSZBytes), crSize) } - maxSSZsize := uint64(crSize) * params.BeaconConfig().MaxConsolidationsRequestsPerPayload + maxSSZsize := uint64(crSize) * maxConsolidations if uint64(len(requestListInSSZBytes)) > maxSSZsize { return nil, fmt.Errorf("invalid consolidation requests SSZ size, requests should not be more than the max per payload, got %d max %d", len(requestListInSSZBytes), maxSSZsize) } diff --git a/proto/engine/v1/electra_test.go b/proto/engine/v1/electra_test.go index e70e8bb09a..281436c2e3 100644 --- a/proto/engine/v1/electra_test.go +++ b/proto/engine/v1/electra_test.go @@ -13,6 +13,7 @@ import ( var depositRequestsSSZHex = "0x706b0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000077630000000000000000000000000000000000000000000000000000000000007b00000000000000736967000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000c801000000000000706b00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000776300000000000000000000000000000000000000000000000000000000000090010000000000007369670000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002000000000000000" func TestGetDecodedExecutionRequests(t *testing.T) { + cfg := params.BeaconConfig() t.Run("All requests decode successfully", func(t *testing.T) { depositRequestBytes, err := hexutil.Decode("0x610000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000" + "620000000000000000000000000000000000000000000000000000000000000000" + @@ -31,7 +32,7 @@ func TestGetDecodedExecutionRequests(t *testing.T) { append([]byte{uint8(enginev1.WithdrawalRequestType)}, withdrawalRequestBytes...), append([]byte{uint8(enginev1.ConsolidationRequestType)}, consolidationRequestBytes...)}, } - requests, err := ebe.GetDecodedExecutionRequests() + requests, err := ebe.GetDecodedExecutionRequests(cfg.ExecutionRequestLimits()) require.NoError(t, err) require.Equal(t, len(requests.Deposits), 1) require.Equal(t, len(requests.Withdrawals), 1) @@ -50,7 +51,7 @@ func TestGetDecodedExecutionRequests(t *testing.T) { ebe := &enginev1.ExecutionBundleElectra{ ExecutionRequests: [][]byte{append([]byte{uint8(enginev1.DepositRequestType)}, depositRequestBytes...), append([]byte{uint8(enginev1.ConsolidationRequestType)}, consolidationRequestBytes...)}, } - requests, err := ebe.GetDecodedExecutionRequests() + requests, err := ebe.GetDecodedExecutionRequests(cfg.ExecutionRequestLimits()) require.NoError(t, err) require.Equal(t, len(requests.Deposits), 1) require.Equal(t, len(requests.Withdrawals), 0) @@ -69,7 +70,7 @@ func TestGetDecodedExecutionRequests(t *testing.T) { ebe := &enginev1.ExecutionBundleElectra{ ExecutionRequests: [][]byte{append([]byte{uint8(enginev1.ConsolidationRequestType)}, consolidationRequestBytes...), append([]byte{uint8(enginev1.DepositRequestType)}, depositRequestBytes...)}, } - _, err = ebe.GetDecodedExecutionRequests() + _, err = ebe.GetDecodedExecutionRequests(cfg.ExecutionRequestLimits()) require.ErrorContains(t, "invalid execution request type order", err) }) t.Run("Requests should error if the request type is shorter than 1 byte", func(t *testing.T) { @@ -80,7 +81,7 @@ func TestGetDecodedExecutionRequests(t *testing.T) { ebe := &enginev1.ExecutionBundleElectra{ ExecutionRequests: [][]byte{append([]byte{}, []byte{}...), append([]byte{uint8(enginev1.ConsolidationRequestType)}, consolidationRequestBytes...)}, } - _, err = ebe.GetDecodedExecutionRequests() + _, err = ebe.GetDecodedExecutionRequests(cfg.ExecutionRequestLimits()) require.ErrorContains(t, "invalid execution request, length less than 1", err) }) t.Run("a duplicate request should fail", func(t *testing.T) { @@ -93,7 +94,7 @@ func TestGetDecodedExecutionRequests(t *testing.T) { ebe := &enginev1.ExecutionBundleElectra{ ExecutionRequests: [][]byte{append([]byte{uint8(enginev1.WithdrawalRequestType)}, withdrawalRequestBytes...), append([]byte{uint8(enginev1.WithdrawalRequestType)}, withdrawalRequestBytes2...)}, } - _, err = ebe.GetDecodedExecutionRequests() + _, err = ebe.GetDecodedExecutionRequests(cfg.ExecutionRequestLimits()) require.ErrorContains(t, "requests should be in sorted order and unique", err) }) t.Run("a duplicate withdrawals ( non 0 request type )request should fail", func(t *testing.T) { @@ -110,7 +111,7 @@ func TestGetDecodedExecutionRequests(t *testing.T) { ebe := &enginev1.ExecutionBundleElectra{ ExecutionRequests: [][]byte{append([]byte{uint8(enginev1.DepositRequestType)}, depositRequestBytes...), append([]byte{uint8(enginev1.DepositRequestType)}, depositRequestBytes2...)}, } - _, err = ebe.GetDecodedExecutionRequests() + _, err = ebe.GetDecodedExecutionRequests(cfg.ExecutionRequestLimits()) require.ErrorContains(t, "requests should be in sorted order and unique", err) }) t.Run("If a request type is provided, but the request list is shorter than the ssz of 1 request we error", func(t *testing.T) { @@ -121,11 +122,11 @@ func TestGetDecodedExecutionRequests(t *testing.T) { ebe := &enginev1.ExecutionBundleElectra{ ExecutionRequests: [][]byte{append([]byte{uint8(enginev1.DepositRequestType)}, []byte{}...), append([]byte{uint8(enginev1.ConsolidationRequestType)}, consolidationRequestBytes...)}, } - _, err = ebe.GetDecodedExecutionRequests() + _, err = ebe.GetDecodedExecutionRequests(cfg.ExecutionRequestLimits()) require.ErrorContains(t, "invalid deposit requests SSZ size", err) }) t.Run("If deposit requests are over the max allowed per payload then we should error", func(t *testing.T) { - requests := make([]*enginev1.DepositRequest, params.BeaconConfig().MaxDepositRequestsPerPayload+1) + requests := make([]*enginev1.DepositRequest, cfg.MaxDepositRequestsPerPayload+1) for i := range requests { requests[i] = &enginev1.DepositRequest{ Pubkey: bytesutil.PadTo([]byte("pk"), 48), @@ -142,11 +143,11 @@ func TestGetDecodedExecutionRequests(t *testing.T) { append([]byte{uint8(enginev1.DepositRequestType)}, by...), }, } - _, err = ebe.GetDecodedExecutionRequests() + _, err = ebe.GetDecodedExecutionRequests(cfg.ExecutionRequestLimits()) require.ErrorContains(t, "invalid deposit requests SSZ size, requests should not be more than the max per payload", err) }) t.Run("If withdrawal requests are over the max allowed per payload then we should error", func(t *testing.T) { - requests := make([]*enginev1.WithdrawalRequest, params.BeaconConfig().MaxWithdrawalRequestsPerPayload+1) + requests := make([]*enginev1.WithdrawalRequest, cfg.MaxWithdrawalRequestsPerPayload+1) for i := range requests { requests[i] = &enginev1.WithdrawalRequest{ SourceAddress: bytesutil.PadTo([]byte("sa"), 20), @@ -161,11 +162,11 @@ func TestGetDecodedExecutionRequests(t *testing.T) { append([]byte{uint8(enginev1.WithdrawalRequestType)}, by...), }, } - _, err = ebe.GetDecodedExecutionRequests() + _, err = ebe.GetDecodedExecutionRequests(cfg.ExecutionRequestLimits()) require.ErrorContains(t, "invalid withdrawal requests SSZ size, requests should not be more than the max per payload", err) }) t.Run("If consolidation requests are over the max allowed per payload then we should error", func(t *testing.T) { - requests := make([]*enginev1.ConsolidationRequest, params.BeaconConfig().MaxConsolidationsRequestsPerPayload+1) + requests := make([]*enginev1.ConsolidationRequest, cfg.MaxConsolidationsRequestsPerPayload+1) for i := range requests { requests[i] = &enginev1.ConsolidationRequest{ SourceAddress: bytesutil.PadTo([]byte("sa"), 20), @@ -180,7 +181,7 @@ func TestGetDecodedExecutionRequests(t *testing.T) { append([]byte{uint8(enginev1.ConsolidationRequestType)}, by...), }, } - _, err = ebe.GetDecodedExecutionRequests() + _, err = ebe.GetDecodedExecutionRequests(cfg.ExecutionRequestLimits()) require.ErrorContains(t, "invalid consolidation requests SSZ size, requests should not be more than the max per payload", err) }) } diff --git a/proto/engine/v1/fulu.go b/proto/engine/v1/fulu.go index 26202a239b..e3db6cc56e 100644 --- a/proto/engine/v1/fulu.go +++ b/proto/engine/v1/fulu.go @@ -4,7 +4,7 @@ import ( "github.com/pkg/errors" ) -func (ebe *ExecutionBundleFulu) GetDecodedExecutionRequests() (*ExecutionRequests, error) { +func (ebe *ExecutionBundleFulu) GetDecodedExecutionRequests(limits ExecutionRequestLimits) (*ExecutionRequests, error) { requests := &ExecutionRequests{} var prevTypeNum *uint8 for i := range ebe.ExecutionRequests { @@ -18,19 +18,19 @@ func (ebe *ExecutionBundleFulu) GetDecodedExecutionRequests() (*ExecutionRequest prevTypeNum = &requestType switch requestType { case DepositRequestType: - drs, err := unmarshalDeposits(requestListInSSZBytes) + drs, err := unmarshalDeposits(requestListInSSZBytes, limits.Deposits) if err != nil { return nil, err } requests.Deposits = drs case WithdrawalRequestType: - wrs, err := unmarshalWithdrawals(requestListInSSZBytes) + wrs, err := unmarshalWithdrawals(requestListInSSZBytes, limits.Withdrawals) if err != nil { return nil, err } requests.Withdrawals = wrs case ConsolidationRequestType: - crs, err := unmarshalConsolidations(requestListInSSZBytes) + crs, err := unmarshalConsolidations(requestListInSSZBytes, limits.Consolidations) if err != nil { return nil, err } diff --git a/proto/prysm/v1alpha1/attestation/BUILD.bazel b/proto/prysm/v1alpha1/attestation/BUILD.bazel index 3ba7170a59..00da0e7a62 100644 --- a/proto/prysm/v1alpha1/attestation/BUILD.bazel +++ b/proto/prysm/v1alpha1/attestation/BUILD.bazel @@ -10,7 +10,6 @@ go_library( visibility = ["//visibility:public"], deps = [ "//beacon-chain/core/signing:go_default_library", - "//config/params:go_default_library", "//consensus-types/primitives:go_default_library", "//crypto/bls:go_default_library", "//crypto/hash:go_default_library", diff --git a/proto/prysm/v1alpha1/attestation/aggregation/attestations/BUILD.bazel b/proto/prysm/v1alpha1/attestation/aggregation/attestations/BUILD.bazel index 006af59062..60168e29a9 100644 --- a/proto/prysm/v1alpha1/attestation/aggregation/attestations/BUILD.bazel +++ b/proto/prysm/v1alpha1/attestation/aggregation/attestations/BUILD.bazel @@ -23,6 +23,7 @@ go_test( name = "go_default_test", srcs = [ "attestations_test.go", + "export_test.go", "maxcover_test.go", ], embed = [":go_default_library"], diff --git a/proto/prysm/v1alpha1/attestation/aggregation/attestations/attestations_test.go b/proto/prysm/v1alpha1/attestation/aggregation/attestations/attestations_test.go index 468f1862ac..910cf72039 100644 --- a/proto/prysm/v1alpha1/attestation/aggregation/attestations/attestations_test.go +++ b/proto/prysm/v1alpha1/attestation/aggregation/attestations/attestations_test.go @@ -1,4 +1,4 @@ -package attestations +package attestations_test import ( "io" @@ -10,6 +10,7 @@ import ( "github.com/OffchainLabs/prysm/v6/encoding/ssz/equality" ethpb "github.com/OffchainLabs/prysm/v6/proto/prysm/v1alpha1" "github.com/OffchainLabs/prysm/v6/proto/prysm/v1alpha1/attestation/aggregation" + "github.com/OffchainLabs/prysm/v6/proto/prysm/v1alpha1/attestation/aggregation/attestations" aggtesting "github.com/OffchainLabs/prysm/v6/proto/prysm/v1alpha1/attestation/aggregation/testing" "github.com/OffchainLabs/prysm/v6/testing/assert" "github.com/OffchainLabs/prysm/v6/testing/require" @@ -46,7 +47,7 @@ func TestAggregateAttestations_AggregatePair(t *testing.T) { }, } for _, tt := range tests { - got, err := AggregatePair(tt.a1, tt.a2) + got, err := attestations.AggregatePair(tt.a1, tt.a2) require.NoError(t, err) require.Equal(t, true, equality.DeepEqual(got, tt.want)) } @@ -67,7 +68,7 @@ func TestAggregateAttestations_AggregatePair_OverlapFails(t *testing.T) { }, } for _, tt := range tests { - _, err := AggregatePair(tt.a1, tt.a2) + _, err := attestations.AggregatePair(tt.a1, tt.a2) require.ErrorContains(t, aggregation.ErrBitsOverlap.Error(), err) } } @@ -83,7 +84,7 @@ func TestAggregateAttestations_AggregatePair_DiffLengthFails(t *testing.T) { }, } for _, tt := range tests { - _, err := AggregatePair(tt.a1, tt.a2) + _, err := attestations.AggregatePair(tt.a1, tt.a2) require.ErrorContains(t, bitfield.ErrBitlistDifferentLength.Error(), err) } } @@ -209,7 +210,7 @@ func TestAggregateAttestations_Aggregate(t *testing.T) { for _, tt := range tests { runner := func() { - got, err := Aggregate(aggtesting.MakeAttestationsFromBitlists(tt.inputs)) + got, err := attestations.Aggregate(aggtesting.MakeAttestationsFromBitlists(tt.inputs)) if tt.err != nil { require.ErrorContains(t, tt.err.Error(), err) return @@ -233,7 +234,7 @@ func TestAggregateAttestations_Aggregate(t *testing.T) { t.Run("broken attestation bitset", func(t *testing.T) { wantErr := "bitlist cannot be nil or empty: invalid max_cover problem" - _, err := Aggregate(aggtesting.MakeAttestationsFromBitlists([]bitfield.Bitlist{ + _, err := attestations.Aggregate(aggtesting.MakeAttestationsFromBitlists([]bitfield.Bitlist{ {0b00000011, 0b0}, {0b00000111, 0b100}, {0b00000100, 0b1}, @@ -245,7 +246,7 @@ func TestAggregateAttestations_Aggregate(t *testing.T) { // The first item cannot be aggregated, and should be pushed down the list, // by two swaps with aggregated items (aggregation is done in-place, so the very same // underlying array is used for storing both aggregated and non-aggregated items). - got, err := Aggregate(aggtesting.MakeAttestationsFromBitlists([]bitfield.Bitlist{ + got, err := attestations.Aggregate(aggtesting.MakeAttestationsFromBitlists([]bitfield.Bitlist{ {0b10000000, 0b1}, {0b11000101, 0b1}, {0b00011000, 0b1}, diff --git a/proto/prysm/v1alpha1/attestation/aggregation/attestations/export_test.go b/proto/prysm/v1alpha1/attestation/aggregation/attestations/export_test.go new file mode 100644 index 0000000000..5139348ac6 --- /dev/null +++ b/proto/prysm/v1alpha1/attestation/aggregation/attestations/export_test.go @@ -0,0 +1,11 @@ +package attestations + +// export attList for the attestations_test package +type AttList attList + +func (a AttList) ValidateForTesting() error { + return attList(a).validate() +} + +var RearrangeProcessedAttestations = rearrangeProcessedAttestations +var AggregateAttestations = aggregateAttestations diff --git a/proto/prysm/v1alpha1/attestation/aggregation/attestations/maxcover_test.go b/proto/prysm/v1alpha1/attestation/aggregation/attestations/maxcover_test.go index 42ac4849e9..cc03137b9f 100644 --- a/proto/prysm/v1alpha1/attestation/aggregation/attestations/maxcover_test.go +++ b/proto/prysm/v1alpha1/attestation/aggregation/attestations/maxcover_test.go @@ -1,4 +1,4 @@ -package attestations +package attestations_test import ( "testing" @@ -6,6 +6,7 @@ import ( "github.com/OffchainLabs/prysm/v6/crypto/bls" ethpb "github.com/OffchainLabs/prysm/v6/proto/prysm/v1alpha1" "github.com/OffchainLabs/prysm/v6/proto/prysm/v1alpha1/attestation/aggregation" + "github.com/OffchainLabs/prysm/v6/proto/prysm/v1alpha1/attestation/aggregation/attestations" "github.com/OffchainLabs/prysm/v6/testing/assert" "github.com/prysmaticlabs/go-bitfield" ) @@ -70,7 +71,7 @@ func TestAggregateAttestations_MaxCover_NewMaxCover(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assert.DeepEqual(t, tt.want, NewMaxCover(tt.args.atts)) + assert.DeepEqual(t, tt.want, attestations.NewMaxCover(tt.args.atts)) }) } } @@ -78,7 +79,7 @@ func TestAggregateAttestations_MaxCover_NewMaxCover(t *testing.T) { func TestAggregateAttestations_MaxCover_AttList_validate(t *testing.T) { tests := []struct { name string - atts attList + atts attestations.AttList wantedErr string }{ { @@ -88,17 +89,17 @@ func TestAggregateAttestations_MaxCover_AttList_validate(t *testing.T) { }, { name: "empty list", - atts: attList{}, + atts: attestations.AttList{}, wantedErr: "empty list", }, { name: "first bitlist is nil", - atts: attList{ðpb.Attestation{}}, + atts: attestations.AttList{ðpb.Attestation{}}, wantedErr: "bitlist cannot be nil or empty", }, { name: "non first bitlist is nil", - atts: attList{ + atts: attestations.AttList{ ðpb.Attestation{AggregationBits: bitfield.NewBitlist(64)}, ðpb.Attestation{}, }, @@ -106,14 +107,14 @@ func TestAggregateAttestations_MaxCover_AttList_validate(t *testing.T) { }, { name: "first bitlist is empty", - atts: attList{ + atts: attestations.AttList{ ðpb.Attestation{AggregationBits: bitfield.Bitlist{}}, }, wantedErr: "bitlist cannot be nil or empty", }, { name: "non first bitlist is empty", - atts: attList{ + atts: attestations.AttList{ ðpb.Attestation{AggregationBits: bitfield.NewBitlist(64)}, ðpb.Attestation{AggregationBits: bitfield.Bitlist{}}, }, @@ -121,7 +122,7 @@ func TestAggregateAttestations_MaxCover_AttList_validate(t *testing.T) { }, { name: "valid bitlists", - atts: attList{ + atts: attestations.AttList{ ðpb.Attestation{AggregationBits: bitfield.NewBitlist(64)}, ðpb.Attestation{AggregationBits: bitfield.NewBitlist(64)}, ðpb.Attestation{AggregationBits: bitfield.NewBitlist(64)}, @@ -131,7 +132,7 @@ func TestAggregateAttestations_MaxCover_AttList_validate(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := tt.atts.validate() + err := tt.atts.ValidateForTesting() if tt.wantedErr != "" { assert.ErrorContains(t, tt.wantedErr, err) } else { @@ -292,7 +293,7 @@ func TestAggregateAttestations_rearrangeProcessedAttestations(t *testing.T) { } } } - rearrangeProcessedAttestations(tt.atts, candidates, tt.keys) + attestations.RearrangeProcessedAttestations(tt.atts, candidates, tt.keys) assert.DeepEqual(t, tt.atts, tt.wantAtts) }) } @@ -312,7 +313,7 @@ func TestAggregateAttestations_aggregateAttestations(t *testing.T) { { name: "nil attestation", wantTargetIdx: 0, - wantErr: ErrInvalidAttestationCount.Error(), + wantErr: attestations.ErrInvalidAttestationCount.Error(), keys: []int{0, 1, 2}, }, { @@ -321,13 +322,13 @@ func TestAggregateAttestations_aggregateAttestations(t *testing.T) { ðpb.Attestation{}, }, wantTargetIdx: 0, - wantErr: ErrInvalidAttestationCount.Error(), + wantErr: attestations.ErrInvalidAttestationCount.Error(), keys: []int{0, 1, 2}, }, { name: "no keys", wantTargetIdx: 0, - wantErr: ErrInvalidAttestationCount.Error(), + wantErr: attestations.ErrInvalidAttestationCount.Error(), }, { name: "two attestations, none selected", @@ -336,7 +337,7 @@ func TestAggregateAttestations_aggregateAttestations(t *testing.T) { ðpb.Attestation{AggregationBits: bitfield.Bitlist{0x01}}, }, wantTargetIdx: 0, - wantErr: ErrInvalidAttestationCount.Error(), + wantErr: attestations.ErrInvalidAttestationCount.Error(), keys: []int{}, }, { @@ -346,7 +347,7 @@ func TestAggregateAttestations_aggregateAttestations(t *testing.T) { ðpb.Attestation{AggregationBits: bitfield.Bitlist{0x01}}, }, wantTargetIdx: 0, - wantErr: ErrInvalidAttestationCount.Error(), + wantErr: attestations.ErrInvalidAttestationCount.Error(), keys: []int{0}, }, { @@ -414,7 +415,7 @@ func TestAggregateAttestations_aggregateAttestations(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - gotTargetIdx, err := aggregateAttestations(tt.atts, tt.keys, tt.coverage) + gotTargetIdx, err := attestations.AggregateAttestations(tt.atts, tt.keys, tt.coverage) if tt.wantErr != "" { assert.ErrorContains(t, tt.wantErr, err) return diff --git a/proto/prysm/v1alpha1/attestation/aggregation/maxcover_bench_test.go b/proto/prysm/v1alpha1/attestation/aggregation/maxcover_bench_test.go index 358d173809..250551cff0 100644 --- a/proto/prysm/v1alpha1/attestation/aggregation/maxcover_bench_test.go +++ b/proto/prysm/v1alpha1/attestation/aggregation/maxcover_bench_test.go @@ -1,10 +1,11 @@ -package aggregation +package aggregation_test import ( "fmt" "testing" "github.com/OffchainLabs/prysm/v6/config/params" + "github.com/OffchainLabs/prysm/v6/proto/prysm/v1alpha1/attestation/aggregation" aggtesting "github.com/OffchainLabs/prysm/v6/proto/prysm/v1alpha1/attestation/aggregation/testing" "github.com/prysmaticlabs/go-bitfield" ) @@ -66,7 +67,7 @@ func BenchmarkMaxCoverProblem_MaxCover(b *testing.B) { }, } for _, tt := range tests { - name := fmt.Sprintf("%d_attestations_with_%d_bit(s)_set", tt.numCandidates, tt.numMarkedBits) + name := fmt.Sprintf("%d_aggregations_with_%d_bit(s)_set", tt.numCandidates, tt.numMarkedBits) b.Run(fmt.Sprintf("cur_%s", name), func(b *testing.B) { b.StopTimer() var bitlists []bitfield.Bitlist @@ -78,11 +79,11 @@ func BenchmarkMaxCoverProblem_MaxCover(b *testing.B) { } b.StartTimer() for i := 0; i < b.N; i++ { - candidates := make([]*MaxCoverCandidate, len(bitlists)) + candidates := make([]*aggregation.MaxCoverCandidate, len(bitlists)) for i := 0; i < len(bitlists); i++ { - candidates[i] = NewMaxCoverCandidate(i, &bitlists[i]) + candidates[i] = aggregation.NewMaxCoverCandidate(i, &bitlists[i]) } - mc := &MaxCoverProblem{Candidates: candidates} + mc := &aggregation.MaxCoverProblem{Candidates: candidates} _, err := mc.Cover(len(bitlists), tt.allowOverlaps) _ = err } @@ -98,7 +99,7 @@ func BenchmarkMaxCoverProblem_MaxCover(b *testing.B) { } b.StartTimer() for i := 0; i < b.N; i++ { - _, _, err := MaxCover(bitlists, len(bitlists), tt.allowOverlaps) + _, _, err := aggregation.MaxCover(bitlists, len(bitlists), tt.allowOverlaps) _ = err } }) diff --git a/proto/prysm/v1alpha1/attestation/attestation_utils.go b/proto/prysm/v1alpha1/attestation/attestation_utils.go index 4503881d22..94c93d9fae 100644 --- a/proto/prysm/v1alpha1/attestation/attestation_utils.go +++ b/proto/prysm/v1alpha1/attestation/attestation_utils.go @@ -11,7 +11,6 @@ import ( "sort" "github.com/OffchainLabs/prysm/v6/beacon-chain/core/signing" - "github.com/OffchainLabs/prysm/v6/config/params" "github.com/OffchainLabs/prysm/v6/consensus-types/primitives" "github.com/OffchainLabs/prysm/v6/crypto/bls" "github.com/OffchainLabs/prysm/v6/monitoring/tracing/trace" @@ -174,6 +173,10 @@ func VerifyIndexedAttestationSig(ctx context.Context, indexedAtt ethpb.IndexedAt // spec indexed attestation validation starting at Check if “indexed_attestation“ // comment and ends at Verify aggregate signature comment. // +// requires the caller to pass config params: +// MAX_VALIDATORS_PER_COMMITTEE = committeeValMax +// MAX_COMMITTEES_PER_SLOT = maxCommittees +// // Spec pseudocode definition: // // def is_valid_indexed_attestation(state: BeaconState, indexed_attestation: IndexedAttestation) -> bool: @@ -189,7 +192,7 @@ func VerifyIndexedAttestationSig(ctx context.Context, indexedAtt ethpb.IndexedAt // domain = get_domain(state, DOMAIN_BEACON_ATTESTER, indexed_attestation.data.target.epoch) // signing_root = compute_signing_root(indexed_attestation.data, domain) // return bls.FastAggregateVerify(pubkeys, signing_root, indexed_attestation.signature) -func IsValidAttestationIndices(ctx context.Context, indexedAttestation ethpb.IndexedAtt) error { +func IsValidAttestationIndices(ctx context.Context, indexedAttestation ethpb.IndexedAtt, committeeValMax, maxCommittees uint64) error { _, span := trace.StartSpan(ctx, "attestationutil.IsValidAttestationIndices") defer span.End() @@ -202,12 +205,12 @@ func IsValidAttestationIndices(ctx context.Context, indexedAttestation ethpb.Ind return errors.New("expected non-empty attesting indices") } if indexedAttestation.Version() < version.Electra { - maxLength := params.BeaconConfig().MaxValidatorsPerCommittee + maxLength := committeeValMax if uint64(len(indices)) > maxLength { return fmt.Errorf("validator indices count exceeds MAX_VALIDATORS_PER_COMMITTEE, %d > %d", len(indices), maxLength) } } else { - maxLength := params.BeaconConfig().MaxValidatorsPerCommittee * params.BeaconConfig().MaxCommitteesPerSlot + maxLength := committeeValMax * maxCommittees if uint64(len(indices)) > maxLength { return fmt.Errorf("validator indices count exceeds MAX_VALIDATORS_PER_COMMITTEE * MAX_COMMITTEES_PER_SLOT, %d > %d", len(indices), maxLength) } diff --git a/proto/prysm/v1alpha1/attestation/attestation_utils_test.go b/proto/prysm/v1alpha1/attestation/attestation_utils_test.go index 5b60b85006..d8ba8eff3f 100644 --- a/proto/prysm/v1alpha1/attestation/attestation_utils_test.go +++ b/proto/prysm/v1alpha1/attestation/attestation_utils_test.go @@ -204,7 +204,7 @@ func TestIsValidAttestationIndices(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := attestation.IsValidAttestationIndices(context.Background(), tt.att) + err := attestation.IsValidAttestationIndices(context.Background(), tt.att, params.BeaconConfig().MaxValidatorsPerCommittee, params.BeaconConfig().MaxCommitteesPerSlot) if tt.wantedErr != "" { assert.ErrorContains(t, tt.wantedErr, err) } else { @@ -240,7 +240,7 @@ func BenchmarkIsValidAttestationIndices(b *testing.B) { } b.ResetTimer() for i := 0; i < b.N; i++ { - if err := attestation.IsValidAttestationIndices(context.Background(), att); err != nil { + if err := attestation.IsValidAttestationIndices(context.Background(), att, params.BeaconConfig().MaxValidatorsPerCommittee, params.BeaconConfig().MaxCommitteesPerSlot); err != nil { require.NoError(b, err) } } diff --git a/testing/middleware/builder/builder.go b/testing/middleware/builder/builder.go index 402fc79ae7..048baf54c1 100644 --- a/testing/middleware/builder/builder.go +++ b/testing/middleware/builder/builder.go @@ -631,7 +631,7 @@ func (p *Builder) handleHeaderRequestElectra(w http.ResponseWriter) { http.Error(w, err.Error(), http.StatusInternalServerError) return } - requests, err := b.GetDecodedExecutionRequests() + requests, err := b.GetDecodedExecutionRequests(params.BeaconConfig().ExecutionRequestLimits()) if err != nil { p.cfg.logger.WithError(err).Error("Could not get decoded execution requests") http.Error(w, err.Error(), http.StatusInternalServerError)