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

feat(rpc): debug_executionWitness #9249

Merged
merged 9 commits into from
Aug 1, 2024
Merged

feat(rpc): debug_executionWitness #9249

merged 9 commits into from
Aug 1, 2024

Conversation

clabby
Copy link
Collaborator

@clabby clabby commented Jul 2, 2024

Overview

Adds a new endpoint to the debug namespace for generating historical execution witnesses.

Rationale

For ZK and Optimistic proving systems, generating execution witnesses for stateless execution can be tough. Before peter's recent cross-client validation proposal, there were fairly limited proposals for this feature pre-verkle. All existing options involve expensive-to-run nodes with large storage requirements, i.e. gcmode=archive + state.scheme=hash geth.

Hash scheme geth does the trick, but in an abusive way towards their debug_dbGet endpoint, which just-so-happens to key all RLP-encoded trie nodes by their hash (H(rlp(trie_node)) -> rlp(trie_node)).

How does it work?

  1. The block is re-executed on top if its parent's state.
  2. All referenced accounts & their storage slots (both referenced and modified) are pulled from the wrapping CacheDB.
  3. For each of these accounts, an AccountProof is generated, utilizing the HistoricalStateProvider::proof method.
  4. All RLP encoded trie nodes are hashed, and placed into a map of H(rlp(trie_node)) -> rlp(trie_node).

DoS concerns

Due to reth's database format only storing merkle diffs, this process can become very expensive for the same reason that state root generation in historical execution within reth can be very expensive. Each diff is layered, which means the further back we reach, the more expensive the operation becomes (approaching upper bound, potentially unable to even fit in memory due to the sheer volume of layers.)

Because of this, this endpoint has been placed in the debug_* namespace, which is not often exposed to the open internet by node operators. Exposing this endpoint can pose major DoS concerns for node operators as it stands.

Future historical eth_getProof support

Historical eth_getProof support was already possible in reth before this PR, but was never implemented due to the above DoS concerns (per @rkrasiuk). This PR does not remove the requirement for being at tip to use eth_getProof, but this could be supported in a future PR, by adding a sane default for allowed eth_getProof depth, configurable via the CLI. A max chain depth for eth_getProof would effectively cap the number of in-memory layers that must be applied to the trie.

TODO

In order to complete this feature, we'll need to be able to also include branch sibling nodes that must be referenced in order to delete leaves in the MPT. This endpoint currently gives the entirety of the witness data required for execution, but not to re-produce the block header for the processed block. The crux of this is that we cannot know which nodes we need to reveal until we're performing the deletion, meaning we also have to compute the state root in this endpoint and collect them, as these nodes are not included within the account proofs we already generate.

To outline the problem, let's say we start with this small trie, where the root node is a branch containing 2 children (both leaf nodes):

Untitled (21)

If we want to delete the node in the 3 bucket, the branch node would be left with only one child - this is an invalid state. To complete the deletion, we must reveal A (which is hashed), and collapse the branch node into A.

In order of operation:

  1. Replace 3 with rlp(empty) = 0x80
  2. Reveal the preimage of H(node_at(A)), and prepend the branch nibble (A) to its path.
  3. Replace the branch node with the truncated leaf.
Untitled (19)
Untitled (23)

@clabby clabby added C-enhancement New feature or request A-rpc Related to the RPC implementation A-trie Related to Merkle Patricia Trie implementation labels Jul 2, 2024
@clabby clabby self-assigned this Jul 2, 2024
@clabby clabby force-pushed the cl/witness-producer branch 2 times, most recently from 4189012 to 2c93be0 Compare July 2, 2024 20:30
@clabby clabby added the S-feedback-wanted This issue/PR is looking for feedback label Jul 2, 2024
crates/rpc/rpc/src/debug.rs Outdated Show resolved Hide resolved
@clabby clabby force-pushed the cl/witness-producer branch 3 times, most recently from b0dbd43 to d9f1ca1 Compare July 2, 2024 20:50
.with_database(StateProviderDatabase::new(state))
.with_bundle_update()
.build();

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reminder to self: Need 4788 call here

Copy link
Member

@Rjected Rjected Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wondering if we can eventually use the block executor traits here, as there will be more system contract calls and maintaining a copy of block execution logic in debug APIs seems brittle

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. I also specifically need this for Optimism execution, so that would be ideal. Glad to circle back around to this before we merge, just have to get it working first.

Copy link
Member

@gakonst gakonst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should prob be part of the Evm Executor as a execute_witnessed() style operation? and then the RPC method calls that?

Comment on lines 594 to 593
let account_proofs = db
.cache
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be access list inspector instead

@rkrasiuk rkrasiuk force-pushed the cl/witness-producer branch from 6184498 to 6c40fe6 Compare July 23, 2024 17:25
@rkrasiuk rkrasiuk mentioned this pull request Jul 25, 2024
@rkrasiuk rkrasiuk force-pushed the cl/witness-producer branch from 6c40fe6 to 81526f9 Compare July 30, 2024 21:19
@rkrasiuk rkrasiuk force-pushed the cl/witness-producer branch from 81526f9 to 26e133c Compare August 1, 2024 15:29
@rkrasiuk rkrasiuk marked this pull request as ready for review August 1, 2024 15:30
@rkrasiuk rkrasiuk requested review from mattsse and emhane as code owners August 1, 2024 15:30
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestions and nits

crates/rpc/rpc/src/debug.rs Outdated Show resolved Hide resolved
crates/rpc/rpc/src/debug.rs Outdated Show resolved Hide resolved
crates/rpc/rpc/src/debug.rs Outdated Show resolved Hide resolved
crates/rpc/rpc/src/debug.rs Show resolved Hide resolved
crates/rpc/rpc/src/debug.rs Outdated Show resolved Hide resolved
.with_database(StateProviderDatabase::new(state))
.with_bundle_update()
.build();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still need to call 4788 etc here right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

@rkrasiuk rkrasiuk force-pushed the cl/witness-producer branch from 47398b0 to ccbfa3a Compare August 1, 2024 19:46
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@rkrasiuk rkrasiuk added this pull request to the merge queue Aug 1, 2024
.with_bundle_update()
.build();

pre_block_beacon_root_contract_call(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one more thing, this doesn't check if the target block is post merge

do we need to call any other systemcalls or just 4788?

Merged via the queue into main with commit d5d46a8 Aug 1, 2024
33 checks passed
@rkrasiuk rkrasiuk deleted the cl/witness-producer branch August 1, 2024 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rpc Related to the RPC implementation A-trie Related to Merkle Patricia Trie implementation C-enhancement New feature or request S-feedback-wanted This issue/PR is looking for feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants