receivePending accepts double spends.

[GoogleCodeExporter]

Wallet.receivePending(Transaction, List<Transaction>) happily accepts double 
spends against transactions in pending.

I can reproduce this by sweeping a paper wallet and then immediately sweeping 
it again (before the UXTO API notices the UXTO change). I think it will create 
an identical transaction except for the signature. At least it spends the same 
UXTOs. The double spend is allowed into the wallet which I think is a bug.

Luckily the double spend is marked as dead as soon as the "real one" confirms. 
So at least the wallet isn't left in an inconsistent state.

Original issue reported on code.google.com by andreas....@gmail.com on 9 Jun 2014 at 5:43

[GoogleCodeExporter]

I think this is the same as issue 533.

Original comment by mh.in.en...@gmail.com on 10 Jun 2014 at 9:49

[GoogleCodeExporter]

Maybe. Looking at the date, issue 533 most likely was caused by tx malleability.

Original comment by andreas....@gmail.com on 10 Jun 2014 at 10:15

[GoogleCodeExporter]

Yes, but it's the same underlying issue: the wallet handles these transactions 
OK but the confidence API has no notion that a tx is "in jeopardy", which is 
what I'd call a tx where we know there's a double spend out there on the 
network.

Someone is developing double spend relay alerts for Core. Assuming that goes 
through OK, we'd need such an API change to be able to expose the fact that 
someone is trying to double spend you with a pending tx.

I think the underlying fix is not very hard.

Original comment by mh.in.en...@gmail.com on 10 Jun 2014 at 10:18

[GoogleCodeExporter]

I just witnessed a much more ugly variant of this problem. This time, I tested 
on prodnet.

I charged a paper wallet from my wallet using this transaction:

https://www.biteasy.com/blockchain/transactions/71b26a4ddfade7b85c113a2446ab007a
c5062feceacb0a8da3da79ea228cebe1

I immediately swiped the paper wallet into my wallet:

https://www.biteasy.com/blockchain/transactions/39ee02332b77b4d3ccbedb5e77569c64
f9c705af6735092ec1f4b8d7d32d4c63

At this point I waited until a block arrived and it contained both of the above 
transactions.
Then I double spent the swiping transaction, using this tx (its obviously not 
in the blockchain):

>>> PENDING:
Sends 0.00 and receives 0.0009, total value 0.0009.
  165ac2b1d79e7184e497bb7218e3a406f4d19594d4d9549a9bce2ea470eeaf03: Pending/unconfirmed.
     in   [3045022100e29346dabc46c56e7bf9a9f9e4090e25212bfdf329f7e6cafc03e1fefc5dba42022073807ad889437519cf722a6e3b6625b05881b0325bc7b99236f95c9a2f81a6cd01] [04c92b6de5e714cd07a42ada90e4cf958350abb23aa971ad4745e89b8c751154ca7d1421d2953960e7fc252ebb45471836b606cd166c683d1ad243f611cc744e6e]
          outpoint:71b26a4ddfade7b85c113a2446ab007ac5062feceacb0a8da3da79ea228cebe1:0 hash160:d5d7402a1ddc4122af07cd7c00ee9ee611389d78
     out  DUP HASH160 [48902cfbfae995dc75d18b25206c2007e5b87a62] EQUALVERIFY CHECKSIG 0.0009 BTC

Immediately, the charging transaction vanished from my wallet! Upon examining 
my wallet dump, I found the following:

Sends 0.00 and receives 0.00, total value 0.00.
  71b26a4ddfade7b85c113a2446ab007ac5062feceacb0a8da3da79ea228cebe1: Appeared in best chain at height -1, depth 0, work done 0.
  INCOMPLETE: No inputs!

So it seems as if receiving double spends can corrupt the wallet!

Original comment by andreas....@gmail.com on 13 Jun 2014 at 8:51

[GoogleCodeExporter]

Here is the relevant part of the log.

Original comment by andreas....@gmail.com on 13 Jun 2014 at 8:57

Attachments:

• wallet.576533555.log

[GoogleCodeExporter]

When you say "then I double spent", what did you do exactly? The wallet won't 
make double spends itself, so you're doing something a bit unusual here. What's 
more the tx didn't propagate as marked by the reject messages in the log, and 
PeerGroup will only commit a TX to the wallet once it's been seen propagating.

I have never seen this problem, but you mentioned you're doing odd things with 
creating partial wallets and inserting data from biteasy, so it may be that you 
found an unusual case we don't properly support.

Original comment by mh.in.en...@gmail.com on 13 Jun 2014 at 9:33

[GoogleCodeExporter]

I've uploaded my branch and the code can be examined here:

https://github.com/schildbach/bitcoin-wallet/tree/sweep-wallet/wallet/src/de/sch
ildbach/wallet/ui/send

Sweeping a paper wallet works like this:

RequestWalletBalanceTask queries the UXTOs and constructs a list of 
transactions that resemble these UXTOs. These txns will have no inputs. There 
can be some dummy outputs inbetween to get the indexes right (I mark them with 
a value of negative one).

SweepWalletFragment puts those txns in a walletToSweep, together with the 
needed private keys. It then executes

SendRequest sendRequest = SendRequest.emptyWallet(selectedAddress); [line 491]

The sendRequest is handed to SendCoinsOfflineTask which basically executes 
(asynchonously)

Transaction sweepTransaction = walletToSweep.sendCoinsOffline(sendRequest); 
[line 58]

This transaction then runs into 
AbstractOnDemandServiceActivity.processDirectTransaction() which has the 
following code:

if (wallet.isTransactionRelevant(tx))
{
  wallet.receivePending(tx, null);
  // plus broadcast here
}

Note that at this point, it's the REAL wallet not the walletToSweep (which may 
be already garbage collected because its not needed any more).

By "double spending" I mean that I just execute this sequence twice. Since the 
BitEasy API currently takes very long to update the UXTOs from unspent to 
spent, it is very easy to test this just by sweeping twice.

Original comment by andreas....@gmail.com on 13 Jun 2014 at 9:54

[GoogleCodeExporter]

Small update: The corrupt wallet did not survive the protobuf serialization 
roundtrip and caused a restore of the auto-backup.

I'm not sure where it got the 835839 hash from. I don't see it in the dump from 
before that crash. The 71b26a hash is for the charging transaction that went 
missing in the UI (was ripped of its inputs).

12:10:42.399 [main] WalletApplication - problem loading wallet
com.google.bitcoin.store.UnreadableWalletException: Transaction did not 
deserialize completely: 
83583903f49568d557738074d6411f830a93d69303da17f8aec439a54c268e7e vs 
71b26a4ddfade7b85c113a2446ab007ac5062feceacb0a8da3da79ea228cebe1
    at com.google.bitcoin.store.WalletProtobufSerializer.readTransaction(WalletProtobufSerializer.java:575) ~[na:0.0]
    at com.google.bitcoin.store.WalletProtobufSerializer.readWallet(WalletProtobufSerializer.java:450) ~[na:0.0]
    at com.google.bitcoin.store.WalletProtobufSerializer.readWallet(WalletProtobufSerializer.java:376) ~[na:0.0]
    at de.schildbach.wallet.WalletApplication.loadWalletFromProtobuf(WalletApplication.java:235) ~[na:0.0]
    at de.schildbach.wallet.WalletApplication.onCreate(WalletApplication.java:129) ~[na:0.0]
    at android.app.Instrumentation.callApplicationOnCreate(Instrumentation.java:1007) ~[na:0.0]
    at android.app.ActivityThread.handleBindApplication(ActivityThread.java:4444) ~[na:0.0]
    at android.app.ActivityThread.access$1300(ActivityThread.java:141) ~[na:0.0]
    at android.app.ActivityThread$H.handleMessage(ActivityThread.java:1316) ~[na:0.0]
    at android.os.Handler.dispatchMessage(Handler.java:99) ~[na:0.0]
    at android.os.Looper.loop(Looper.java:137) ~[na:0.0]
    at android.app.ActivityThread.main(ActivityThread.java:5103) ~[na:0.0]
    at java.lang.reflect.Method.invokeNative(Native Method) ~[na:0.0]
    at java.lang.reflect.Method.invoke(Method.java:525) ~[na:0.0]
    at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:737) ~[na:0.0]
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:553) ~[na:0.0]
    at dalvik.system.NativeStart.main(Native Method) ~[na:0.0]

Original comment by andreas....@gmail.com on 13 Jun 2014 at 11:11