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

fix: make manual mining produce non-empty blocks #459

Merged
merged 5 commits into from
Dec 3, 2024

Conversation

itegulov
Copy link
Contributor

@itegulov itegulov commented Dec 2, 2024

What πŸ’»

  • Minor refactorings to reduce boilerplate and unnecessary locking
  • Makes all manual mining methods take transactions from the mempool when they can

Closes #404

Why βœ‹

Correct behavior expected by users

@itegulov itegulov requested a review from a team as a code owner December 2, 2024 03:09
.next()
.expect("unexpected failure, value must exist");

tracing::info!("Reverting node to snapshot '{snapshot_id:?}'");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Throughout snapshot / mining modes we are using tracing::info!, do we want these to be written out by default? I know we want to move away from tracing::info all together but curious your thoughts on outputting this information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I am not adding new tracing::info, this line existed before this PR and I think we should refactor logging in a separate PR to reduce the noise.

Generally speaking IMO we should only use tracing::trace or tracing::debug.

dutterbutter
dutterbutter previously approved these changes Dec 2, 2024
Copy link
Collaborator

@dutterbutter dutterbutter left a comment

Choose a reason for hiding this comment

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

lgtm

* fix `v` calculation for legacy txs

* add `anvil_mine_detailed`
@dutterbutter dutterbutter added the approved βœ… PR is ready to be merged label Dec 2, 2024
@itegulov itegulov merged commit 7587a7a into main Dec 3, 2024
13 checks passed
@itegulov itegulov deleted the daniyar/manual-mine-non-empty-blocks branch December 3, 2024 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved βœ… PR is ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix: evm_mine should produce non-empty blocks when it can
2 participants