I don't think the current CI/CD runs tests. We should include it

We use the standard library log.Println to log RPC server errors. I'd consider something more exotic like go-ethereum/log or sirupsen/logrus so we can do something like log.Warn...

This should probably tracked in the mergemock repo, but just adding it here for now so we don't lose it.

Is it possible to add something like a liveness endpoint we could poll with more frequency?

tersec on Feb 2 •

Along the lines of the comments about SignedBlindedBeaconBlock (#69), but here, the JSON-RPC and SSZ types are freely intermixed, with the sort-of-SSZ-type-looking SignedMEVPayloadHeader containing a MEVPayloadHeader the fields of which are a Data and Quantity (JSON-RPC serialization types), which itself contains a ExecutionPayloadHeaderV1(a consensus layer SSZ type, defined using the SSZ-serialization-defined Hash32, ExecutionAddress, Bytes32, uint64, etc types). It's not internally consistent.

Implementing this might involve drawing out, at each layer, the dual JSON-RPC form of SignedMEVPayloadHeader, the dual SSZ form of MEVPayloadHeader (because the sort-of-SSZ-defined SignedMEVPayloadHeader either has to be read as a purely JSON-RPC object, or if not, for all its component parts to have consensus layer-like definitions in addition to the purely JSON-RPC definitions here), and likewise but inverted for the nominally-natively-SSZ ExecutionPayloadHeaderV1.

For example, one might imagine the CL/SSZ MEVPayloadHeader:

class MEVPayloadHeader(Container):
payloadHeader: ExecutionPayloadHeaderV1
feeRecipient: ExecutionAddress
feeRecipientBalance: uint256

https://github.com/realbigsean/lighthouse/blob/mev-lighthouse/beacon_node/execution_layer/src/engine_api/json_structures.rs fills this out with some gaps, but none of it is directly specified and there are various points of ambiguity.

If anyone but given slot's proposer is allowed to call builder_getHeader and receive best header, the header auction is no longer sealed and builders will try and bid for the lowest possible transfer to the proposer.

Only proposer for the given slot should be able to call builder_getHeader, this ensures sealed bid as long as relay can be trusted (is not submitting the bids off chain).

TLDR: We'd like to consider whether it would make sense for builders to bid unconditionally for their header to be selected by the proposer. This would imply the proposer gets paid regardless of whether the body gets revealed. We care about this because it maps onto the current spec of in-protocol PBS more closely (further helping test PBS with MEV-Boost), because it is more favorable to proposers, and because it places less trust on relays.

Context
In the current version of MEV-Boost:

• searchers submit bundles to builders
• builders build blocks with these bundles and public mempool transactions
• builders send their blocks and bids (body, header, bid) to relays
• relays check the validity of and store builder blocks, get pinged by the proposer and share only the headers of all the valid blocks they have alongside their respective bids (header,bid)
• the proposer selects the header with the highest bid and signs on it within the beacon block they propose
• once the relay knows the proposer has signed on this header, the block body of the associated header is revealed by the relay. The payment is contained within the body.

Note:
This commit-reveal scheme was chosen in order to ensure there is no trust assumption placed on validators, making accessing MEV revenue via MEV-Boost completely permissionless. This is the 'trusted relay' model in this document: https://hackmd.io/8cUfu-HKQuyYjWk-f9DVuw. This Github issue doesn't discuss in detail why this solution was chosen. Please refer to a presentation given at EthStaker for more detail for why ensuring all validators have access to MEV revenue is especially important in PoS Ethereum: https://youtu.be/GJwS7VF40wk?t=23292.

Unconditional payments
In the current model, the proposer only gets paid if the block body whose header they've signed on is revealed by the relay. This means the proposer is at the mercy of the relay. If the relay doesn't reveal the body, then the proposer gets slashed from proposing an empty execution block loses protocol rewards they would've gotten from proposing a block as well as the MEV rewards (incl. transaction tips) that are in the block body. (edit: correcting previous statement that proposer gets slashed from proposing an empty block).

The relay is therefore trusted by 1) the builders who submit their blocks to it, 2) the validators who sign on one of the headers they receive from the relays.

We would like to consider an alternative system where builders pay unconditionally for their bid. In this system:

• searchers submit bundles to builders
• builders build blocks with these bundles and public mempool transactions
• builders send their (header,bid) pairs to relays
• relays store builder's (header,bid) pairs as well the money needed to fullfill the bid
• relays get pinged by the proposer and release each (header,bids) pairs it has for the current slot.
• the proposer selects the header with the highest bid and signs on it within the beacon block they propose
• as soon as the relay sees the header, it releases the payment to the proposer, unconditionally of the body being revealed.
• the block body of the associated header is revealed by the builder who is now incentivized to do so since they've already paid for it.

How does this differ from the current system?

• The relay now essentially acts as a payment escrow.
• The relay cannot see the block bodies and does not check for the validity of the bodies. This reduces the trust builders and proposers need to have in the relay.
• The proposer now gets paid regardless of whether the body gets revealed.
• The builders are now responsible to reveal their header's body, and they are incentivized to do so since they've already paid for it. Similarly, they are incentivized to make sure their body is valid.
• This shifts the attack vector to builders, who can now be griefed by proposers who can pick a header late enough such that a builder doesn't have time to reveal the body before the appropriate time window but a relay has enough time to see it and release the payment.

A document outlining this alternative was written by @thegostep here: https://hackmd.io/@flashbots/Skc0vuyCt

Outstanding questions

1. How does unconditional payment work with multiple relays? In other words, who does the builder send money to if it sends its header,bid pair to two relays? (h/t @lightclient for asking this question)
2. Does this system put a prohibitively higher capital requirements on builders who now need to put up money upfront?
3. Is a griefing vector from proposers acceptable?
4. Which solution is better between the current design and unconditional payments? Do we fully understand the trade-off space?

We open this up for discussion and look forward to your questions and comments 🤗

https://github.com/flashbots/flashbots-repository-template/blob/main/CODE_OF_CONDUCT.md

Some consensus clients have expressed the desire of implementing the mev-boost logic as a module to the consensus client directly. This would generally be good for security as it would provide greater diversity in implementation, but it may also make it more difficult to publish updates to mev-boost in the future. My recommendation would be to continue down the path of using mev-boost as a canonical implementation, but acknowledge this future direction.

One architecture level change which can be implemented now is the removal of the use of engine_getPayloadV1 from the block proposal process. This can be done because consensus clients are expected to fallback to a local execution client only if no payloads are returned from relays.

This change requires updating the specs, updating the mev-boost implementation and notifying client implementation teams.

Description:
In the current design of MEV-Boost, relays release (header,bid) pairs to all instances of mev-boost that pings it (please correct me if this is wrong). This means the pairs are public information that can be used as input for auction participants.

For example, a builder could watch the 'mempool' of (header,bid) pairs, see what the maximum bid and compare it to the maximum they're willing to bid. If they can bid higher than max_current_bid then they just add 1 wei to it.

If you assume this behaviour from multiple builders, we have a game where:

• the optimal bidding strategy is to bid by increments and always bid the smallest increment allowed more than the current 'winning' builder.
• the optimal bidding strategy is to wait until the last possible moment to bid as to not reveal the information to other bidders.

Questions:

1. Should we make the 'mempool' private only to the proposer of a slot? Technically this would require proposers to send an authenticated message to the relays, proving that they are the proposers of the current slot. How technically difficult is this?
2. Is this PGA behaviour problematic? Does it overwhelm the communication network or pollute its bandwidth with incremental bids from builders or burst of bids close to the deadline?
3. What does this non-sealed bid auction look like?
4. Is this a worry at all since in a repeated game, this should trend towards bidding max of the opportunity?

The original spec had as step 3 "mev-boost begins polling connected relays for their SignedMEVPayloadHeader using relay_getPayloadHeaderV1 requests."

The current implementation does not poll, just when it receives builder_getPayloadHeaderV1 it queries all the relays. We removed the polling now from the spec, but want to open an issue for discussion.

In #20 (comment) @thegostep suggests:

Should this just be the full PayloadAttributesV1 to avoid confusion?

this was supported by @terencechain and @realbigsean with 👍 , so lets just do it.

Reported from Sean in discord:

The big outstanding issue I'm having is that the initial payload header response from MEV-boost is something like this:

{
  "jsonrpc": "2.0",
  "result": {
    "parentHash": "0x29c12a5f851fa137cc967b8e7084605a9b2b0f3b92d7bc4ae05ac3158f5fd38d",
    "feeRecipient": "0x0000000000000000000000000000000000000001",
    "stateRoot": "0x1014a93b39908122be4202db72ba66cf287be6c6820473f5ad62cc67d1692407",
    "receiptsRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
    "logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",
    "random": "0x29c12a5f851fa137cc967b8e7084605a9b2b0f3b92d7bc4ae05ac3158f5fd38d",
    "blockNumber": "0x1",
    "gasLimit": "0x1c9c380",
    "gasUsed": "0x0",
    "timestamp": "0x61ba2559",
    "extraData": "0x",
    "baseFeePerGas": "0x7",
    "blockHash": "0x4fe17cc83472a6887c9216b04c4cc7fb2c8a1f99f94fdc239fe7e0007e62f6db",
    "transactionsRoot": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421"
  },
  "error": null,
  "id": 1
}

But the ExecutionPayload that's returned by proposeBlindedBlock looks like this:

{
  "jsonrpc": "2.0",
  "result": {
    "parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
    "feeRecipient": "0x0000000000000000000000000000000000000000",
    "stateRoot": "0x0000000000000000000000000000000000000000000000000000000000000000",
    "receiptsRoot": "0x0000000000000000000000000000000000000000000000000000000000000000",
    "logsBloom": "0x",
    "random": "0x0000000000000000000000000000000000000000000000000000000000000000",
    "blockNumber": "0x0",
    "gasLimit": "0x0",
    "gasUsed": "0x0",
    "timestamp": "0x0",
    "extraData": "0x",
    "baseFeePerGas": null,
    "blockHash": "0x0000000000000000000000000000000000000000000000000000000000000000",
    "transactionsRoot": "0x0000000000000000000000000000000000000000000000000000000000000000"
  },
  "error": null,
  "id": 1
}

Looks like everything is zeroed out, baseFeePerGas is null, which we can't parse, and there's a transactionsRoot as opposed to a transactions array

Pros:

• go-ethereum/log seems to be the standard around Ethereum related projects
• also used by mergemock, and various Flashbots projects
• it has pretty much the same features as logrus
• we already use go-ethereum as dependency, and we could simply remove the logrus dependency

wdyt?

what

The current spec for builder_setFeeRecipientV1 uses Unix timestamps to order updates from validators and the method requirements suggest that builders will respond with an error in the event that mev-boost suggests an update prior to one the builder already knows about.

I'd suggest instead to use sequence numbers in place of timestamps. A sequence number here is simply a monotonically increasing number the validator provides to order their updates. We can keep the uint64 type and validators are free to use whatever policy they choose but the obvious one is to just start at 0 and increment by one for each update.

why

clock skew

One reason to do this is it eliminates issues of clock skew from this entire API path (from validator to builder to possibly relays). If a validator is having issues with their local clock then they could accidentally confuse builders on which feeRecipient address to use with a potential loss in funds (redirecting fees to an outdated address).

You may consider the builder_setFeeRecipientV1 messages to be infrequent enough that we wouldn't see enough clock skew for this to be an issue in practice. However, relying on that invariant constrains the builder design space (as it assumes a particular usage pattern) and in doing so raises the barrier to entry for builders of all types.

I'll also point to the roughtime bug on Medalla during an early beacon chain testnet so these issues are not as rare as you may think.

looking forward

There is another reason to make this change as well in that it mirrors the consensus networking protocol where we have a similar idea of versioning which attestation subnets a validator is currently listening to (cf. https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/p2p-interface.md#metadata). Making this change now will make it forward compatible with any migration to a more distributed architecture leveraging peer-to-peer networking (e.g. putting recipient updates into a DHT or others in #92). Moreover, the symmetry to these existing concepts means lower conceptual load for implementers, reviewers and others looking at this corner of the ethereum software stack. Use of sequence numbers (over e.g. timestamps) is important in these contexts for the same reasons as clock skew, exaggerated by the fact that you have even higher asynchrony (and resulting partial views) in these more distributed settings.

If there is support for this change, I'm happy to make a PR to update the code + specs.

Currently, the store is accumulating but not freeing.

Braindump for a wishlist of functionality:

• setFeeRecipient, to allow clients building only for a specific validator (see also #82)
• polling (with short or no delay), with the relays handling
• streaming of headers from relay to mev-boost

sequenceDiagram
    participant consensus
    participant mev_boost
    participant relays

    Note over consensus: startup
    consensus->>+mev_boost: builder_setFeeRecipientV1
    mev_boost->>relays: relay_setFeeRecipientV1    

    Note over mev_boost: poll relays always
    mev_boost->>relays: relay_getPayloadHeaders
    relays-->>mev_boost: return headers for all possible forks

    Note over mev_boost: start a push subscription
    mev_boost->>relays: relay_subscribePayloadHeaders
    relays-->>mev_boost: stream new headers as they get build

    Note over consensus: wait for allocated slot
    consensus->>+mev_boost: builder_getPayloadHeaderV1

    Note over mev_boost: select most valuable cached payload
    mev_boost-->>-consensus: builder_getPayloadHeaderV1 response

    Note over consensus: sign the block
    consensus->>+mev_boost: builder_proposeBlindedBlockV1

    Note over mev_boost: identify payload source
    mev_boost->>relays: relay_proposeBlindedBlockV1

    Note over relays: validate signature
    relays-->>mev_boost: relay_proposeBlindedBlockV1 response
    mev_boost-->>-consensus: builder_proposeBlindedBlockV1 response

ping @jparyani @lightclient @terencechain @ElOpio

How will MEV-boost handle auth for CL<>EL communication? https://github.com/ethereum/execution-apis/blob/main/src/engine/authentication.md

Seems like most requests could just be forwarded, but engine_forkchoiceUpdatedV1 will need to be sent to the relay. So I think MEV-boost will need to accept a jwt-secret to let it decrypt engine_forkchoiceUpdatedV1 before sending it to the relay.

Currently engine_forkchoiceUpdatedV1 is used to communicate to relays the feeRecipient that a validator will be using. As a side effect, a lot of unnecessary information is communicated to the relays.

I propose two potential alternatives:

1. builder_getPayloadHeader(hash, feeRecipient) -> PayloadHeader
2. builder_preparePayload(hash, feeRecipient) -> uint64 + builder_getPayloadHeader(payloadId) -> PayloadHeader

I don't know if I have enough of an understanding of the latency of 1) to choose it over 2), but it is naively my preference.

• add relaySig as return value to builder_getPayloadHeaderV1
• define root of ExecutionPayloadHeader message as SSZ object root
• verify relaySig matches root of submitted ExecutionPayloadHeader
• add relaySig as parameter to builder_proposeBlindedBlockV1
• verify transactions of ExecutionPayload object returned by builder_proposeBlindedBlockV1 matches transactions_root before returning to consensus client

Played with builder_getPayloadHeaderV1 with the new relay endpoint, tried invalid payload ID as an experiment. Unfortunately, I don't think the error response is getting appropriately parsed as we are getting "unexpected end of JSON input."

./mev-boost -relayUrl https://relay-kintsugi.flashbots.net

curl -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"builder_getPayloadHeaderV1","params":["0x1"]}' http://localhost:18550

ERRO[0002] Could not parse response                      err="unexpected end of JSON input" method=engine_getPayloadV1 prefix=lib/service

Description:
Cancellations are a useful feature that allow individuals to cancel transactions they submitted before they land on chain. This feature is particularly useful for people executing cross-domain arbitrage.

There are 2 possible 'types' of cancellations:

• searchers cancelling their bundles to the builder
• the builder cancelling their block to the relay/proposer

The former is introduced by the builder, in which case this doesn't affect mev-boost code. The latter would concern mev-boost. We focus on this second case.

Naturally, no cancellation is possible after a header has been signed upon by the proposer - aside from a 'brute-force cancellation' where a relay does not reveal the body of a block.

Questions

• Does allowing cancellations at the mev-boost level opens up a DoS vector for the relay?
  • The DoS here would be a builder submitting blocks, relay checking validity and then receiving cancellation request for it. There is a likely asymmetry between compute needed to cancel a body and re-submit a new one vs checking the validity of a body
• Does allowing cancellations mean bidding is now messed up? please refer to issue #112 for more details here.
  • In particular, I worry this means someone could bid very high, scare other builders from participating in a round then cancel right before the end. This feel pretty harmless though but I would like to surface the concern.

The GetHeaderV1 method is crashing when nil parameter is provided as the blockHash value

Description

I was reading the code and testing the project when I realized that the GetHeaderV1 method takes a pointer as parameter, the blockHash.

Here is the original function code :

// GetHeaderV1 TODO
func (m *BoostService) GetHeaderV1(ctx context.Context, blockHash *string) (*GetHeaderResponse, error) {
	method := "builder_getHeaderV1"
	logMethod := m.log.WithField("method", method)

	if len(*blockHash) != 66 {
		return nil, fmt.Errorf("invalid block hash: %s", *blockHash)
	}
        // The rest of the function
}

Dereferencing the pointer without pre-verification of its content (here a non-nil check) causes the method call to crash when providing a nil value for the blockHash parameter.

I know that the function is executed in a goroutine, but still, I think it impacts stability.

Your environment

• OS and version: macOS Monterey v12.3.1
• branch/commit hash that causes this issue: 1108378

Steps to reproduce

• How to reproduce this issue ?
  
  As written in the project's README, I built mev-boost using the following command :

make build

And executed the binary using (don't mind the port, the default one was used by another program):

./mev-boost -port 8080

Then, in a new terminal, I ran the following curl command :

curl -X POST http://127.0.0.1:8080/ -H 'Content-Type: application/json' --data '{"jsonrpc":"2.0","method":"builder_getHeaderV1","params":[],"id":1}'

To confirm the behaviour, I also wrote the following test before patching the function :

func TestE2E_GetHeaderError(t *testing.T) {
	relay1 := setupMockRelay()
	server, err := newTestBoostRPCServer([]string{relay1.URL})
	require.Nil(t, err, err)
	defer server.Stop()

	client := gethRpc.DialInProc(server)
	defer client.Close()

	res := new(GetHeaderResponse)
	err = client.Call(&res, "builder_getHeaderV1", nil)
	require.Error(t, err)
}

And both of them lead to a goroutine crash.

• Where the issue is ?
  
  As mentionned in the description, the issue is caused by dereferencing a nil pointer.

Expected behaviour

The server should reject my invalid request with an error message (I suppose ?).

Actual behaviour

The goroutine handling the request crashed. Check out the logs for more info.

Logs

With the CURL command :

INFO[0000] mev-boost dev                                 prefix=cmd/mev-boost
INFO[0000] listening on  localhost:8080                  prefix=cmd/mev-boost
ERROR[04-24|01:11:44.095] RPC method builder_getHeaderV1 crashed: runtime error: invalid memory address or nil pointer dereference
goroutine 40 [running]:
github.com/ethereum/go-ethereum/rpc.(*callback).call.func1()
        /Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/[email protected]/rpc/service.go:200 +0x74
panic({0x10123db80, 0x101540e20})
        /Users/ptitluca/go/go1.18.1/src/runtime/panic.go:838 +0x204
github.com/flashbots/mev-boost/lib.(*BoostService).GetHeaderV1(0x14000236180, {0x1012b5028?, 0x14000222f40}, 0x0)
        /Users/ptitluca/GolandProjects/mev-boost/lib/service.go:144 +0xe8
reflect.Value.call({0x14000236200?, 0x1400021e198?, 0x1017b8a68?}, {0x10112b5f4, 0x4}, {0x14000228960, 0x3, 0x1010fcc6c?})
        /Users/ptitluca/go/go1.18.1/src/reflect/value.go:556 +0x5e4
reflect.Value.Call({0x14000236200?, 0x1400021e198?, 0x14000235968?}, {0x14000228960, 0x3, 0x3})
        /Users/ptitluca/go/go1.18.1/src/reflect/value.go:339 +0x98
github.com/ethereum/go-ethereum/rpc.(*callback).call(0x1400020eae0, {0x1012b5028?, 0x14000222f40}, {0x14000232228, 0x13}, {0x14000230420, 0x1, 0x1010f7370?})
        /Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/[email protected]/rpc/service.go:206 +0x38c
github.com/ethereum/go-ethereum/rpc.(*handler).runMethod(0x14000235958?, {0x1012b5028?, 0x14000222f40?}, 0x14000293d50, 0x1?, {0x14000230420?, 0x100010000?, 0x0?})
        /Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/[email protected]/rpc/handler.go:389 +0x44
github.com/ethereum/go-ethereum/rpc.(*handler).handleCall(0x1400023c240, 0x1400027f2c0, 0x14000293d50)
        /Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/[email protected]/rpc/handler.go:337 +0x1dc
github.com/ethereum/go-ethereum/rpc.(*handler).handleCallMsg(0x1400023c240, 0x1400027f2c0?, 0x14000293d50)
        /Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/[email protected]/rpc/handler.go:298 +0x80
github.com/ethereum/go-ethereum/rpc.(*handler).handleMsg.func1(0x1400027f2c0)
        /Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/[email protected]/rpc/handler.go:139 +0x38
github.com/ethereum/go-ethereum/rpc.(*handler).startCallProc.func1()
        /Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/[email protected]/rpc/handler.go:226 +0xc4
created by github.com/ethereum/go-ethereum/rpc.(*handler).startCallProc
        /Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/[email protected]/rpc/handler.go:222 +0x90
 
WARN [04-24|01:11:44.097] Served builder_getHeaderV1               conn=127.0.0.1:50085 reqid=1 t=1.580458ms err="method handler crashed"

While running the test :

=== RUN   TestE2E_GetHeaderError
ERROR[04-24|01:13:57.100] RPC method builder_getHeaderV1 crashed: runtime error: invalid memory address or nil pointer dereference
goroutine 51 [running]:
github.com/ethereum/go-ethereum/rpc.(*callback).call.func1()
	/Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/[email protected]/rpc/service.go:200 +0x74
panic({0x1052defe0, 0x105640c00})
	/Users/ptitluca/go/go1.18.1/src/runtime/panic.go:838 +0x204
github.com/flashbots/mev-boost/lib.(*BoostService).GetHeaderV1(0x140000ac500, {0x1053691a0?, 0x14000312080}, 0x0)
	/Users/ptitluca/GolandProjects/mev-boost/lib/service.go:144 +0xe8
reflect.Value.call({0x140000ac580?, 0x140000a41f8?, 0x105990f18?}, {0x1051ae701, 0x4}, {0x1400031e050, 0x3, 0x105145f4c?})
	/Users/ptitluca/go/go1.18.1/src/reflect/value.go:556 +0x5e4
reflect.Value.Call({0x140000ac580?, 0x140000a41f8?, 0x140003242d8?}, {0x1400031e050, 0x3, 0x3})
	/Users/ptitluca/go/go1.18.1/src/reflect/value.go:339 +0x98
github.com/ethereum/go-ethereum/rpc.(*callback).call(0x1400009cd80, {0x1053691a0?, 0x14000312080}, {0x14000334000, 0x13}, {0x1400031a180, 0x1, 0x105140430?})
	/Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/[email protected]/rpc/service.go:206 +0x38c
github.com/ethereum/go-ethereum/rpc.(*handler).runMethod(0x140003242d0?, {0x1053691a0?, 0x14000312080?}, 0x14000332000, 0x1?, {0x1400031a180?, 0x60000101010000?, 0x12c844800?})
	/Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/[email protected]/rpc/handler.go:389 +0x44
github.com/ethereum/go-ethereum/rpc.(*handler).handleCall(0x140001402d0, 0x1400030e360, 0x14000332000)
	/Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/[email protected]/rpc/handler.go:337 +0x1dc
github.com/ethereum/go-ethereum/rpc.(*handler).handleCallMsg(0x140001402d0, 0x1400030e360?, 0x14000332000)
	/Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/[email protected]/rpc/handler.go:298 +0x80
github.com/ethereum/go-ethereum/rpc.(*handler).handleMsg.func1(0x1400030e360)
	/Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/[email protected]/rpc/handler.go:139 +0x38
github.com/ethereum/go-ethereum/rpc.(*handler).startCallProc.func1()
	/Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/[email protected]/rpc/handler.go:226 +0xc4
created by github.com/ethereum/go-ethereum/rpc.(*handler).startCallProc
	/Users/ptitluca/go/pkg/mod/github.com/!marius!van!der!wijden/[email protected]/rpc/handler.go:222 +0x90

Proposed solution

A verification should be made before dereferencing the pointer to blockHash, and return an error if no such value has been provided.
I have created a PR for this in #101.

engine_getPayloadV1 gets requested to both relay and execution. Instead, I think we can save a request for a happy path and only call execution part of failing over

https://github.com/flashbots/mev-boost/blob/master/lib/service.go#L247

From @djrtwo in https://notes.ethereum.org/WX1hwcMRRzy5oh-zHyzqOA?view#step-3

in step (3) there is a missing (implicit) set of calls to relay_getPayloadHeaderV1 that mev-boost makes to each relayer it had previously initiated a build process with. suggest saying this explicitly in that step

engine_executePayloadV1 is currently not used, should we just remove engine_executePayloadV1 and ExecutePayloadV1?

Tasks:

• #136
• forwarding error codes from the relay (clarify error handling if 1/n or n/n relays fail, even with different results?)

wip:

• use correct error payload format - #166
• Store registerValidator payloads and resend to relays at regular intervals #149

done:

• verify signatures/payloads: #138
• require relay pubkey: #143
• check relays on startup: #140

MIT license will make it easier for other projects to integrate mev-boost.

@terencechain are you okay with relicensing your contributions under the MIT license?

(and fix the typo on the response at the end of the sequence diagram)

I will rename the default branch from master to main.

Create new issues for future goals, including those formulated in #34, in particular milestone2.md

Those include:

1. Merkle proof of payment (see milestone-2.md#accountproof)
2. Payload validity fraud proof (see #88 )
3. Validator authentication (partly covered with setFeeRecipient in current v0.2 spec. Perhaps open is how to allow multiple validators use one CL client 🤔 )

Maybe more / others

🙏

The wiki doesn't seem to support mermaid.
We should copy there the diagram of the interactions. I'm adding this here just so I won't forget.

As we move to a p2p relay network, we need a mechanism that ensure that relays and builders who are new or go offline for a period of time are able to catch up to the latest. With the SetFeeRecipient method, this can be achieved roughly by sending announcements at regular intervals. However, this creates a lot of unnecessary network traffic. It would be better if there was a pull mechanism that a relay/builder could invoke when they wish to calculate the mapping. Here are a few ideas:

• Fee recipient bulletin -- there could be a smart contract on mainnet that validators submit signed announcements to and builders could process all events from the contract to calculate the mapping. Since there is still no BLS primitive on mainnet, the mapping couldn't be authenticated on-chain. Thus, it is more of a bulletin.
• Fee recipient DHT -- since the p2p network will likely already use discv5, we could use one of the extensions to store a mapping of validator to fee recipient. It's not clear how expensive it would be to iterate through this DHT, but at worst the relay/builder should be able to quickly get the proposers for the next epoch.
• Fee recipient syncing -- as each announcement is signed, it would be possible to create a syncing strategy to acquire the mapping. It could be ranged over either validator indexes or unix time (e.g. "get latest announcements for validators n...m" or "get all announcements from n...m).

Are there other approaches? I'm leaning a bit towards the syncing strategy as it seems simplest and most robust of the above ideas.

The specification mentions feeRecipientBalance but the implementation calls it feeRecipientDiff

What is the current direction this is headed? I'm not sure how bad payloads are supposed to be communicated between mev-boost nodes and I think this is important in order to know ahead of time which relays to avoid using.

The options I can think of are:

1. consensus layer p2p - seems like the best solution but not feasible pre-merge
2. mev-boost implements p2p - also seems unlikely pre-merge, but maybe with re-use of Prysm's libraries this wouldn't actually be too bad
3. relays are used to also relay fraud proofs, and this would rely on relays policing each other. Their incentive for doing this is to increase their own flow of payloads by notifying mev-boost nodes that other relays are misbehaving. This one seems least appealing but maybe the easiest to actually implement pre-merge. Relevant comment: #82 (comment)

According to the specification,

mev-boost is initialized with a list of (BLSPublicKey, RelayURI) pairs representing trusted relays.

But currently we are not using any keys.

Is this required for the 0.1 release? Or better for the next milestone?

The latest release of the pos Ethereum spec is called "Kiln" and will be used as the new testnet. The full spec can be referenced here: https://hackmd.io/@n0ble/kiln-spec

The following work needs to take place in order to be up to date with this release:

• update mev-boost specs to reference the latest engine API: https://github.com/ethereum/execution-apis/blob/v1.0.0-alpha.7/src/engine/specification.md
• engine_executePayloadV1 was renamed to engine_newPayloadV1 and the behavior was modified. This has implications for the fraud proof mechanism of milestone 2 as it is unclear if mev-boost can rely on this API to perform historical payload simulation. The POS research team and implementation teams should be contacted to address this issue.

From #20 (comment):

@tersec tersec on Feb 2

This is JSON-RPC serialized, while SignedBlindedBeaconBlock is only specified here as an SSZ type. Aspects of SSZ type serialization aren't otherwise specified by the engine API spec on which this builds, such as SignedBlindedBeaconBlock.signature and SignedMEVPayloadHeader.signature (both BLSSignatures), leaving room for noninteroperability across implementations.

https://github.com/ethereum/execution-apis/blob/v1.0.0-alpha.6/src/engine/specification.md#executionpayloadv1 provides an explicit mapping between a JSON-RPC-serialization of engine.ExecutionPayloadV1 and SSZ representation of bellatrix.ExecutionPayload, where both are supposed to be otherwise isomorphic.
Contributor
@realbigsean realbigsean on Feb 25

I agree this should be in JSON-RPC terms.

We could fully specify blocks here, similar to how ExecutionPayloadV1 is defined by the execution API, but that might not be worth doing and maintaining because we already have blocks specified ethereum/beacon-APIs#194, and I don't think we'd want to leave room for discrepancy between JSON serialization of the same type between the two APIs.

I've raised a PR here to add Bellatrix types to the beacon API as well as endpoints and types necessary for blind transaction signing.
Member
@elopio elopio 14 days ago

Thanks @tersec and @realbigsean. I need help here to understand the proposal of using JSON-RPC for encoding these new methods. As I understand, SSZ is the new encoding for the consensus layer. I wonder why do we want to use the old encoding for the new things. Probably this has been discussed long ago, so please be patient with me :)
The way I imagine it, these APIs will be implemented by new software in the future, so these seems like a good way to start moving the future to be more consistent and easier to read. Am I being naive or missing some part of the story?
@tersec tersec 14 days ago

SSZ is used as as the intra-consensus layer/inter-beacon-node serialization over the libp2p gossip network, yes.

JSON-RPC serializations appear in the REST API and related areas, e.g., validator clients mostly use JSON-RPC-type encodings to interact with beacon nodes. Both remain in use.
@ElOpio
tersec

Until the spec is at a place where we're comfortable PRing into ethereum/execution-apis, it would be nice if it was placed in the docs/ directory so that PRs can be made against it. Right now, as a non-member of this repository, I have no efficient way of proposing changes.

We can use codecov or coveralls to make it looks nice.

We can get the report with go test ./lib/... ./cmd/... -v covermode=count -coverprofile=coverage.out

On the mev-boost workshop it came up that it would be great to include the proof of payment in the getHeader response, with which the builder proves to the proposer/mev-boost that the payment has actually happened.

This could go into the initial v0.2.x (and would probably be good to have, even if we don't automatically verify it).

How would this proof work exactly?

The specs should be merged in order to provide client teams with a stable api to build against. Once merged, it will be necessary to work with the consensus client teams to add compatibility.

TLDR: There exist a financial incentive for the execution payload to be proposed later than the anticipated time window for it. We worry that this might lead to favoring larger validators, encourage attester bribing infrastructure and might contribute to making consensus attacks described in recent papers more likely.

Context:
The beacon chain has slots. A slot is currently every 12s. Within this slot exists an execution chain block as well as consensus objects. While there is theoretically (12s-epsilon) time to put the beacon block together within the slot, there are actually sub-slots within this slot that are softly enforced by consensus clients. These sub-slots are necessary in order to account for propagation time (incl. network delays) to attesters and for the proper construction of a beacon block.

Afaik - an execution chain block is supposed to be proposed in the first 4s of the 12s slot. This gives 8s for it to be voted on by attesters and in MEV-Boost, leaves enough for the body to be revealed by the end of the slot. For eg. see waitToOneThirdOrValidBlock() in Prysm's client here: https://github.com/prysmaticlabs/prysm/blob/8c8f1bb9c18ebbc42241848a328ed275b12587e5/validator/client/attest.go#L250 (thanks @terencechain )

Worry 1: Late execution block proposals:
In theory, the proposer has until 12s-epsilon to submit a beacon block. This means the execution chain block could be constructed maybe at 11s rather than 4s in. This time difference means the builder would have an additional 7s to construct a block. In those 7s, new transactions could’ve come in and made the block more profitable.

The proposer and builders now have an incentive to delay the block submission as much as possible.

This is problematic because:

• a proposer that controls a lot of validators can inherently force them to vote later (by modifying their consensus client source codes) vs a solo validator. This means there is an advantage to controlling a lot of validators that are well-connected to the p2p network.
• another flavor of this problem is that there might exist an incentive to bribe attesters in order to delay their attestations.

Questions:

1. on average, how much more profitable are blocks after X seconds from Flashbots data? This will help us understand how big of an advantage would larger validators get.
2. what is the technical complexity of either bribing attesters and making sure the attesters attest later than expected?
3. what is the financial risk encountered by doing this (later attestations get less rewards + there is a risk of missing the slot afaik which would incur a slashing penalty)? can we surface and calculate the expected value of doing this?
4. Given this information, how expensive would it be to bribe attesters?

The good news is that we have numbers to quantify this, in particular, Flashbots finds that on average, each additional second of waiting is worth 0.034 eth in additional miner payment (incl. 1559 tips). The outlier numbers we have (top 0.1% of blocks excluding tips - only bundle payments) are around 1 eth/s (thx @metachris and @Ruteri ). These numbers generally seem too small for this worry to be relevant.

The bad news is that this quantification is imperfect. In particular, if someone can delay their ultimate proposal to a few seconds later, they are able to do cross-domain arbitrage with low-latency domains more effectively. Priced in, this might be worth large sums of money.

To-do:

• poll relevant parties (searchers who run these type of strategies) to understand what this could amount to for them.

Worry 2: Reorgs
Two papers (https://arxiv.org/pdf/2203.01315.pdf and https://eprint.iacr.org/2021/1413.pdf) were recently released detailing 5 attacks on PoS consensus (co-authored by @casparschwa @barnabemonnot et.al). One attack in particular relies on the timing of a block proposal.

We worry wether the financial incentives outlined above exert an upwards pressure on the likeliness of such a timing attack happening in the wild. I understand these attacks have been fixed and suspect this worry can be easily alleviated by @casparschwa sharing how these attacks were fixed.

We open the discussion here and look forward to your questions and comments :)!

see milestone 1 specification