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 fee_recipient returned from EE #3156

Closed
paulhauner opened this issue Apr 12, 2022 · 3 comments
Closed

Check fee_recipient returned from EE #3156

paulhauner opened this issue Apr 12, 2022 · 3 comments
Labels
bellatrix Required to support the Bellatrix Upgrade enhancement New feature or request v2.5.0 Required for Goerli merge release

Comments

@paulhauner
Copy link
Member

Description

It's possible that an EE will return a different fee recipient to the one provided (the suggested_fee_recipient) in get_payload. Whilst this is legal behaviour, I think it would make sense to raise some sort of log whenever the EE returns a different value. It might help users detect when their fees are being hijacked.

I'm not sure the log level, I'm tempted to go with an ERRO but I'm open to suggestion.

@paulhauner paulhauner added enhancement New feature or request bellatrix Required to support the Bellatrix Upgrade labels Apr 12, 2022
bors bot pushed a commit that referenced this issue Jun 3, 2022
## Issue Addressed

#3156

## Proposed Changes

Emit a `WARN` log whenever the value of `fee_recipient` as returned from the EE is different from the value of `suggested_fee_recipient` as set on the BN, for example by the `--suggested-fee-recipient` CLI flag.

## Additional Info

I have set the log level to `WARN` since it is legal behaviour (meaning it isn't really an error but is important to know when it is occurring).

If we feel like this behaviour is almost always undesired (caused by a misconfiguration or malicious EE) then an `ERRO` log would be more appropriate. Happy to change it in that case.
@michaelsproul
Copy link
Member

michaelsproul commented Jul 18, 2022

IMO we might want a flag that prevents the VC from signing any block with a divergent fee recipient. This would apply to blocks from builders too, which I think the check from #3202 does not. Filtering available blocks by their fee recipient would be superior, but I don't think the current builder APIs really allow that (unless you use multiple relays?).

(I've been thinking about this in the context of MEV smoothing: https://twitter.com/sproulM_/status/1548900364342009857)

@paulhauner paulhauner added the v2.5.0 Required for Goerli merge release label Jul 20, 2022
@realbigsean
Copy link
Member

IMO we might want a flag that prevents the VC from signing any block with a divergent fee recipient

I agree!

If we add this flag, we can make a fee recipient mismatch on a block from the builder API a BlockError::Recoverable error in the block service, so we will use full block endpoints as a fallback (a local execution engine, which will probably respect the fee recipient).

Filtering available blocks by their fee recipient would be superior, but I don't think the current builder APIs really allow that (unless you use multiple relays?).

Yeah, this one sounds like it'd be a good feature in mev-boost though.

bors bot pushed a commit that referenced this issue Jul 26, 2022
## Issue Addressed

Resolves #3267
Resolves #3156 

## Proposed Changes

- Move the log for fee recipient checks from proposer cache insertion into block proposal so we are directly checking what we get from the EE
- Only log when there is a discrepancy with the local EE, not when using the builder API. In the `builder-api` branch there is an `info` log when there is a discrepancy, I think it is more likely there will be a difference in fee recipient with the builder api because proposer payments might be made via a transaction in the block. Not really sure what patterns will become commong.
- Upgrade the log from a `warn` to an `error` - not actually sure which we want, but I think this is worth an error because the local EE with default transaction ordering I think should pretty much always use the provided fee recipient
- add a `strict-fee-recipient` flag to the VC so we only sign blocks with matching fee recipients. Falls back from the builder API to the local API if there is a discrepancy .




Co-authored-by: realbigsean <sean@sigmaprime.io>
@paulhauner
Copy link
Member Author

Resolved via #3363 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bellatrix Required to support the Bellatrix Upgrade enhancement New feature or request v2.5.0 Required for Goerli merge release
Projects
None yet
Development

No branches or pull requests

3 participants