-
Notifications
You must be signed in to change notification settings - Fork 120
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
Add expired transactions to the mempool rejected list #2852
Conversation
Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, we could merge it as-is.
I've made some suggestions about comments that could change.
I also suggested a refactor to remove near-duplicate code.
Please feel free to merge with or without these changes - no need to wait for my re-approval. I think this PR might conflict with ticket #2780, so we should try to get it merged on Monday. |
@conradoplg commit 51e59ae in PR #2855 resolves a tricky conflict between this PR and PR #2826. You might want to cherry-pick it here, after you review it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed comments! @dconnolly can you take a look per @teor2345's comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, let's merge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Motivation
When a mempool transaction expires, we remove it from the mempool. But we should also add it to the rejected list, to avoid redownloading and reverifying it if it's sent again.
Specifications
Designs
Solution
chain_rejected_same_effects
into two lists, to keep evicted and reject txs apart. (This was done by adding the lists to a HashMap, keyed by their respective error cases. This improved type safety a bit compared to just creating two separate lists)Closes #2848
Review
Anyone can review this.
Reviewer Checklist
Follow Up Work