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

Support block building mode #368

Open
hai-rise opened this issue Oct 2, 2024 · 3 comments
Open

Support block building mode #368

hai-rise opened this issue Oct 2, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@hai-rise
Copy link
Contributor

hai-rise commented Oct 2, 2024

We need better solutions for these:

@hai-rise hai-rise added the enhancement New feature or request label Oct 2, 2024
@kien-rise
Copy link
Contributor

kien-rise commented Oct 29, 2024

Commit kien-rise@f9ec57d is actually independent and good to merge. By returning PevmError::FallbackToSequential, the block builder can do the execution by itself. Also, it allows the block builder to call tracing::warn!(...) for logging purpose.

Shall I create a PR containing this commit alone?


For commit kien-rise@bc95f3f, I only remember it is there to avoid pevm from freezing. The underlying cause, I am not sure, but most likely the nonce issue (too low or too high), causing pevm to be stuck at tx K although all the txs 0, 1, ..., K-1 are executed.

Since the NonceTooLow issue has been fixed in the block building side, we might not need this commit anymore. To be sure, I think I should do a benchmark without this commit then see whether pevm freezes or not.

@hai-rise
Copy link
Contributor Author

hai-rise commented Oct 30, 2024

Wouldn't kien-rise@f9ec57d fail tests as now some mainnet blocks assert a fallback error against valid sequential results? We also need to understand why users like block builders need a sequential fallback in the first place. If the original problems have better solutions then we should avoid this expensive fallback. For instance, per the already checked-in comments, we should skip unsound-nonce transactions right within pevm in block-building mode. Even when falling back is the best option, we should return ownership of the TxEnvs (or even BlockEnv) so the callee wouldn't need to initially clone, or recalculate it during re-execution.

For kien-rise@bc95f3f, we should still inspect the actual problems and find a better solution than sequential fallback. Depending on the mempool or block builder's behaviors not to deadlock is not good. Ideally pevm itself should not deadlock for any input.

@kien-rise
Copy link
Contributor

kien-rise commented Oct 31, 2024

See #406

See #408

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants