Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion beacon-chain/rpc/eth/config/handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ func TestGetSpec(t *testing.T) {
config.MaxBuildersPerWithdrawalsSweep = 112
config.MaxBuilderDepositRequestsPerPayload = 113
config.MaxBuilderExitRequestsPerPayload = 114
config.MaxBidsPerBuilder = 115
config.BLSWithdrawalPrefixByte = byte('b')
config.ETH1AddressWithdrawalPrefixByte = byte('c')
config.BuilderWithdrawalPrefixByte = byte('e')
Expand Down Expand Up @@ -244,7 +245,7 @@ func TestGetSpec(t *testing.T) {
require.NoError(t, json.Unmarshal(writer.Body.Bytes(), &resp))
data, ok := resp.Data.(map[string]any)
require.Equal(t, true, ok)
assert.Equal(t, 211, len(data))
assert.Equal(t, 212, len(data))
for k, v := range data {
t.Run(k, func(t *testing.T) {
switch k {
Expand Down Expand Up @@ -336,6 +337,8 @@ func TestGetSpec(t *testing.T) {
assert.Equal(t, "113", v)
case "MAX_BUILDER_EXIT_REQUESTS_PER_PAYLOAD":
assert.Equal(t, "114", v)
case "MAX_BIDS_PER_BUILDER":
assert.Equal(t, "115", v)
case "MIN_ANCHOR_POW_BLOCK_DIFFICULTY":
assert.Equal(t, "1000", v)
case "BLS_WITHDRAWAL_PREFIX":
Expand Down
36 changes: 30 additions & 6 deletions beacon-chain/sync/validate_execution_payload_bid.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/OffchainLabs/prysm/v7/beacon-chain/core/transition"
"github.com/OffchainLabs/prysm/v7/beacon-chain/p2p"
"github.com/OffchainLabs/prysm/v7/beacon-chain/verification"
"github.com/OffchainLabs/prysm/v7/config/params"
"github.com/OffchainLabs/prysm/v7/consensus-types/blocks"
"github.com/OffchainLabs/prysm/v7/consensus-types/interfaces"
"github.com/OffchainLabs/prysm/v7/consensus-types/primitives"
Expand Down Expand Up @@ -58,10 +59,14 @@ func (s *Service) validateExecutionPayloadBidGossip(ctx context.Context, pid pee
return pubsub.ValidationIgnore, err
}

// [IGNORE] this is the first signed bid seen with a valid signature from the given builder for this slot.
// [IGNORE] this is the first signed bid seen with a valid signature from the given builder for the tuple (bid.slot, bid.parent_block_hash, bid.parent_block_root).
// Cache is populated only after VerifySignature below; a hit here implies a valid-sig bid was already seen.
if s.hasSeenExecutionPayloadBidTuple(executionPayloadBidTupleKey(bid)) {
return pubsub.ValidationIgnore, nil
}
// [IGNORE] no more than MAX_BIDS_PER_BUILDER signed bids with a valid signature have been seen from the given builder for this slot.
builderKey := executionPayloadBidBuilderKey(bid.Slot(), bid.BuilderIndex())
if s.hasSeenExecutionPayloadBidBuilder(builderKey) {
if s.executionPayloadBidCount(builderKey) >= params.BeaconConfig().MaxBidsPerBuilder {
return pubsub.ValidationIgnore, nil
}

Expand Down Expand Up @@ -112,7 +117,7 @@ func (s *Service) validateExecutionPayloadBidGossip(ctx context.Context, pid pee
if err := v.VerifySignature(st); err != nil {
return pubsub.ValidationReject, err
}
s.setSeenExecutionPayloadBidBuilder(bid.Slot(), builderKey)
s.setSeenExecutionPayloadBid(bid.Slot(), executionPayloadBidTupleKey(bid), builderKey)
// [IGNORE] this bid is the highest value bid seen for the tuple (bid.slot, bid.parent_block_hash, bid.parent_block_root).
if !s.isHighestExecutionPayloadBid(bid) {
return pubsub.ValidationIgnore, nil
Expand Down Expand Up @@ -170,13 +175,32 @@ func executionPayloadBidBuilderKey(slot primitives.Slot, builderIndex primitives
return string(b)
}

func (s *Service) hasSeenExecutionPayloadBidBuilder(key string) bool {
func executionPayloadBidTupleKey(bid interfaces.ROExecutionPayloadBid) string {
parentHash := bid.ParentBlockHash()
parentRoot := bid.ParentBlockRoot()
return executionPayloadBidBuilderKey(bid.Slot(), bid.BuilderIndex()) + string(parentHash[:]) + string(parentRoot[:])
}

func (s *Service) hasSeenExecutionPayloadBidTuple(key string) bool {
_, seen := s.seenExecutionPayloadBidCache.Get(key)
return seen
}

func (s *Service) setSeenExecutionPayloadBidBuilder(slot primitives.Slot, key string) {
s.seenExecutionPayloadBidCache.Add(slot, key, true)
func (s *Service) executionPayloadBidCount(key string) uint64 {
v, ok := s.seenExecutionPayloadBidCache.Get(key)
if !ok {
return 0
}
count, ok := v.(uint64)
if !ok {
return 0
}
return count
}

func (s *Service) setSeenExecutionPayloadBid(slot primitives.Slot, tupleKey, builderKey string) {
s.seenExecutionPayloadBidCache.Add(slot, tupleKey, true)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we update the value of seenExecutionPayloadBidSize ?

s.seenExecutionPayloadBidCache.Add(slot, builderKey, s.executionPayloadBidCount(builderKey)+1)
}

// proposerDependentRoot returns the post-Fulu spec's proposer dep root for
Expand Down
55 changes: 44 additions & 11 deletions beacon-chain/sync/validate_execution_payload_bid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,24 +43,59 @@ func TestValidateExecutionPayloadBidGossip_InvalidTopic(t *testing.T) {
require.Equal(t, pubsub.ValidationReject, result)
}

func TestValidateExecutionPayloadBidGossip_AlreadySeenBuilder(t *testing.T) {
func TestValidateExecutionPayloadBidGossip_AlreadySeenTuple(t *testing.T) {
ctx := context.Background()
s, msg, signedBid := setupExecutionPayloadBidService(t)
s.newExecutionPayloadBidVerifier = testNewExecutionPayloadBidVerifier(mockExecutionPayloadBidVerifier{})

key := executionPayloadBidBuilderKey(signedBid.Message.Slot, signedBid.Message.BuilderIndex)
s.setSeenExecutionPayloadBidBuilder(signedBid.Message.Slot, key)
tupleKey := executionPayloadBidTupleKey(mustBid(t, signedBid))
builderKey := executionPayloadBidBuilderKey(signedBid.Message.Slot, signedBid.Message.BuilderIndex)
s.setSeenExecutionPayloadBid(signedBid.Message.Slot, tupleKey, builderKey)
result, err := s.validateExecutionPayloadBidGossip(ctx, "", msg)
require.NoError(t, err)
require.Equal(t, pubsub.ValidationIgnore, result)
}

func TestValidateExecutionPayloadBidGossip_MaxBidsPerBuilderReached(t *testing.T) {
ctx := context.Background()
s, msg, signedBid := setupExecutionPayloadBidService(t)
s.newExecutionPayloadBidVerifier = testNewExecutionPayloadBidVerifier(mockExecutionPayloadBidVerifier{})

builderKey := executionPayloadBidBuilderKey(signedBid.Message.Slot, signedBid.Message.BuilderIndex)
s.seenExecutionPayloadBidCache.Add(signedBid.Message.Slot, builderKey, params.BeaconConfig().MaxBidsPerBuilder)

result, err := s.validateExecutionPayloadBidGossip(ctx, "", msg)
require.NoError(t, err)
require.Equal(t, pubsub.ValidationIgnore, result)
}

func TestValidateExecutionPayloadBidGossip_SameBuilderDifferentParentAccepted(t *testing.T) {
ctx := context.Background()
s, msg, signedBid := setupExecutionPayloadBidService(t)
s.newExecutionPayloadBidVerifier = testNewExecutionPayloadBidVerifier(mockExecutionPayloadBidVerifier{})

result, err := s.validateExecutionPayloadBidGossip(ctx, "", msg)
require.NoError(t, err)
require.Equal(t, pubsub.ValidationAccept, result)

other := proto.Clone(signedBid).(*ethpb.SignedExecutionPayloadBid)
other.Message.ParentBlockHash = bytesutil.PadTo([]byte{0x09}, 32)
msg = executionPayloadBidToPubsub(t, s, s.cfg.p2p, other)
result, err = s.validateExecutionPayloadBidGossip(ctx, "", msg)
require.NoError(t, err)
require.Equal(t, pubsub.ValidationAccept, result)

builderKey := executionPayloadBidBuilderKey(signedBid.Message.Slot, signedBid.Message.BuilderIndex)
require.Equal(t, uint64(2), s.executionPayloadBidCount(builderKey))
}

// Dedup must short-circuit before every later check; duplicates pay only the cache lookup.
func TestValidateExecutionPayloadBidGossip_DedupShortCircuitsAllLaterChecks(t *testing.T) {
ctx := context.Background()
s, msg, signedBid := setupExecutionPayloadBidService(t)
key := executionPayloadBidBuilderKey(signedBid.Message.Slot, signedBid.Message.BuilderIndex)
s.setSeenExecutionPayloadBidBuilder(signedBid.Message.Slot, key)
tupleKey := executionPayloadBidTupleKey(mustBid(t, signedBid))
builderKey := executionPayloadBidBuilderKey(signedBid.Message.Slot, signedBid.Message.BuilderIndex)
s.setSeenExecutionPayloadBid(signedBid.Message.Slot, tupleKey, builderKey)
// Every subsequent verifier method would Reject/Ignore if it ran; the cache hit must skip them all.
s.newExecutionPayloadBidVerifier = testNewExecutionPayloadBidVerifier(mockExecutionPayloadBidVerifier{
errCurrentOrNextSlot: errors.New("slot"),
Expand Down Expand Up @@ -214,11 +249,10 @@ func TestValidateExecutionPayloadBidGossip_LowerOrEqualBidIgnored(t *testing.T)
result, err := s.validateExecutionPayloadBidGossip(ctx, "", msg)
require.NoError(t, err)
require.Equal(t, pubsub.ValidationIgnore, result)
builderKey := executionPayloadBidBuilderKey(signedBid.Message.Slot, signedBid.Message.BuilderIndex)
require.Equal(t, true, s.hasSeenExecutionPayloadBidBuilder(builderKey))
require.Equal(t, true, s.hasSeenExecutionPayloadBidTuple(executionPayloadBidTupleKey(mustBid(t, signedBid))))
}

func TestValidateExecutionPayloadBidGossip_LowerBidIgnoredStillMarksBuilderSeen(t *testing.T) {
func TestValidateExecutionPayloadBidGossip_LowerBidIgnoredStillMarksTupleSeen(t *testing.T) {
ctx := context.Background()
s, msg, signedBid := setupExecutionPayloadBidService(t)
s.newExecutionPayloadBidVerifier = testNewExecutionPayloadBidVerifier(mockExecutionPayloadBidVerifier{})
Expand All @@ -231,7 +265,7 @@ func TestValidateExecutionPayloadBidGossip_LowerBidIgnoredStillMarksBuilderSeen(
require.NoError(t, err)
require.Equal(t, pubsub.ValidationIgnore, result)

// If the lower valid bid did not mark the builder as seen, the same bid would
// If the lower valid bid did not mark the tuple as seen, the same bid would
// be accepted once the highest-bid cache is cleared.
s.highestExecutionPayloadBidCache = cache.NewHighestExecutionPayloadBidCache()
msg = executionPayloadBidToPubsub(t, s, s.cfg.p2p, signedBid)
Expand Down Expand Up @@ -268,8 +302,7 @@ func TestValidateExecutionPayloadBidGossip_HappyPath(t *testing.T) {
require.NoError(t, err)
require.Equal(t, pubsub.ValidationAccept, result)

builderKey := executionPayloadBidBuilderKey(signedBid.Message.Slot, signedBid.Message.BuilderIndex)
require.Equal(t, true, s.hasSeenExecutionPayloadBidBuilder(builderKey))
require.Equal(t, true, s.hasSeenExecutionPayloadBidTuple(executionPayloadBidTupleKey(mustBid(t, signedBid))))
got, ok := msg.ValidatorData.(*ethpb.SignedExecutionPayloadBid)
require.Equal(t, true, ok)
require.DeepEqual(t, signedBid, got)
Expand Down
3 changes: 3 additions & 0 deletions changelog/terence_bid-per-branch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Changed

- Allow builders to bid on multiple branches, deduplicate bids per `(slot, parent_block_hash, parent_block_root)` and cap at `MAX_BIDS_PER_BUILDER` per slot.
1 change: 1 addition & 0 deletions config/params/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ type BeaconChainConfig struct {
MaxPerEpochActivationChurnLimitGloas uint64 `yaml:"MAX_PER_EPOCH_ACTIVATION_CHURN_LIMIT_GLOAS" spec:"true"` // MaxPerEpochActivationChurnLimitGloas is the per-epoch cap on activation churn in Gloas (EIP-8061).
MaxBuilderDepositRequestsPerPayload uint64 `yaml:"MAX_BUILDER_DEPOSIT_REQUESTS_PER_PAYLOAD" spec:"true"` // MaxBuilderDepositRequestsPerPayload is the maximum number of builder deposit requests in each payload (EIP-8282).
MaxBuilderExitRequestsPerPayload uint64 `yaml:"MAX_BUILDER_EXIT_REQUESTS_PER_PAYLOAD" spec:"true"` // MaxBuilderExitRequestsPerPayload is the maximum number of builder exit requests in each payload (EIP-8282).
MaxBidsPerBuilder uint64 `yaml:"MAX_BIDS_PER_BUILDER" spec:"true"` // MaxBidsPerBuilder is the maximum number of signed bids gossiped per builder per slot.

// Networking Specific Parameters
MaxPayloadSize uint64 `yaml:"MAX_PAYLOAD_SIZE" spec:"true"` // MAX_PAYLOAD_SIZE is the maximum allowed size of uncompressed payload in gossip messages and rpc chunks.
Expand Down
1 change: 1 addition & 0 deletions config/params/mainnet_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ var mainnetBeaconConfig = &BeaconChainConfig{
MaxPerEpochActivationChurnLimitGloas: 256_000_000_000,
MaxBuilderDepositRequestsPerPayload: 256, // 2**8 (= 256)
MaxBuilderExitRequestsPerPayload: 16, // 2**4 (= 16)
MaxBidsPerBuilder: 3,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

constant seenExecutionPayloadBidSize = 1000 is still the same, is this churn 4x faster ok? or should we increase that number too?


// Values related to networking parameters.
MaxPayloadSize: 10 * 1 << 20, // 10 MiB
Expand Down
Loading