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: nonce-syncing check of mempool transactions (PR 8) #508

Merged
merged 1 commit into from
Oct 4, 2023

Conversation

kanej
Copy link
Member

@kanej kanej commented Oct 3, 2023

There was a wrong check on whether the transactions against the last network interaction are still in the mempool. If at least one transaction is in the mempool we don't do the further checks that assume the transaction with that nonce has been either dropped or replaced by the user.

I have added a test for the failing case of rerun at the command line.

Fixes #506.

@kanej kanej force-pushed the fix/rerun-nonce-check branch 2 times, most recently from 8b59f63 to 41e103f Compare October 3, 2023 15:45
@kanej kanej force-pushed the fix/rerun-nonce-check branch 2 times, most recently from 277b374 to 49fa3ca Compare October 3, 2023 16:07
@kanej kanej changed the title fix: clarify nonce comparisons in nonce-syncing fix: nonce-syncing check of mempool transactions Oct 3, 2023
@kanej kanej marked this pull request as ready for review October 3, 2023 16:14
@kanej kanej requested a review from alcuadrado October 3, 2023 16:14
@kanej kanej changed the title fix: nonce-syncing check of mempool transactions fix: nonce-syncing check of mempool transactions (PR 8) Oct 3, 2023
@kanej kanej force-pushed the fix/additional-nonces-checks branch from 27c9b30 to 9d8ee09 Compare October 4, 2023 09:41
@kanej kanej force-pushed the fix/rerun-nonce-check branch from 49fa3ca to 349ac76 Compare October 4, 2023 09:45
Copy link
Member

@alcuadrado alcuadrado left a comment

Choose a reason for hiding this comment

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

Approved, will get deeper into the tests in the following pr

@kanej kanej force-pushed the fix/additional-nonces-checks branch 2 times, most recently from 77a16e9 to 60bd877 Compare October 4, 2023 16:53
Base automatically changed from fix/additional-nonces-checks to development October 4, 2023 17:01
There was a wrong check on whether the transactions against the last
network interaction are still in the mempool. If at least one
transaction is in the mempool we don't do the further checks that assume
the transacion with that nonce has been either dropped or replaced by
the user.

I have added a test for the failing case of rerun at the command line.

Fixes #506.
@kanej kanej force-pushed the fix/rerun-nonce-check branch from 349ac76 to 65b1338 Compare October 4, 2023 17:14
@kanej kanej merged commit 078d515 into development Oct 4, 2023
6 checks passed
@kanej kanej deleted the fix/rerun-nonce-check branch October 4, 2023 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

rerun bug
2 participants