feat: add rpc endpoint `engine_getPayloadV3` + fixes to block building pipeline by fmoletta · Pull Request #571 · lambdaclass/ethrex

fmoletta · 2024-09-26T19:52:15Z

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

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

Closes None, but is part of #344

This reverts commit a66dc76.

This reverts commit adf5204.

…ne-hive

**Motivation** Add basic block building to forkChoiceUpdatedV3  **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)   Closes None, is part of #344

mpaulucci · 2024-09-27T16:19:19Z

+    let spec_id = spec_id(storage, args.timestamp)?;
+    let mut evm_state = evm_state(storage.clone(), parent_block.number);
+    if args.beacon_root.is_some() && spec_id == SpecId::CANCUN {
+        beacon_root_contract_call(&mut evm_state, &payload.header, spec_id)?;


I feel like this should be done inside the evm crate. spec_id is specific to revm and should not be leaked outside ideally.

Yes, we need to refactor that bit of code so that the spec id is computed inside the evm methods instead of required by them, but I wanted to keep this PR small and focused

mpaulucci · 2024-09-27T16:22:11Z

            Ok(()) => {
                info!("Block with hash {block_hash} executed succesfully");
+
+                storage.set_canonical_block(block.header.number, block_hash)?;


we should only set a canonical block on forkchoiceUpdated.

The problem im having is that I ran into some hive tests that run the following pipeline:

engine_forkChoiceUpdatedV3 (with a new payload attributes) -> Here we build a new payload (a block with number head + 1) and return its id engine_getPayloadV3 -> Here we return the payload created in the previous call engine_newPayloadV3 -> Here we receive the payload (a block), execute it and add it to the chain engine_getBlockByNumber -> Here we receive the block number for the block we created in engine_newPayloadV3, but as it is not a canonical block storage.get_block_header returns null and the test fails

Ideally we should have a way to fetch this block without it needing to be canonical, I could add a TODO to it so that we know it is a temporary solution we must remove in the future

Added TODO along with issue explanation

This is weird, maybe getPayloadV3 sets the block as canonical. Let me look into the specs

I have not seen it. I will create a ticket.

mpaulucci · 2024-09-27T16:24:58Z

+    process_withdrawals(&mut evm_state, &args.withdrawals)?;
+    let account_updates = get_state_transitions(&mut evm_state);
+    payload.header.state_root = storage
+        .apply_account_updates(parent_block.number, &account_updates)?


Similar to #443 and maybe out of scope of this PR, but you should be able to do this even when parent_block is not canonical. So apply_account_updates should receive a block hash instead of a block number

…ne-new-payload

**Motivation** Add basic block building to forkChoiceUpdatedV3  **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)   Closes None, is part of #344

…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  **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   Closes None, but is part of #344

fmoletta added 25 commits September 23, 2024 16:18

Revert "fix: remove totalDifficulty (#347)"

bc150e9

This reverts commit a66dc76.

Complete forkChoiceUpdatedV3 up until payload validation

0c2cd4d

Add BuildPayloadArgs + id

1a1b781

Output proper payload id

7386f2b

Move execution payload to core

adf5204

Revert "Move execution payload to core"

772fac1

This reverts commit adf5204.

Push uncommited file

f8409b0

Add block building

7446289

Clippy + fmt

2b2f4dd

Modularize fork choice update fn

5e47673

Fix timestamp

31c380f

Fix ser

1bdf863

Fix

69d280b

Merge branch 'main' of github.com:lambdaclass/ethereum_rust into engi…

677a7ec

…ne-hive

Integrate previous changes (part 1)

9737674

fetch only header instead of block

1e3d710

Fix typo + remove unwrap

a634c13

fetch only header instead of block

995aca8

Clarify hardcoded values

a856b7a

Add doc

3f1e4ed

Add InvalidForkChoiceState RpcErr

8adfe28

revert uneeded changes

69607ae

Change miner to builder

9618e32

rename: local_block -> payload

76e4bdf

Add request + impl ExecutionPayloadV3::from_block

517d45b

fmoletta changed the base branch from main to engine-hive September 26, 2024 19:52

fmoletta added 4 commits September 26, 2024 16:54

Make ExecutionPayloadV3 serializable

b1bb89f

Impl endpoint

dc6da98

Add empty missing fields

5ba2b60

Compute new state root when building payloads

956d7cb

fmoletta added 4 commits September 26, 2024 18:38

Map error

f3c356f

fmt

8f3fdaa

remove some pointless clones in storage

cf4f86e

Set newly added block as canonical in engine_NewPayloadV3

b14e226

fmoletta changed the title ~~feat: add rpc endpoint engine_getPayloadV3~~ feat: add rpc endpoint engine_getPayloadV3 + fixes to bock building pipeline Sep 26, 2024

fmoletta changed the title ~~feat: add rpc endpoint engine_getPayloadV3 + fixes to bock building pipeline~~ feat: add rpc endpoint engine_getPayloadV3 + fixes to block building pipeline Sep 26, 2024

fmoletta added 2 commits September 26, 2024 19:05

Fix comment + trace paylaod get request

694930f

Clippy

a491e8f

fmoletta mentioned this pull request Sep 26, 2024

feat: expand forkChoiceUpdatedV3 with basic block building #540

Merged

Compute block value

6a9d884

fmoletta marked this pull request as ready for review September 27, 2024 14:57

fmoletta requested a review from a team as a code owner September 27, 2024 14:57

Base automatically changed from engine-hive to main September 27, 2024 16:14

mpaulucci reviewed Sep 27, 2024

View reviewed changes

Comment thread crates/blockchain/payload.rs Outdated

fmoletta added 3 commits September 27, 2024 16:32

Merge branch 'main' of github.com:lambdaclass/ethereum_rust into engi…

0e358e6

…ne-new-payload

Fix typo

24acaf6

Add TODO

f2de4a1

mpaulucci approved these changes Sep 30, 2024

View reviewed changes

fmoletta added this pull request to the merge queue Sep 30, 2024

Merged via the queue into main with commit 9d3aefe Sep 30, 2024

fmoletta deleted the engine-new-payload branch September 30, 2024 17:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add rpc endpoint `engine_getPayloadV3` + fixes to block building pipeline#571

feat: add rpc endpoint `engine_getPayloadV3` + fixes to block building pipeline#571
fmoletta merged 39 commits into
mainfrom
engine-new-payload

fmoletta commented Sep 26, 2024 •

edited

Loading

Uh oh!

mpaulucci Sep 27, 2024

Uh oh!

fmoletta Sep 27, 2024

Uh oh!

mpaulucci Sep 27, 2024

Uh oh!

fmoletta Sep 27, 2024 •

edited

Loading

Uh oh!

fmoletta Sep 27, 2024

Uh oh!

fmoletta Sep 27, 2024

Uh oh!

mpaulucci Sep 30, 2024

Uh oh!

mpaulucci Sep 30, 2024

Uh oh!

mpaulucci Sep 30, 2024

Uh oh!

mpaulucci Sep 27, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

fmoletta commented Sep 26, 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!

fmoletta Sep 27, 2024 • 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!

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fmoletta commented Sep 26, 2024 •

edited

Loading

fmoletta Sep 27, 2024 •

edited

Loading