feat: expand forkChoiceUpdatedV3 with basic block building#540
Merged
Conversation
mpaulucci
reviewed
Sep 26, 2024
| } | ||
|
|
||
| impl BuildPayloadArgs { | ||
| /// Computes an 8-byte identifier by hashing the components of the payload arguments. |
Collaborator
There was a problem hiding this comment.
this is what the spec says? not against it, just curious
Contributor
Author
There was a problem hiding this comment.
I think I took it from geth. The spec says 8 Bytes - identifier of the payload build process
mpaulucci
reviewed
Sep 26, 2024
| gas_limit, | ||
| gas_used: 0, | ||
| timestamp: args.timestamp, | ||
| // TODO: should use miner config's extra_data |
Collaborator
There was a problem hiding this comment.
nit, the concept of miner doesn't exist anymore in post merge networks. it's either proposer or builder
Contributor
Author
There was a problem hiding this comment.
Switched to builder
mpaulucci
reviewed
Sep 26, 2024
| // Set the canonical block hash for a given block number. | ||
| fn set_canonical_block(&self, number: BlockNumber, hash: BlockHash) -> Result<(), StoreError>; | ||
|
|
||
| fn add_local_block(&self, payload_id: u64, block: Block) -> Result<(), StoreError>; |
Collaborator
There was a problem hiding this comment.
hmm, I would rename to payload, since block might be a bit confusing. WDYT?
Contributor
Author
There was a problem hiding this comment.
I figured block should be included in the name as the type stored is a Block, but I agree
mpaulucci
approved these changes
Sep 27, 2024
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Sep 30, 2024
…g pipeline (#571) Depends on #540 **Motivation** Being able to run the full basic block building pipeline (forkChoiceUpdated -> getPayload -> newPayload) CURRENT_STATUS: I could successfully run some tests that ran the above sequence, we still need to add transactions sent by sendRawTransaction endpoint to the payload in order to fully build blocks <!-- Why does this pull request exist? What are its goals? --> **Description** * Add rpc endpoint `engine_getPayloadV3` * Compute new state root (after applying beacon_root contract & withdrawals) when building a new payload (`engine_forkChoiceUpdated`) * Set new block as canonical in `engine_newPayloadV3` (TEMP FIX for not being able to fetch non canonical blocks) Missing work (for a later PR) * Fetch transactions from the mempool and add them to the block * Populate BlobsBundle <!-- A clear and concise general description of the changes this PR introduces --> <!-- Link to issues: Resolves #111, Resolves #222 --> Closes None, but is part of #344
mpaulucci
pushed a commit
that referenced
this pull request
Oct 16, 2024
**Motivation** Add basic block building to forkChoiceUpdatedV3 <!-- Why does this pull request exist? What are its goals? --> **Description** * Expand engine_forkChoiceUpdated: - Add validations - Fix fork state update conditions - Add basic block building With these changes hive engine tests no longer fail after calling engine_forkChoiceUpdatedV3. We still need to implement engine_getPayload to progress further NOTE: Some fixes over this code have been included in #571 (Computing the state root for newly built payloads instead of using the parent state root) <!-- A clear and concise general description of the changes this PR introduces --> <!-- Link to issues: Resolves #111, Resolves #222 --> Closes None, is part of #344
mpaulucci
pushed a commit
that referenced
this pull request
Oct 16, 2024
…g pipeline (#571) Depends on #540 **Motivation** Being able to run the full basic block building pipeline (forkChoiceUpdated -> getPayload -> newPayload) CURRENT_STATUS: I could successfully run some tests that ran the above sequence, we still need to add transactions sent by sendRawTransaction endpoint to the payload in order to fully build blocks <!-- Why does this pull request exist? What are its goals? --> **Description** * Add rpc endpoint `engine_getPayloadV3` * Compute new state root (after applying beacon_root contract & withdrawals) when building a new payload (`engine_forkChoiceUpdated`) * Set new block as canonical in `engine_newPayloadV3` (TEMP FIX for not being able to fetch non canonical blocks) Missing work (for a later PR) * Fetch transactions from the mempool and add them to the block * Populate BlobsBundle <!-- A clear and concise general description of the changes this PR introduces --> <!-- Link to issues: Resolves #111, Resolves #222 --> Closes None, but is part of #344
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Add basic block building to forkChoiceUpdatedV3
Description
With these changes hive engine tests no longer fail after calling engine_forkChoiceUpdatedV3. We still need to implement engine_getPayload to progress further
NOTE: Some fixes over this code have been included in #571 (Computing the state root for newly built payloads instead of using the parent state root)
Closes None, is part of #344