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

Refactor dry_run #3360

Closed
grarco opened this issue Jun 4, 2024 · 2 comments
Closed

Refactor dry_run #3360

grarco opened this issue Jun 4, 2024 · 2 comments
Assignees
Labels

Comments

@grarco
Copy link
Contributor

grarco commented Jun 4, 2024

Currently the dry_run command has some custom logic to evaluate a transaction. This sometimes leads to problems like #3358 where the logic of dry run doesn't match the one in protocol. We should try to refactor this to at least rely on the dispatch_tx function so that changes of a diverging behavior are minimized.

We could also think about mocking a one-transaction block and directly call finalize_block: this would enforce the same exact behavior even though it might slow down the simulation given the extra operations performed there. These operations though run only on new epoch so we might be able to side-step them by just mocking a block which is not the first one of a new epoch.

For reference: @Fraccaman, @yito88

@grarco grarco added the prio:low label Jun 4, 2024
@cwgoes cwgoes added this to the Nice to have and non-breaking milestone Aug 20, 2024
@grarco grarco self-assigned this Aug 30, 2024
@grarco grarco mentioned this issue Sep 5, 2024
1 task
@grarco
Copy link
Contributor Author

grarco commented Sep 5, 2024

None of the two solutions proposed above can actually be achieved.

To call finalize_block we'd need a mutable reference to the Shell that we don't want.
The solution using dispatch_tx is feasible but it requires to modify some functions' signatures from requiring WlState to requiring a generic S with some trait bounds. Unfortunately it gets complicated to handle ethereum-bridge related functions.

#3758 closes this issue by using dispatch_inner_txs which is instead easily modifiable to be used by dry_run_tx.

@grarco
Copy link
Contributor Author

grarco commented Sep 9, 2024

Closed by #3781

@grarco grarco closed this as completed Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants