Enforce per-builder bid limits from the block request

[terencechain]

• Builder-API bids are checked against their own entry's max execution payment, min bid, boost factor, and builder pubkeys
• P2P bids use the config-level min bid and boost, payments are trusted only from entries naming the builder key
• Removes the per-pubkey maxExecutionPayments map, the preferences push now sends each builder its own cap

Last of the stack replacing #17124, on top of #17373.

[james-prysm]

I don't think this is spec compliant for small bids under 100 gwei.

 t.Run("max boost prefers a sub-100-gwei bid over local", func(t *testing.T) {
    // builder_boost_factor=2**64-1 MUST mean "prefer this builder unless
    // unviable" for any nonzero bid, including one below 100 Gwei.
    win := &winningBuilderBid{
      bid:   newBid(99, 0, builderIdx),
      entry: &ethpb.BuilderEntry{MaxExecutionPayment: 0, BuilderBoostFactor: math.MaxUint64},
    }
    got, src, _ := bestBid(nil, localWithGwei(1), nil, win, nil)
    require.NotNil(t, got)
    require.Equal(t, bidSourceBuilderAPI, src)
  })

  t.Run("boost applies the full value below 100 gwei granularity", func(t *testing.T) {
    // 199 boosted by 50% is 199*50/100 = 99, which beats local 75.
    win := &winningBuilderBid{
      bid:   newBid(199, 0, builderIdx),
      entry: &ethpb.BuilderEntry{MaxExecutionPayment: 0, BuilderBoostFactor: 50},
    }
    got, src, _ := bestBid(nil, localWithGwei(75), nil, win, nil)
    require.NotNil(t, got)
    require.Equal(t, bidSourceBuilderAPI, src)
  })

these tests fail ( let me know if you think this is wrong)

[james-prysm]

that's unfortunate we needed to add this here, will be annoying to change down the road but ok with it for now i suppose

[james-prysm]

do we need to pass the head state around like this or just extract what we need? i see head state introduced in several areas here. more of a nit than anything

[terencechain]

I think head state is better than passing a pubkey lookup closure

[james-prysm]

This has a ton of parameters, wondering if it's better to have some wrapper object now or not

[terencechain]

I think is fine, it's an internal function with exactly one caller

[james-prysm]

because of this there's a potential to nil pointer I believe...

we need to do something like

default:
log.Errorf("Unsupported block version %s", version.String(blk.Version()))
if v.emitAccountMetrics {
ValidatorProposeFailVec.WithLabelValues(fmtKey).Inc()
}
return
}

otherwise log will fall through and cause a nil pointer here, you could also set it since it's trying to set it for be fore gloas?

you could also have a nil check gate

[james-prysm]

LGTM, will have a follow up pr for submit builder preferences timing