-
Notifications
You must be signed in to change notification settings - Fork 38.7k
Relay first double-spend as alert, notify wallet #4570
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
Conversation
|
Prior development work subsumed in this request: #3883 #4450 #4484 |
|
UI needs to have a more restrictive idea of what is a double-spend. Make it only trigger the UI display of a double-spend if the double-spend doesn't pay you at least as much as the previous tx. Right now this warning will be set off by accident in many cases, for instance wallets like GreenAddress.it where dual-factor controls expire after some time period, allowing the txouts to be spent with more than one scriptSig. |
|
Also there are valid double spend cases employed in contract protocols. For example, payment channels rely on only-A-or-B-is-valid protocol property to guarantee that the payment channel refund transaction OR the payment channel payment transaction is valid, but not both. |
|
@jgarzik Are there double-spend cases where an alert would be undesirable? For a payment channel, merchant would get an alert for the known attack where merchant claims payment close to the refund unlock time, then customer broadcasts the refund transaction as a double-spend as soon as it unlocks. |
|
@petertodd That is a bit of a minefield. Consider these 3 transactions tx2 double-spends both tx1-1 and tx1-2. It pays as much as either one alone, but is not innocuous because it can stop both of them from confirming. What the UI currently does is highlight both tx1-1 and tx1-2 (conflict seen). tx2 is displayed (only because it pays us), and is also highlighted because it is a non-clone conflict. Re: GreenAddress can you connect the dots for me? How does the script getting more permissive imply it's not important to alert if the coin is spent twice? |
|
@dgenr8 I don't think anyone said implementing this correctly would be easy ;) |
|
@luke-jr It wasn't ;P The example can be made arbitrarily complex by making overlapping groups of conflicts. The wording is careful -- another transaction was seen spending the money you think you're getting [unsaid -- there may be a third that you haven't seen]. Best to wait for confirmation. |
|
@dgenr8 You've also forgotten the case there tx2 spends tx1 and tx1 is double-spent. You're code doesn't right now warn users if they're only paid by tx2. Probably better to remove the UI stuff in this patch for now; make it a separate patch. |
|
@dgenr8 Warning users about perfectly reasonable behaviour is not appropriate. And you should always wait for confirmation if you need confirmation, period. |
|
@petertodd Thank you. Proper alerts for a double-spend of an unconfirmed parent will require adding the branch of unconfirmed parents to the wallet at the time the child is first added. I'd like to get feedback on whether anyone sees a problem with this. These parents would be persistent like all wallet txes, and would not be displayed unless they are IsMine/IsFromMe themselves. They were already allowed into the mempool. |
|
Lately, miners are pretty consistently not mining respends if they occur quickly, apparently up to a day or so after the first spend. That is nice, but the network still buries respends. Accepting 0-conf payments is still susceptible to the following attack, even if all miners are nice: When ready to "pay," attacker first transmits a bunch of transactions tx1 paying self, each from a different coin cX. Then, at a succession of time offsets, he transmits a respend of each coin to the target. The goal is for at least one tx2 to be seen by the target first, but the eventual miner second. An additional constant input c0 is included in all the tx2 to ensure that in an adverse scenario for attacker, the target can only be paid once at most. Change is not shown: Respend relay, with a properly monitored alert, is an effective countermeasure to this attack. Target would be alerted almost immediately to withhold goods or services. In addition to tuning the parameters, the attacker can use diverse nodes to gain variability in the timings, helping at least one pair to achieve the goal. |
3c695ea to
0772833
Compare
0772833 to
d17d9cc
Compare
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4570_d17d9ccf44625207415e6450171579ce8351c99d/ for binaries and test log. |
d17d9cc to
016152b
Compare
016152b to
2769c18
Compare
2769c18 to
68af929
Compare
be79ec4 to
4c55985
Compare
|
needs rebase. |
4c55985 to
9a18a42
Compare
|
Some serious problems with this still: respend checking is performed after IsStandard , so a fraudster need only craft his transactions carefully to defeat this entirely; if the order of this is inverted, you then create a situation where it encourages people who want to send real non-IsStandard transactions to do so as a double spend. I think the only way to resolve this is to remove IsStandard from the policy entirely (and hope enough nodes continue to run with this policy). This is potentially very dangerous. Furthermore, it appears an attacker can flood the network with double-spends in order to rate limit relaying of some of them. I also see no efforts made to consider the relay costs: since these are double spends, only one or the other will confirm, and thus the fee from the non-confirming one is never paid. |
|
@luke-jr First to catch up, the "wait for confirmation" part of the alert dialog message was removed. You're right, it's best to stick to the facts. You are also correct that this change does not secure unconfirmed spends, and as currently written, does not help in the construction of a system that does. What this change claims to do, and does, is to stop delaying the moment when interested parties can discover relayable double-spends. The wallet portion also serves the important function of notifying the user of successful double-spends. Even people with diametrically opposite views on whether unconfirmed transactions can ever gain more reliability agree that relaying double-spends, when possible, is an improvement. The reason is that they happened, and that is useful information. |
As an alert, relay at most one transaction that respends a prevout already spent in an unconfirmed transaction in this node's mempool. As before, respends are not added to the mempool. Anti-Denial-of-Service-Attack provisions: - Use a bloom filter to relay only one respend per mempool prevout - Rate-limit respend relays to a default of 100 thousand bytes/minute - Define tx2.IsEquivalentTo(tx1): equality when scriptSigs are not considered - Do not relay these equivalent transactions - Do not allow relayed respends into our wallet unless they conflict with it, even if they pay us. This addresses a resource exhaustion attack CWallet::SyncMetaData is changed to sync only to equivalent transactions. Remove an unused variable declaration in txmempool.cpp.
Respend transactions that conflict with transactions already in the wallet are added to it. They are not displayed unless they also involve the wallet, or get into a block. If they do not involve the wallet, they continue not to affect balance. Transactions that involve the wallet, and have conflicting non-equivalent transactions, are highlighted in red. When the conflict first occurs, a modal dialog is thrown. When a conflict is added to the wallet, counter nConflictsReceived is incremented. This acts like a change in active block height for the purpose of triggering UI updates.
-respendnotify=<cmd> Execute command when a network tx respends wallet tx input (%s=respend TxID, %t=wallet TxID) Add respendsobserved array to gettransaction, listtransactions, and listsinceblock RPCs. This omits the malleability clones that are included in the walletconflicts array. Add RPC help for respendsobserved and walletconflicts (help was missing for the latter). New test txn_doublespendrelay.py uses a 4-node network to test 5 relay scenarios. New test txn_clone does what the old txnmall.sh test did. It creates an equivalent malleated clone and tests that SyncMetaData syncs the accounting effects from the original transaction to the confirmed clone. Existing test txn_doublespend.py is changed to remove reliance on accounting "move" ledger entries and on broken SyncMetaData. Instead, expect double-spend amount to be debited from the default "" account.
9a18a42 to
d9351ee
Compare
|
Rebased. There is a question of where to put the equivalent-tx-check function. At the moment there are identical versions in CTransaction and CWalletTx. See #6365. |
|
Observations on this pull request:
Going to close. That is perhaps mildly controversial given the current core/xt code fork debates, but it seems within bounds of reasonable project management. We can re-open if ACKs suddenly appear or concerns are alleviated, etc. The overall motivation is simply janitorial, closing pull requests that aren't moving after a long time. |
Relay at most one double-spend per unspent output, subject to anti-DoS provisions.
Unconfirmed double-spends carry information that is time-critical to the payee of the first spend, but today, they are silently dropped.
Add a -respendnotify option that can immediately notify a user-supplied command when a double-spend affecting the wallet is received.
Change the GUI wallet to highlight a transaction in red when a transaction that double-spends it has been seen. When the conflict first occurs, display a warning dialog.
This change has been running on mainnet since 2014-05-02. Relayable double-spends seen by the change are recorded here in realtime.