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): add preimages to execution witness response #10456

Merged
merged 8 commits into from
Sep 2, 2024

Conversation

shekhirin
Copy link
Collaborator

@shekhirin shekhirin commented Aug 22, 2024

Closes #10404

@shekhirin shekhirin force-pushed the alexey/execution-witness-preimages branch from 5c9158f to 8df597a Compare August 23, 2024 09:42
@shekhirin shekhirin added C-enhancement New feature or request A-rpc Related to the RPC implementation labels Aug 23, 2024
@shekhirin shekhirin marked this pull request as ready for review August 23, 2024 09:42
@@ -659,6 +667,9 @@ where
let witness = state_provider
.witness(HashedPostState::default(), hashed_state)
.map_err(Into::into)?;

// TODO(alexey): return `state_preimages` as well when https://github.com/alloy-rs/alloy/pull/1178 is released
Copy link
Member

Choose a reason for hiding this comment

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

lets add a follow up issue to track

Comment on lines 652 to 659
state_preimages.insert(hashed_address, alloy_rlp::encode(address));

for (slot, value) in account.storage {
let hashed_slot = keccak256(B256::from(slot));
let slot = B256::from(slot);
let hashed_slot = keccak256(slot);
storage.storage.insert(hashed_slot, value);

state_preimages.insert(hashed_slot, alloy_rlp::encode(slot));
Copy link
Member

Choose a reason for hiding this comment

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

ideally i'd want to reuse HashedPostState::from_cache_state for this, but no need to tackle here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

right, but you'd need to walk the cache state anyway because HashedPostState won't contain pre-images, no?

@@ -659,6 +667,9 @@ where
let witness = state_provider
.witness(HashedPostState::default(), hashed_state)
.map_err(Into::into)?;

// TODO(alexey): return `state_preimages` as well when https://github.com/alloy-rs/alloy/pull/1178 is released
Copy link
Member

Choose a reason for hiding this comment

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

let's maybe wait for the release? otherwise, it's collecting preimages for no reason

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sg

@rkrasiuk rkrasiuk added the S-blocked This cannot more forward until something else changes label Aug 23, 2024
@rkrasiuk
Copy link
Member

blocked by alloy release w/ alloy-rs/alloy#1178

@shekhirin
Copy link
Collaborator Author

Alloy release just happened, cc @rkrasiuk

@mattsse mattsse added this pull request to the merge queue Aug 30, 2024
@mattsse mattsse removed this pull request from the merge queue due to a manual request Aug 30, 2024
@mattsse mattsse requested a review from rkrasiuk August 30, 2024 08:54
@emhane emhane removed the S-blocked This cannot more forward until something else changes label Aug 30, 2024
@rkrasiuk rkrasiuk added this pull request to the merge queue Sep 2, 2024
Merged via the queue into main with commit ef975f1 Sep 2, 2024
35 checks passed
@rkrasiuk rkrasiuk deleted the alexey/execution-witness-preimages branch September 2, 2024 09:38
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 C-enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(rpc): expose execution pre-images
5 participants