Use REJECT_DUPLICATE for already known and conflicted txn #10503
Conversation
|
I would rather remove P2P reject codes; they waste bandwidth and AFAIK aren't actually used for anything except perhaps our tests where they are only moderately helpful due to their narrow bandwidth and instability. (if your change doesn't make tests fail, it would further show their lack of use). |
|
Concept ACK @gmaxwell |
|
utACK 49be502dcc890f5d00930661e7ec926e71489bdb |
|
@laanwj fair request, but I really had hoped to avoid having to think about if additional disclosure hurts privacy for a feature that I think we should be backing away from. OK. |
src/validation.cpp
Outdated
There was a problem hiding this comment.
I would have added braces on this and the other lines that are being changed to update the style.
|
Needs rebase. |
|
Rebased. |
|
I believe there are a few clients out there that do actually use this crap, at least I recall breadwallet did at some point to identify issues with txn they were sending to peers. Probably should go through a more formal announce/BIP process for these kinds of changes, sadly :(. |
|
One thing that this does fix is the fact that a reject message wasn't being sent if a transaction conflicted with another one that was already in the mempool. A reject message should be sent for such a scenario to let the sending peer know that there is a double spend attempt going on. I noticed this while doing some debugging with Armory. |
This change, as it is, just adds visible reject messages for cases where the node is silent now. As it is in the spirit of BIP61 I don't think this requires a new BIP. It would be nice to mention in the release notes, though. |
|
@TheBlueMatt The current implementation violates BIP61, in my interpretation. |
|
utACK d9bec88 |
d9bec88 Use REJECT_DUPLICATE for already known and conflicted txn (Pieter Wuille) Tree-SHA512: adc6dc5caed731c3fd5c8784e8820a074e320360cdb5579c5b9299f9799dd99de60b7382d336ab1909dab8b23e744456d78aa0c3b1c8dd1af3d1b779314cf8fa
…icted txn d9bec88 Use REJECT_DUPLICATE for already known and conflicted txn (Pieter Wuille) Tree-SHA512: adc6dc5caed731c3fd5c8784e8820a074e320360cdb5579c5b9299f9799dd99de60b7382d336ab1909dab8b23e744456d78aa0c3b1c8dd1af3d1b779314cf8fa
7 participants
Per BIP61, reject code 0x12 for transactions corresponds to "input already spent".
We currently have two internal codes (REJECT_CONFLICT and REJECT_ALREADY_KNOWN) for validation failures for transactions that conflict with the mempool, and are already known repectively.
I think that under a reasonable interpretation of BIP61, those failures actually match "input already spent" and should thus trigger a reject code 0x12.
Implement that.
Reported by @achow101 that some failures did not cause a reject message.