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

cryptonote_protocol: prevent duplicate txs in fluff queue #9353

Merged

Conversation

0xFFFC0000
Copy link
Collaborator

@0xFFFC0000 0xFFFC0000 commented Jun 5, 2024

Fix duplicate transaction #9335

Thanks @Boog900 for suggesting how it can be done easier.

There is another (draft) implementation which passes tests based on std::set here.

src/cryptonote_protocol/levin_notify.cpp Outdated Show resolved Hide resolved
@vtnerd
Copy link
Contributor

vtnerd commented Jun 5, 2024

As a clarification - is this a race condition on the txpool? If the tx is already in the pool, it shouldn't re-relay it. But this should be possible if two connections fluff nearly at the same time.

@Boog900
Copy link
Contributor

Boog900 commented Jun 5, 2024

@vtnerd This happens when the tx pool size limit is set too low and you start dropping transactions too soon after receiving them.

When I tested this I set the tx pool limit really low, then sent 2 separate fluff messages of the same tx one after the other (not at the same time) with an extra (not duplicate) tx in the first message, then on another connection monerod sent a duplicate tx in the fluff, which would cause peers to disconnect.

I think this is the cause of monerod struggling to keep connections with a low max txpool size.

@vtnerd
Copy link
Contributor

vtnerd commented Jun 5, 2024

I keep forgetting about that limit. This fix (once corrected) should keep the connections alive from what I've seen (just re-reviewed the tx relay code).

1. Fix duplicate transaction monero-project#9335
2. Add test for cases where there are duplicate transaction in fluff

Co-authored-by: Boog900 <[email protected]>
@0xFFFC0000 0xFFFC0000 force-pushed the dev/0xfffc/unique_fluff_queue branch from 334d050 to 32f3245 Compare June 5, 2024 16:33
@selsta
Copy link
Collaborator

selsta commented Jun 6, 2024

@0xFFFC0000 please also open against release branch

@luigi1111 luigi1111 merged commit 66c5917 into monero-project:master Jul 16, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants