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

feat: set default builderBoostFactor to 90 #6568

Merged
merged 10 commits into from
Apr 10, 2024

Conversation

fredriksvantes
Copy link
Contributor

Motivation

Improve censorship resistance

Description

In order to help increase censorship resistance, I propose to change the default builderBoostFactor to 90. This means validators will prioritize local block building unless the bid from the external block builder is 10% or higher than what the validator would receive when building locally.

Looking at (stats), it can be seen that currently 63.7% of external builders are censoring transactions compared to 8.53% of validators who do local block building, so setting a minimum 10% as default can help increase the overall censorship resistance of the network.

It is still easy for users to opt out of this by manually setting the flag to 100, but many are likely to use the default which could help with censorship resistance for the network.

@fredriksvantes fredriksvantes requested a review from a team as a code owner March 19, 2024 20:44
@fredriksvantes fredriksvantes changed the title Set default builderBoostFactor to 90 feat: Set default builderBoostFactor to 90 Mar 19, 2024
@fredriksvantes
Copy link
Contributor Author

@philknows philknows changed the title feat: Set default builderBoostFactor to 90 feat: set default builderBoostFactor to 90 Mar 19, 2024
@nflaig
Copy link
Member

nflaig commented Mar 19, 2024

This means validators will prioritize local block building unless the bid from the external block builder is 10% or higher than what the validator would receive when building locally.

Setting a builder boost factor of 90 means the builder block value will be reduced by 10% but this is not the same as the increasing the local block value by 10%.

Let's assume a local block value of 100, if builder block has value of 110 (10% more) it will still pick local block

110 * 0.90 = 99 < 100

We have notes about the calculation in our docs.

I don't think it matters in this case as the difference is marginal.

Copy link

codecov bot commented Mar 19, 2024

Codecov Report

Merging #6568 (0c58151) into unstable (7fadd30) will decrease coverage by 0.01%.
The diff coverage is 20.00%.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #6568      +/-   ##
============================================
- Coverage     61.69%   61.68%   -0.01%     
============================================
  Files           556      556              
  Lines         58820    58829       +9     
  Branches       1887     1888       +1     
============================================
+ Hits          36287    36288       +1     
- Misses        22492    22500       +8     
  Partials         41       41              

ensi321
ensi321 previously approved these changes Mar 21, 2024
Copy link
Contributor

@ensi321 ensi321 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Would prefer holding off merging until ACD and see if testing is needed

@philknows
Copy link
Member

We should do a follow up PR on this to make users aware of the default setting on our documentation.

@philknows
Copy link
Member

philknows commented Mar 21, 2024

I have a question to this after giving it some more thought about how we deal with builder.selection UX. If I'm not mistaken, we need to have --builder.selection maxprofit in order to benefit from --builderBoostFactor X ?

This will essentially break the idea of having a maxprofit setting because if the default value is set to anything other than 100, it's really not maxprofit anymore. I would hate breaking this, but maybe we need an additional setting that is the actual default like default or boostdefault or something better. I'm just concerned that maxprofit is no longer accurately defining what it's doing and is misleading to users, even if it was disclaimed in the docs. We could also consider just having a default setting and getting rid of the maxprofit altogether.

@nflaig
Copy link
Member

nflaig commented Mar 21, 2024

@philknows that's a good point, maybe we add another alias that translates to boost factor 90?

Also on another thought, this change might be good for censorship resistance but it might also hurt the credible neutrality as something like this should be enforced via market forces or changes that do not reduce profits of stakers without them potentially being aware of it, not everybody closely reads release notes or follow the news.

So the question for me is also should the client software be opinionated about something like this because "we know what's best for the user (and the network)" or should the user decide themselves.

Might be better to push this similar to client diversity

@philknows
Copy link
Member

So the question for me is also should the client software be opinionated about something like this because "we know what's best for the user (and the network)" or should the user decide themselves.

To be fair, it'll be disclosed and the user has full ability to change their builder selection preference at free will. It's just the name maxprofit that is misleading if 90 is actually the default. I think --builder.selection default is ambiguous enough that if someone wanted to understand what "default" means, they would ideally look at the docs to understand what that means or what that number is. Our default is a suggestion based on our analysis of what is safe for the network (which is credibly neutral imo), but ultimately users can configure how they want, which is is the more important part.

@nflaig
Copy link
Member

nflaig commented Mar 22, 2024

our analysis of what is safe for the network

I don't think this analysis is profound at all. Censoring transactions is economically irrational and if more contracts (like Uniswap) would be on the OFAC list builders in US would be forced to relocate to a different jurisdiction or would become less competitive and lose their market share sooner or later.

Websites like censorship.pics claiming that a certain % of slots are being censored is also not correct, looking at Tornado cash router it doesn't even come close to a tx every slot. And if there is no tx like this in the mempool that the builder explicitly does not include then claiming the slot is being censored is just not correct.

So how exactly do we explain to the user that it was in the interest of the network to produce the block locally and missing out on some rewards? Maybe I am too much of a capitalist here but generally I think "state intervention" should be avoided if it's not critical.

@fredriksvantes
Copy link
Contributor Author

My thinking is that people have always needed to change default settings in order to not build local blocks 100% of the time (e.g. they have to explicitly specify a relay or mev-boost proxy), so max profit has never been the default setting for a validator. When they are viewing the documentation on how to configure lodestar to use an external builder, they would also see the default builder boost and how to adjust it.

I am primarily looking at this from the angle of making Ethereum more censorship resistant, but from a capitalist point of view I can also see some positive long term effects for validators (and the whole ecosystem). EIP-1559 was for example seen by many miners as short term negative as the basefee was lost, but the long term effects for the ecosystem appears to have been positive. With this proposed change, builders would need to submit blocks that are at least 10% more profitable than the local mempool, which - assuming builders have or can achieve that profit margin - could mean validators end up receiving higher rewards. Long term I also believe increased censorship resistance will have a positive effect on Ethereum.


With Lodestar's `--builder.selection` validator options, you can select:

- `maxprofit`: Default setting for Lodestar set at `--builder.boostFactor=100`. This default setting will always choose the more profitable block. Using this option, you may customize your `--builder.boostFactor` to your preference. Examples of its usage are below.
- `default`: Default setting for Lodestar set at `--builder.boostFactor=90`. This default setting will have a local block boost of ~10%. Using this option, you may customize your `--builder.boostFactor` to your preference. Examples of its usage are below.
Copy link
Member

Choose a reason for hiding this comment

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

The docs are a bit misleading here, maxprofit is actually not the default but executiononly is. We only produce builder blocks if --builder flag is set or if --builder.selection is set to maxprofit | builderalways | builderonly, see builder.selection docs

@nflaig
Copy link
Member

nflaig commented Mar 24, 2024

so max profit has never been the default setting for a validator

Our docs are bit misleading here, see #6568 (comment). It is not actually the default setting.

I am primarily looking at this from the angle of making Ethereum more censorship resistant

I appreciate the push for this and at first I also thought it is a good idea but after thinking about this a bit more it does not seem reasonable to change this right now. Censorship resistance is a really vague term imo, it hard to define when is Ethereum is or is not censorship resistant. But it is easy to tell if Ethereum is censoring or not, and looking at tornado cash txs, those are clearly not censored.

Imo what makes Ethereum censorship resistant is a large set of independent node operators which is the case for Ethereum. And I have high confidence in those operators (especially solo stakers) to do the right thing if there was meaningful / measurable censorship on Ethereum.

The change proposed here might increase the % of locally build blocks slightly and make the graphs on censorship.pics and other similar websites look prettier but other than that doesn't meaningfully change anything. So it seems more like a marketing measure than solving a real issue.

And don't get me wrong, building local blocks is great for many reasons, like not extracting value from user transactions which is also why I use a high min-bid myself. But I don't think right now it matters for censorship resistance.

@philknows
Copy link
Member

Just wanted to include this reference with data for some discussion about whether or not this is a meaningful change, which would require a bit of re-working on how to handle the UX.

https://hackmd.io/@dataalways/resilience

@nflaig
Copy link
Member

nflaig commented Mar 26, 2024

data for some discussion about whether or not this is a meaningful change

We also need to keep in mind that this is highly dependent on market conditions, in a bull market with high price volatility, the local block needs to be boosted much more as MEV increases disproportional compared to local block value.

Considering this, educating users about this setting as an alternative to min-bid seem to be the better way to approach this.

@philknows
Copy link
Member

Based on our standup on Apr 2, we decided that we will move forward with this with feedback from the community and with additional data that shows a very mild difference for inclusion. Although this will not end the conversation on censorship resistance, there is harder support for #6602 which data points will make more of a difference.

Once we figure out the alias option name because it's no longer maxprofit we can include this. Some options thrown in was recommended or default as an alias for 90. PR is currently set with default as the alias and if there's no additional comments to the alias, we can merge.

philknows
philknows previously approved these changes Apr 3, 2024
Copy link
Member

@nflaig nflaig left a comment

Choose a reason for hiding this comment

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

I think we should not change the maxprofit behavior and keep it as is. Instead I think it's better to add a new selection option and use that as default instead of maxprofit. This would ensure that users who have set this explicitly won't be affected and it's no longer a breaking change.

packages/cli/src/util/proposerConfig.ts Show resolved Hide resolved
Comment on lines 517 to 518
builderSelection = builderSelection ?? routes.validator.BuilderSelection.MaxProfit;
builderBoostFactor = builderBoostFactor ?? BigInt(100);
Copy link
Member

@nflaig nflaig Apr 7, 2024

Choose a reason for hiding this comment

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

I reverted the changes on the beacon node side as in case of Lodestar the boost factor is dictated by the validator client and setting a different default here is undocumented behavior we should avoid.

@@ -260,7 +260,7 @@ export const validatorOptions: CliCommandOptions<IValidatorCliArgs> = {
"builder.boostFactor": {
type: "string",
description:
"Percentage multiplier the block producing beacon node must apply to boost (>100) or dampen (<100) builder block value for selection against execution block. The multiplier is ignored if `--builder.selection` is set to anything other than `default` or `maxprofit`",
Copy link
Member

@nflaig nflaig Apr 7, 2024

Choose a reason for hiding this comment

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

Since we have the default for users that do not change the defaults it makes more sense to only apply the manually set boost factor in case of maxprofit as it is anyhow a setting for more advanced users.

@@ -10,12 +10,12 @@ proposer_config:
builder:
gas_limit: "35000000"
selection: "maxprofit"
boost_factor: "18446744073709551616"
boost_factor: "50"
Copy link
Member

Choose a reason for hiding this comment

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

Might encourage users to set it even lower :)

nflaig
nflaig previously approved these changes Apr 7, 2024
nflaig
nflaig previously approved these changes Apr 7, 2024
@philknows
Copy link
Member

Got a conflict here before merging.

Copy link
Member

@nflaig nflaig left a comment

Choose a reason for hiding this comment

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

Resolved merge conflicts, and are we ok with the semantics around this, i.e. boost factor can only be configured if maxprofit, and default uses the "default" boost factor we set?

Copy link
Contributor

@g11tech g11tech left a comment

Choose a reason for hiding this comment

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

lgtm

@nflaig nflaig merged commit 8675305 into ChainSafe:unstable Apr 10, 2024
20 checks passed
nflaig added a commit that referenced this pull request Apr 11, 2024
* Set default builderBoostFactor to 90

* Changing "MaxProfit" to "Default"

* Update tests

* Add default as new builder selection option

* Ignore manually set boost factor if selection is default

* Add ref to twitter thread

* Fix parse proposer config test

* Update comment

---------

Co-authored-by: Nico Flaig <[email protected]>
@wemeetagain
Copy link
Member

🎉 This PR is included in v1.18.0 🎉

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.

6 participants