Skip to content

Conversation

@thegostep
Copy link
Contributor

No description provided.

- consensus client must implement [blind transaction signing](https://hackmd.io/@paulhauner/H1XifIQ_t#Change-1-Blind-Transaction-Signing)
- consensus client must implement [proposal promises](https://hackmd.io/@paulhauner/H1XifIQ_t#Change-2-Proposal-Promises)
- in event of mev-boost crash, consensus client must be able to bypass the middleware to reach a local or remote execution client

Copy link
Collaborator

Choose a reason for hiding this comment

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

consensus client must implement consensus_getfeerecipientsv1 no?


- `payloadHeader`: [`SignedMEVPayloadHeader`](#signedmevpayloadheader) - the invalid payload header signed by the offending relay.
- `payload`: [`ExecutionPayloadV1`](https://github.com/ethereum/consensus-specs/blob/v1.1.6/specs/merge/beacon-chain.md#executionpayload) - the invalid payload revealed to the network.
- `block`: [`SignedBlindedBeaconBlock`](#signedblindedbeaconblock) - the invalid beacon block signed and proposed by the validator.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe I'm missing something... What is SignedBlindedBeaconBlock useful for?

@tbenr
Copy link

tbenr commented Feb 24, 2022

Hi!
In Teku we are start working on mev-boost integration and we have a couple of points to ask\discuss:

  1. We are planning to start directly from M2 which to us seems like the authentication (the usage of AuthenticatedFeeRecipient) supersedes the ProposalPromises from M1. Do you think it is a good approach or is it too risky due to possible big changes ahead?

  2. seems like you are considering the introduction on a new namespace consensus_ that sounds like a new json-rpc endpoint. Consensus Clients do not exposes json-rpc, only REST APIs. We think that it will be wise to remain on REST and avoid to spin up a new server on a different port.

  3. AuthenticatedFeeRecipient: since it sign only the address, it will be valid forever. I feel like it's an insecure thing to have a kind of token that is valid forever. (ie. collecting them over time and use all at once toto DOS relays via relay_publishFeeRecipientsV1?)

  4. testing. Are some tooling available to setup a test environment with (mocked?) mev_boost to run the flows? Are there some how-to docs that can be shared?

  5. Are there some dates on when an M2 testnet might be started?

mev_boost->relays: relay_getValidatorStatusV1
```

1. on startup, mev-boost requests a list of [`AuthenticatedFeeRecipient`](#authenticatedfeerecipient) from the consensus client using a new [`consensus_getFeeRecipientsV1`](#consensus_getfeerecipientsv1) method.
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO this should be an mev-boost endpoint which consensus clients must POST to, something like builder_updateFeeRecipientsV1. I don't think this API should be made bidirectional if possible. Other endpoints in the builder API should respond with a specific status code if it doesn't yet have a suggested fee recipient. If a consensus client ever receives a payload header from mev-boost with an incorrect suggested_fee_recipient, they can update it with another call to builder_updateFeeRecipientsV1

@realbigsean
Copy link
Contributor

2. seems like you are considering the introduction on a new namespace `consensus_` that sounds like a new json-rpc endpoint. Consensus Clients do not exposes json-rpc, only REST APIs. We think that it will be wise to remain on REST and avoid to spin up a new server on a different port.

Missed this when I was reviewing! but had similar thoughts here: #34 (comment)

4. mev-boost must verify the signature and the merkle proof of the [`SignedMEVPayloadHeader`](#signedmevpayloadheader) received to ensure the signature matches the BLS key associated with the IP of the relay, has a matching `payloadId`, and has a valid fee recipient payment.
5. upon receiving a [`builder_getPayloadHeaderV1`](#builder_getpayloadheaderv1) request from the BN, mev-boost must return the [`ExecutionPayloadHeaderV1`](https://github.com/ethereum/consensus-specs/blob/v1.1.6/specs/merge/beacon-chain.md#executionpayloadheader) with the highest associated `feeRecipientBalance` and terminate the polling cycle. If no eligible payload is received from a relay, mev-boost must request and return a payload from the local execution client using [`engine_getPayloadV1`](#engine_getpayloadv1).
6. the BN must use the [`ExecutionPayloadHeaderV1`](https://github.com/ethereum/consensus-specs/blob/v1.1.6/specs/merge/beacon-chain.md#executionpayloadheader) received to assemble and sign a [`SignedBlindedBeaconBlock`](#signedblindedbeaconblock) and return it to mev-boost using [`builder_proposeBlindedBlockV1`](#builder_proposeblindedblockv1).
7. mev-boost must forward the [`SignedBlindedBeaconBlock`](#signedblindedbeaconblock) to all connected relays and attach the matching [`SignedMEVPayloadHeader`](#signedmevpayloadheader) using [`relay_proposeBlindedBlockV1`](#relay_proposeblindedblockv1) to inform the network of which relay created this payload.
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a big hole in the security of the validation/reputation system. If we want to be able to determine when a malicious relay delays or withholds sending its ExecutionPayloadV1, we have to rely on mev-boost delivering the SignedMEVPayloadHeaders to all relays in a timely manner. Timing then becomes a huge edge case, since it will be impossible to tell if the relay is delayed vs the validator is delayed, or if either party lies.

@metachris
Copy link
Collaborator

@thegostep could you remove the changes to README and milestone1 from this PR, to make it more focused? (I could also update it if you'd prefer)

@metachris metachris mentioned this pull request Apr 2, 2022
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@05b4fce). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #34   +/-   ##
=======================================
  Coverage        ?   59.48%           
=======================================
  Files           ?        8           
  Lines           ?      701           
  Branches        ?        0           
=======================================
  Hits            ?      417           
  Misses          ?      221           
  Partials        ?       63           
Flag Coverage Δ
unittests 59.48% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05b4fce...895e6c5. Read the comment docs.

@metachris
Copy link
Collaborator

As a heads-up - we're planning to close this PR and move the discussions into issues. This will go into future versions v0.3+

@come-maiz come-maiz closed this Apr 11, 2022
@metachris metachris deleted the thegostep/milestone-2 branch February 28, 2024 08:51
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.

8 participants