feat(pectra): Use EIP-7865 encoding for execution_requests by ryanschneider · Pull Request #107 · ethereum/builder-specs

ryanschneider · 2024-12-06T00:59:45Z

As discussed here on the Eth R&D Discord I think we should change to using the EIP-7865 encoding for execution_requests for the following reason:

The relays usually validate submissions by sending them to a custom EL implementing the flashbots_validateBuilderSubmissionVN RPC (aside: I'd love to work with everyone and get this RPC standardized!). Since this RPC uses the same payload format as the builder spec, this means that any ELs that want to implement this method (or hopefully the future standardized version) would need to consume the requests in the "full" non-opaque format as well.

Since there didn't seem to be any strong objections to this I was encouraged to proceed with drafting a specs change, so here it is.

A couple more todos/questions:

Currently I've included placeholder 0x_TODO values for the encoded requests lists, ideally these should actually be decodable values (so someone can try converting them to the consensus-spec types as a code test) I just haven't had time to do that yet.
@jtraglia just pointed me to get_execution_requests_list function in the consensus-specs which properly explains the encoding process, I'll link to that as well.
I think Bitvector is the correct type to use in the spec for each entry, but I'm not terribly familiar with the CL specs so and happy to be corrected!

ensi321 · 2024-12-06T01:50:54Z

-        }
-      }
-    },
+        "execution_requests": [ "0x00_TODO_DEPOSITS", "0x01_TODO_WITHDRAWLS", "0x02_TODO_CONSOLIDATIONS"],


Can use the example value in ethereum/execution-apis#607 .

Perfect, updated in 7955984 as well, thanks again!

ensi321 · 2024-12-06T02:00:28Z

+            type: array
+            items:
+              $ref: '../../beacon-apis/types/primitive.yaml#/Bitvector'
+            minItems: 3


minItems should be 0 in case all of the requests (deposit, withdrawal and consolidation) are empty

Oh right I was still thinking pre- ethereum/execution-apis#599, updated in 7955984 thanks!

lucassaldanha · 2024-12-06T02:28:15Z

-            description: "`ExecutionRequests` to include in block proposal."
+            type: array
+            items:
+              $ref: '../../beacon-apis/types/primitive.yaml#/Bitvector'


I am not sure if the Bitvector is a good semantic representation for the opaque field, although technically, it could work.

I would consider adding a Bytes primitive type (similar to Bytes32) but unbounded in length.

Keen to have people weight in on this before committing to it.

Funny when writing this up I originally used Bytes only to go look up the actual primitives and realize it didn't exist. :). So ya I'd be in favor of that change as well. I assume that'd be a separate PR on https://github.com/ethereum/beacon-apis/ ?

I believe so. It looks like builder-specs is importing beacon-api as a git submodule and referring to the types defined there.

Ok cool I just sent ethereum/beacon-APIs#484 I guess if this is accepted I can update the submodule in this PR and point to that. Thanks for the suggestion!

I would consider adding a Bytes primitive type (similar to Bytes32) but unbounded in length.

you can't have unbounded ssz types

nflaig

I don't understand why we want to adopt this format on the CL side, this would mean we also have update the other apis on the beacon-api side to match the format.

SSZ is a native concept to the CL and embedding SSZ serialized data (as hex) inside JSON quite frankly is a horrible pattern.

Would be good if we can keep that hacky approach contained to the engine api

nflaig · 2024-12-06T17:46:50Z

+### `OpaqueExecutionRequests`
+
+```python
+class OpaqueExecutionRequests(List[Bitvector]):


those are not really opaque, just ssz encoded and 1-byte prefixed, anyone can decode the requests and read the data

They're "semantically" opaque, EIP-7865 stipulates that each request type is allowed to decide its own encoding, it's just a coincidence that the current three are all SSZ in the same way that all EIP-2718 txs are currently RLP.

A requests object consists of a request_type prepended to an opaque byte array request_data. The request_data contains zero or more encoded request objects.

Each request type will defines its own requests object with its own request_data format.

I guess if the same wording is used on the engine-api side this is fine

ryanschneider · 2024-12-06T18:17:42Z

I don't understand why we want to adopt this format on the CL side, this would mean we also have update the other apis on the beacon-api side to match the format.

SSZ is a native concept to the CL and embedding SSZ serialized data (as hex) inside JSON quite frankly is a horrible pattern.

Would be good if we can keep that hacky approach contained to the engine api

When you say we "have to update the other apis" do you mean we'd be forced to because of some hard dependency, or just to maintain consistency? If there's a hard dependency then yes I agree we shouldn't do it, but since the builder spec sits in a grey zone between the EL and CL IMO it's ok for it to be less consistent with the rest of the CL APIs.

nflaig · 2024-12-06T19:03:11Z

When you say we "have to update the other apis" do you mean we'd be forced to because of some hard dependency, or just to maintain consistency?

There are apis which pass a SignedBlindedBeaconBlock around, namely produceBlockV3 and publishBlindedBlockV2 (which just passes the block through to builder) and those follow the format used in the builder spec.

Further more, I think we would also have to update BeaconBlockBody on the CL spec as otherwise the hash tree root of full block and blinded block would be different.

but since the builder spec sits in a grey zone between the EL and CL IMO it's ok for it to be less consistent with the rest of the CL APIs.

is it a grey zone? imo the builder spec is a pure CL spec extension

ryanschneider · 2024-12-06T21:24:21Z

There are apis which pass a SignedBlindedBeaconBlock around, namely produceBlockV3 and publishBlindedBlockV2 (which just passes the block through to builder) and those follow the format used in the builder spec.

Ah, thanks, I wasn't aware of these, and ya my reading is they would have to change as well. :( @lucassaldanha or @jtraglia can you confirm?

is it a grey zone? imo the builder spec is a pure CL spec extension

I guess what I meant by grey zone is that the block builders are usually custom ELs, for example rbuilder is built on reth/alloy and will now probably need to handle serde for the requests types (which is what I think we were trying to avoid with the engine api using the opaque types) as mentioned here: flashbots/rbuilder#267

jtraglia · 2024-12-06T21:57:15Z

There are apis which pass a SignedBlindedBeaconBlock around, namely produceBlockV3 and publishBlindedBlockV2 (which just passes the block through to builder) and those follow the format used in the builder spec.

Ah, thanks, I wasn't aware of these, and ya my reading is they would have to change as well. :( @lucassaldanha or @jtraglia can you confirm?

Correct. These would need to change too. And I would advise against that.

ryanschneider · 2024-12-06T22:23:25Z

Ok, sounds like builders will have to go with the JSON encoding for their relay submissions afterall, thanks everyone for the feedback but I'll close this PR.

nflaig · 2024-12-06T22:42:31Z

You don't have to use JSON if we start adopting SSZ on the builder api, see #104. This would address any concerns related to performance / latency overhead.

feat(pectra): Use EIP-7865 encoding for execution_requests

5703561

ensi321 reviewed Dec 6, 2024

View reviewed changes

fixup: Updates as suggested by @ensi321

7955984

ryanschneider requested a review from ensi321 December 6, 2024 02:20

lucassaldanha reviewed Dec 6, 2024

View reviewed changes

ryanschneider mentioned this pull request Dec 6, 2024

Update primitive.yaml: Add Bytes primitive ethereum/beacon-APIs#484

Closed

nflaig reviewed Dec 6, 2024

View reviewed changes

ryanschneider mentioned this pull request Dec 6, 2024

Pectra devnet-5: execution_requests needs latest alloy and should have the request_type byte prefixed to each flashbots/rbuilder#267

Open

ryanschneider closed this Dec 6, 2024

ryanschneider mentioned this pull request Dec 6, 2024

[Bug] SignedBidSubmissionV4 needs to use "JSON" encoding of execution requests alloy-rs/alloy#1763

Closed

This was referenced Dec 11, 2024

feat(relay): Re-add {Deposit,Withdrawal,Consolidation}Request objects for relay submissions alloy-rs/alloy#1786

Closed

feat(relay): ExecutionRequestsV4 with eip7685::Requests conversion alloy-rs/alloy#1787

Merged

Conversation

ryanschneider commented Dec 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nflaig left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanschneider commented Dec 6, 2024

Uh oh!

nflaig commented Dec 6, 2024

Uh oh!

ryanschneider commented Dec 6, 2024

Uh oh!

jtraglia commented Dec 6, 2024

Uh oh!

ryanschneider commented Dec 6, 2024

Uh oh!

nflaig commented Dec 6, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ryanschneider commented Dec 6, 2024 •

edited

Loading

nflaig left a comment •

edited

Loading