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 bug where txs wouldn't get squeezed if coin state already existed #767

Merged
merged 2 commits into from
Nov 9, 2022

Conversation

Voxelot
Copy link
Member

@Voxelot Voxelot commented Nov 9, 2022

Found a bug where submitting more than two txs that conflict on the same input would stop squeezing the lower priced txs after the first squeeze.

This would ultimately lead to the tx with the lower price getting skipped by the executor, making this only a minor annoyance. However it causes wasted computation during block production.

@Voxelot Voxelot added the bug Something isn't working label Nov 9, 2022
@Voxelot Voxelot requested a review from a team November 9, 2022 03:19
Dentosal
Dentosal previously approved these changes Nov 9, 2022
Copy link
Member

@Dentosal Dentosal left a comment

Choose a reason for hiding this comment

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

LGTM. Maybe adding bug description to the test case would be useful, but OTOH this PR can be found from the git history anyway

freesig
freesig previously approved these changes Nov 9, 2022
Copy link
Contributor

@freesig freesig left a comment

Choose a reason for hiding this comment

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

Nice!

xgreenx
xgreenx previously requested changes Nov 9, 2022
fuel-txpool/src/containers/dependency.rs Show resolved Hide resolved
@xgreenx xgreenx dismissed their stale review November 9, 2022 03:55

I'm wrong, if there is not is_spend_by, we should become him

Co-authored-by: Green Baneling <XgreenX9999@gmail.com>
@Voxelot Voxelot dismissed stale reviews from freesig and Dentosal via 6d7dc3b November 9, 2022 04:13
@Voxelot Voxelot enabled auto-merge (squash) November 9, 2022 04:24
@Voxelot
Copy link
Member Author

Voxelot commented Nov 9, 2022

looks like the tests are passing now, ty for the assist @xgreenx

@Voxelot Voxelot merged commit 07b206a into master Nov 9, 2022
@Voxelot Voxelot deleted the Voxelot/fix-spent-by branch November 9, 2022 04:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants