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.

Sign up for GitHub

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

Jump to bottom

feat(witness): use block executor to execute block inside debug_execution_witness #11149

Merged
merged 4 commits into from
Sep 24, 2024
Merged

feat(witness): use block executor to execute block inside debug_execution_witness #11149

merged 4 commits into from
Sep 24, 2024

Conversation

0x00101010
Copy link
Contributor

@0x00101010 0x00101010 commented Sep 24, 2024 •
edited
Loading

The code does the following:

  1. allow passing BlockExecutorProvider to RpcModuleBuilder and eventually passed down to DebugApi initialization
  2. rework debug_execution_witness API to use block_executor to execute blocks.

@0x00101010 0x00101010 mentioned this pull request Sep 24, 2024
Closed
@mattsse
Copy link
Collaborator

mattsse commented Sep 24, 2024

cool this should actually do it!
we do this first and then make it nicer by finding a better approach

@mattsse mattsse marked this pull request as ready for review September 24, 2024 08:40
@mattsse mattsse added the A-rpc Related to the RPC implementation label Sep 24, 2024
mattsse
mattsse requested changes Sep 24, 2024
View reviewed changes
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.

awesome, this is great, exactly what I had in mind as the intermediary solution.

just needs doc cleanup

crates/rpc/rpc/src/debug.rs
Comment on lines +46 to +50
pub struct DebugApi<Provider, Eth, BlockExecutor> {
inner: Arc<DebugApiInner<Provider, Eth, BlockExecutor>>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Rjected as discussed we can move this type in here first and in later step unify execution step by step.

crates/rpc/rpc/src/debug.rs Show resolved Hide resolved
crates/rpc/rpc/src/debug.rs Outdated Show resolved Hide resolved
@mattsse mattsse added the C-enhancement New feature or request label Sep 24, 2024
@0x00101010 0x00101010 force-pushed the block_executor_in_debug_execution_witness branch from d3e80a7 to 91e2557 Compare September 24, 2024 15:06
@0x00101010 0x00101010 force-pushed the block_executor_in_debug_execution_witness branch from 91e2557 to fba4f33 Compare September 24, 2024 15:09
mattsse
mattsse approved these changes Sep 24, 2024
View reviewed changes
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.

nice,
the state hook looks correct to, mind doing a sanity check @Rjected

Rjected
Rjected approved these changes Sep 24, 2024
View reviewed changes
Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

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

nice! this looks good to me

@mattsse mattsse added this pull request to the merge queue Sep 24, 2024
Merged via the queue into paradigmxyz:main with commit d3114b4 Sep 24, 2024
35 checks passed
@syjn99 syjn99 mentioned this pull request Nov 28, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattsse mattsse mattsse approved these changes

@Rjected Rjected Rjected approved these changes

@emhane emhane Awaiting requested review from emhane emhane is a code owner

@onbjerg onbjerg Awaiting requested review from onbjerg onbjerg is a code owner

@gakonst gakonst Awaiting requested review from gakonst gakonst is a code owner

Assignees
No one assigned
Labels
A-rpc Related to the RPC implementation C-enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@0x00101010 @mattsse @Rjected