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

chore(revm): expose convenience cache state for preimages #1756

Closed
wants to merge 2 commits into from

Conversation

0x00101010
Copy link

Description

expose cache state for getting preimages, its initial user will be debug_execution_witness in reth. Reference PR here: https://github.com/paradigmxyz/reth/pull/10736/files

@0x00101010 0x00101010 changed the title chore(revm): expose cache state for getting preimages chore(revm): expose convenience cache state for preimages Sep 6, 2024
@@ -51,9 +51,8 @@ impl FunctionStack {

/// Pops a frame from the stack and sets current_code_idx to the popped frame's idx.
pub fn pop(&mut self) -> Option<FunctionReturnFrame> {
self.return_stack.pop().map(|frame| {
self.return_stack.pop().inspect(|frame| {
Copy link
Author

Choose a reason for hiding this comment

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

CI clippy error fixes

@rakita
Copy link
Member

rakita commented Sep 9, 2024

I would rather not have this function as State would show wrong changes if CacheDB is taken without taking bundle state.

@rakita rakita closed this Sep 9, 2024
@0x00101010
Copy link
Author

0x00101010 commented Sep 9, 2024

I would rather not have this function as State would show wrong changes if CacheDB is taken without taking bundle state.

Thanks for the review @rakita!

Any suggestions on how to take it out? Would it better to directly do it like https://github.com/paradigmxyz/reth/pull/10736/files#diff-7fbb0c1e65c92e8879aa308fee2538af673a2cd6d70f9b5b1103cf4d6554604dR386 or create a function like take_bundle_and_cache?

The background story here is that in order to implement debug_execution_witness API, we'll need to get CacheDB out so that we can retrieve preimages properly

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