Skip to content
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

Merged
merged 5 commits into from
Aug 5, 2024
Merged

Conversation

potuz
Copy link
Contributor

@potuz potuz commented Jul 11, 2024

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.

@potuz potuz requested a review from a team as a code owner July 11, 2024 01:18
@@ -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) {
Copy link
Contributor

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?

Copy link
Contributor Author

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.

beacon-chain/node/config.go Outdated Show resolved Hide resolved
@@ -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{
Copy link
Member

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.

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
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// 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,
Copy link
Member

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,
Copy link
Member

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>
@terencechain
Copy link
Member

Since we are not adding maxLocalBoostValue as @dataalways suggested in the post, we should look into deprecating local value boost and refer people to always use min builder diff instead. It can be follow up PR

terencechain
terencechain previously approved these changes Aug 5, 2024
@potuz potuz added this pull request to the merge queue Aug 5, 2024
Merged via the queue into develop with commit af098e7 Aug 5, 2024
16 of 17 checks passed
@potuz potuz deleted the min-bids-locally branch August 5, 2024 14:50
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants