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

[TxPool] Self pruning stuck transactions #541

Merged
merged 7 commits into from
May 11, 2022

Conversation

dbrajovic
Copy link
Contributor

@dbrajovic dbrajovic commented May 9, 2022

Description

This PR addresses the issue of valid "stuck" transactions. These transactions remain in the pool indefinitely due to being considered recoverable by consensus during block building.

Changes include

  • Bugfix (non-breaking change that solves an issue)
  • Hotfix (change that solves an urgent issue, and requires immediate attention)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)

Checklist

  • I have assigned this PR to myself
  • I have added at least 1 reviewer
  • I have added the relevant labels
  • I have updated the official documentation
  • I have added sufficient documentation in code

Testing

  • I have tested this code with the official test suite
  • I have tested this code manually

Additional comments

Fixes EDGE-45

txpool/txpool.go Outdated Show resolved Hide resolved
Copy link
Contributor

@zivkovicmilos zivkovicmilos left a comment

Choose a reason for hiding this comment

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

Lookin' great 🚀

Copy link
Contributor

@Kourin1996 Kourin1996 left a comment

Choose a reason for hiding this comment

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

Looks good to me

@DarianShawn
Copy link

Good to have this feature. 🙌

Just a little confused because of the method name TxPool.Drop().
DropAccountByTx() would make it more clearer?

@dbrajovic
Copy link
Contributor Author

Good to have this feature. 🙌

Just a little confused because of the method name TxPool.Drop(). DropAccountByTx() would make it more clearer?

I do agree the naming is a bit unclear in the TxPool's API, but it was written that way for consistency. The only reason a transaction is needed as an argument is because address and nonce can be fetched from it.

In a future PR, this naming issue will get tackled.

@dbrajovic dbrajovic merged commit 109ff8d into develop May 11, 2022
@dbrajovic dbrajovic deleted the feature/txpool-self-pruning branch May 11, 2022 12:22
@github-actions github-actions bot locked and limited conversation to collaborators May 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New update to Polygon Edge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Txpool pending and queued sets are never pruned, causing eventual overflow or DOS
5 participants