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

devnet-5: Update EIP-7685: exclude empty requests data in commitment #2963

Merged
merged 6 commits into from
Dec 21, 2024

Conversation

jangko
Copy link
Contributor

@jangko jangko commented Dec 20, 2024

No description provided.

@tersec
Copy link
Contributor

tersec commented Dec 20, 2024

getPayloadV4 appears to be missing?

  1. The call MUST return executionRequests list representing execution layer triggered requests. Each list element is a requests byte array as defined by EIP-7685. The first byte of each element is the request_type and the remaining bytes are the request_data. Elements of the list MUST be ordered by request_type in ascending order. Elements with empty request_data MUST be excluded from the list.

@jangko
Copy link
Contributor Author

jangko commented Dec 20, 2024

@tersec
Copy link
Contributor

tersec commented Dec 21, 2024

The ExecutionBundle's representation for the executionRequests is fine (since right now, internally, there are exactly 3 sources):

ExecutionBundle* = object
payload*: ExecutionPayload
blockValue*: UInt256
blobsBundle*: Opt[BlobsBundleV1]
executionRequests*: Opt[array[3, seq[byte]]]
targetBlobsPerBlock*: Opt[Quantity]

But when getPayloadV4 (which, I wasn't very clear I guess, obviously it exists, but the EIP-7685 update does not appear to exist for it): gets this, it simply relays the bundle of `executionRequests out from the JSON-RPC endpoint:

if bundle.executionRequests.isNone:
raise unsupportedFork("getPayloadV4 is missing executionRequests")
let com = ben.com
if not com.isPragueOrLater(ethTime bundle.payload.timestamp):
raise unsupportedFork("bundle timestamp is less than Prague activation")
GetPayloadV4Response(
executionPayload: bundle.payload.V3,
blockValue: bundle.blockValue,
blobsBundle: bundle.blobsBundle.get,
shouldOverrideBuilder: false,
executionRequests: bundle.executionRequests.get,
)

to be serialized implicitly via the default rules.

But two things are different now:

  1. they need to each be prefixed with the request type byte; and
  2. the empty one(s) need to be skipped

exactly the same/symmetrically as in newPayloadV4, i.e. newPayloadV4 needs to be able to consume what getPayloadV4 produces.

@jangko
Copy link
Contributor Author

jangko commented Dec 21, 2024

It start from here in the txpool. I know it's a bit confusing because we don't have dedicated block builder. But until then, we have to use what we have.

func executionRequests*(pst: var TxPacker): seq[seq[byte]] =
template append(dst, reqType, reqData) =
if reqData.len > 0:
reqData.insert(reqType)
dst.add(move(reqData))
result.append(DEPOSIT_REQUEST_TYPE, pst.depositReqs)
result.append(WITHDRAWAL_REQUEST_TYPE, pst.withdrawalReqs)
result.append(CONSOLIDATION_REQUEST_TYPE, pst.consolidationReqs)

@tersec
Copy link
Contributor

tersec commented Dec 21, 2024

It start from here in the txpool. I know it's a bit confusing because we don't have dedicated block builder. But until then, we have to use what we have.

func executionRequests*(pst: var TxPacker): seq[seq[byte]] =
template append(dst, reqType, reqData) =
if reqData.len > 0:
reqData.insert(reqType)
dst.add(move(reqData))
result.append(DEPOSIT_REQUEST_TYPE, pst.depositReqs)
result.append(WITHDRAWAL_REQUEST_TYPE, pst.withdrawalReqs)
result.append(CONSOLIDATION_REQUEST_TYPE, pst.consolidationReqs)

Oops, indeed

@tersec tersec merged commit 86fc24a into master Dec 21, 2024
17 checks passed
@tersec tersec deleted the devnet5-7685 branch December 21, 2024 05:59
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.

2 participants