Skip to content

Conversation

@dgenr8
Copy link
Contributor

@dgenr8 dgenr8 commented Jul 22, 2014

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.

alertshot

This change has been running on mainnet since 2014-05-02. Relayable double-spends seen by the change are recorded here in realtime.

@dgenr8
Copy link
Contributor Author

dgenr8 commented Jul 22, 2014

Prior development work subsumed in this request: #3883 #4450 #4484
Guidance and contributions so far from the following: @gavinandresen @mikehearn @petertodd @SergioDemianLerner @laanwj @sipa @jgarzik @gmaxwell @Drak @Diapolo

@dgenr8 dgenr8 changed the title Relay first double-spend as alert, and notify Relay first double-spend as alert, notify wallet Jul 22, 2014
@petertodd
Copy link
Contributor

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.

@jgarzik
Copy link
Contributor

jgarzik commented Jul 22, 2014

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.

@dgenr8
Copy link
Contributor Author

dgenr8 commented Jul 22, 2014

@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.

@dgenr8
Copy link
Contributor Author

dgenr8 commented Jul 23, 2014

@petertodd That is a bit of a minefield. Consider these 3 transactions

 In       Paysme
tx1-1
 coin1      1

tx1-2
 coin2      1

tx2
 coin1      1
 coin2

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?

@luke-jr
Copy link
Member

luke-jr commented Jul 23, 2014

@dgenr8 I don't think anyone said implementing this correctly would be easy ;)

@dgenr8
Copy link
Contributor Author

dgenr8 commented Jul 23, 2014

@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.

@petertodd
Copy link
Contributor

@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.

@luke-jr
Copy link
Member

luke-jr commented Jul 23, 2014

@dgenr8 Warning users about perfectly reasonable behaviour is not appropriate. And you should always wait for confirmation if you need confirmation, period.

@dgenr8
Copy link
Contributor Author

dgenr8 commented Jul 23, 2014

@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.

@dgenr8
Copy link
Contributor Author

dgenr8 commented Aug 23, 2014

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:

respends=20  offset=.40s  offset increment=.05s

tx1               elapsed        tx2
c1 -> self          .40s         c1 + c0 -> target
c2 -> self          .45s         c2 + c0 -> target
...
c20 -> self        1.35s         c20 + c0 -> target

Worst-case value to attacker: -20*fee
Best-case value to attacker: (value of goods or services) - 19*fee

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.

@dgenr8 dgenr8 force-pushed the relay_first_respend branch 3 times, most recently from 3c695ea to 0772833 Compare August 30, 2014 01:12
@dgenr8 dgenr8 force-pushed the relay_first_respend branch from 0772833 to d17d9cc Compare September 20, 2014 20:54
@BitcoinPullTester
Copy link

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4570_d17d9ccf44625207415e6450171579ce8351c99d/ for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.

@dgenr8 dgenr8 force-pushed the relay_first_respend branch from d17d9cc to 016152b Compare October 8, 2014 00:13
@dgenr8 dgenr8 force-pushed the relay_first_respend branch from 016152b to 2769c18 Compare December 27, 2014 02:00
@dgenr8 dgenr8 force-pushed the relay_first_respend branch from 2769c18 to 68af929 Compare February 10, 2015 03:02
@dgenr8 dgenr8 force-pushed the relay_first_respend branch 2 times, most recently from be79ec4 to 4c55985 Compare February 26, 2015 04:47
@jonasschnelli
Copy link
Contributor

needs rebase.

@dgenr8 dgenr8 force-pushed the relay_first_respend branch from 4c55985 to 9a18a42 Compare March 12, 2015 05:12
@luke-jr
Copy link
Member

luke-jr commented Mar 13, 2015

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.

@dgenr8
Copy link
Contributor Author

dgenr8 commented Mar 16, 2015

@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.

dgenr8 added 4 commits July 12, 2015 11:28
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.
@dgenr8 dgenr8 force-pushed the relay_first_respend branch from 9a18a42 to d9351ee Compare July 13, 2015 00:39
@dgenr8
Copy link
Contributor Author

dgenr8 commented Jul 13, 2015

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.

@jgarzik
Copy link
Contributor

jgarzik commented Jul 23, 2015

Observations on this pull request:

  • Zero ACKs after a year
  • Discussion on mailing list, here and IRC indicates concerns remain

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.

@jgarzik jgarzik closed this Jul 23, 2015
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants