Skip to content

Remove dup tx check from mempool #1801

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

Closed
edsko opened this issue Mar 13, 2020 · 3 comments · Fixed by #1806
Closed

Remove dup tx check from mempool #1801

edsko opened this issue Mar 13, 2020 · 3 comments · Fixed by #1806
Assignees
Labels
consensus issues related to ouroboros-consensus

Comments

@edsko
Copy link
Contributor

edsko commented Mar 13, 2020

This would first require to fix the tx request client though.

Related but orthogonal: #1743

@edsko edsko added consensus issues related to ouroboros-consensus priority medium labels Mar 13, 2020
@edsko
Copy link
Contributor Author

edsko commented Mar 13, 2020

This should be easy to do now, we can just check the cache from the mempool. It would not reintroduce the "recent txs" cache.

@mrBliss mrBliss added this to the S9 2020-03-26 milestone Mar 13, 2020
@edsko
Copy link
Contributor Author

edsko commented Mar 13, 2020

From slack: the tx submission client:
it should check the mempool whether or not to request a tx at all
and then, I guess, again when it actually gets it
since it might have been inserted in the meantime
that last check is going to be very annoying actually now that I think about it
will be hard to avoid a race condition

@mrBliss
Copy link
Contributor

mrBliss commented Mar 13, 2020

From slack: the tx submission client:
it should check the mempool whether or not to request a tx at all
and then, I guess, again when it actually gets it
since it might have been inserted in the meantime
that last check is going to be very annoying actually now that I think about it
will be hard to avoid a race condition

To expand on the second check: the second check avoids adding a transaction already in the Mempool, which will cause the transaction to be rejected. This will pop up in the trace and might confuse users.

IMO, this is not a big deal, we can just do the first check so we don't request a transaction already in the Mempool. If we then request and receive the same transaction from two nodes, one of them will be rejected, so be it. #1743 will help with that, except that the wallet could still add the same transaction between the time we request it from another node and add it to the Mempool.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus issues related to ouroboros-consensus
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants