-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check locally for min-bid and min-bid-difference #14205
Conversation
@@ -96,6 +96,24 @@ func setExecutionData(ctx context.Context, blk interfaces.SignedBeaconBlock, loc | |||
// Compare payload values between local and builder. Default to the local value if it is higher. | |||
localValueGwei := primitives.WeiToGwei(local.Bid) | |||
builderValueGwei := primitives.WeiToGwei(bid.Value()) | |||
// Use local block if min bid is not attained | |||
if builderValueGwei < primitives.Gwei(params.BeaconConfig().MinBuilderBid) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about <= ? should be inclusive of the min right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, if the min bid is attained, we accept it.
@@ -31,6 +31,23 @@ var ( | |||
"Boost is an additional percentage to multiple local block value. Use builder block if: builder_bid_value * 100 > local_block_value * (local-block-value-boost + 100)", | |||
Value: 10, | |||
} | |||
// MinBuilderBid sets an absolute value for the builder bid that this | |||
// node will accept without reverting to local building | |||
MinBuilderBid = &cli.Uint64Flag{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 on adding this on the client side. I know the current MEV-Boost already handles this, but it is useful for epbs and future iterations as well.
cmd/beacon-chain/flags/base.go
Outdated
Value: 0, | ||
} | ||
// MinBuilderDiff sets an absolute value for the difference between the | ||
// builder's bid and the local block value that this node will accept |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// builder's bid and the local block value that this node will accept | |
// builder's bid and the local block value that this node will accept |
// Use local block if min difference is not attained | ||
if builderValueGwei < localValueGwei+primitives.Gwei(params.BeaconConfig().MinBuilderDiff) { | ||
log.WithFields(logrus.Fields{ | ||
"localGweiValue": localValueGwei, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also add a field for builderDiff
?
// Use local block if min bid is not attained | ||
if builderValueGwei < primitives.Gwei(params.BeaconConfig().MinBuilderBid) { | ||
log.WithFields(logrus.Fields{ | ||
"localGweiValue": localValueGwei, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of local value, we should log minBuilderBid
value here. Local value is not useful
Co-authored-by: terence <terence@prysmaticlabs.com>
Since we are not adding |
h/t @dataalways for these flags. Following up on suggestions from
https://hackmd.io/@dataalways/censorship-resistance-today
This PR gives the users the ability of specifying locally a min bid that they would accept from the builder's block and a min difference with the local payload that they would accept before reverting to local execution.