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

Invalid malleated txs are not removed from the mempool cache #788

Closed
evan-forbes opened this issue Jun 14, 2022 · 0 comments · Fixed by #834
Closed

Invalid malleated txs are not removed from the mempool cache #788

evan-forbes opened this issue Jun 14, 2022 · 0 comments · Fixed by #834
Labels
good first issue Good for newcomers low priority T:Bug Type: Bug (confirmed)

Comments

@evan-forbes
Copy link
Member

evan-forbes commented Jun 14, 2022

When removing malleated txs from the mempool via the mempool's Update method, if the config indicates that the the node should not keep invalid txs in the mempool cache, then it should remove those txs from the cache if they are included in a block. This is not currently occuring because the mempool cache only accepts raw txs and then hashes them itself as opposed to accepting the hashes of the txs.

} else if !txmp.config.KeepInvalidTxsInCache {
// allow invalid transactions to be re-submitted
txmp.cache.Remove(tx)

I don't think this is causing any glaring issues at the moment, but it is effectively overriding the node operator's configuration, which we should eventually fix. To fix this, we need to add a method to the cache to remove a tx by its hash instead of the tx itself. That way we can pass the original tx's hash.

} else if originalHash, _, isMalleated := types.UnwrapMalleatedTx(tx); isMalleated {

@evan-forbes evan-forbes added T:Bug Type: Bug (confirmed) low priority good first issue Good for newcomers labels Jun 14, 2022
cmwaters pushed a commit that referenced this issue Sep 20, 2023
(cherry picked from commit ecd5ee112533cda28900cbd75afb349f67da3fa5)

Co-authored-by: Sergio Mena <sergio@informal.systems>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers low priority T:Bug Type: Bug (confirmed)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant