From 726c7291ace0b012c955134ab8c21c01bd0ae96b Mon Sep 17 00:00:00 2001 From: Andreas Schildbach Date: Sun, 14 Jul 2019 18:29:59 +0200 Subject: [PATCH] Wallet: Remove global coinSelector and allowSpendingUnconfirmedTransactions(). Coin selection is a per SendRequest, per createSend() or per getBalance() call affair. Having it wallet-global can lead to race conditions, as sometimes experienced in the WalletTest unit tests. --- .../main/java/org/bitcoinj/wallet/Wallet.java | 68 ++- .../org/bitcoinj/core/ChainSplitTest.java | 5 +- .../bitcoinj/core/TransactionInputTest.java | 5 +- .../java/org/bitcoinj/wallet/WalletTest.java | 518 ++++++++---------- .../java/org/bitcoinj/tools/WalletTool.java | 2 +- .../src/main/java/wallettemplate/Main.java | 3 - .../wallettemplate/SendMoneyController.java | 3 + 7 files changed, 291 insertions(+), 313 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/wallet/Wallet.java b/core/src/main/java/org/bitcoinj/wallet/Wallet.java index 6c010a104..cd1397a64 100644 --- a/core/src/main/java/org/bitcoinj/wallet/Wallet.java +++ b/core/src/main/java/org/bitcoinj/wallet/Wallet.java @@ -230,7 +230,7 @@ public class Wallet extends BaseTaggableObject // that was created after it. Useful when you believe some keys have been compromised. private volatile long vKeyRotationTimestamp; - protected CoinSelector coinSelector = DefaultCoinSelector.get(); + protected final CoinSelector coinSelector = DefaultCoinSelector.get(); // The wallet version. This is an int that can be used to track breaking changes in the wallet format. // You can also use it to detect wallets that come from the future (ie they contain features you @@ -3952,9 +3952,49 @@ public class Wallet extends BaseTaggableObject */ public Transaction createSend(Address address, Coin value) throws InsufficientMoneyException, BadWalletEncryptionKeyException { + return createSend(address, value, false); + } + + /** + *

Statelessly creates a transaction that sends the given value to address. The change is sent to + * {@link Wallet#currentChangeAddress()}, so you must have added at least one key.

+ * + *

If you just want to send money quickly, you probably want + * {@link Wallet#sendCoins(TransactionBroadcaster, Address, Coin)} instead. That will create the sending + * transaction, commit to the wallet and broadcast it to the network all in one go. This method is lower level + * and lets you see the proposed transaction before anything is done with it.

+ * + *

This is a helper method that is equivalent to using {@link SendRequest#to(Address, Coin)} + * followed by {@link Wallet#completeTx(SendRequest)} and returning the requests transaction object. + * Note that this means a fee may be automatically added if required, if you want more control over the process, + * just do those two steps yourself.

+ * + *

IMPORTANT: This method does NOT update the wallet. If you call createSend again you may get two transactions + * that spend the same coins. You have to call {@link Wallet#commitTx(Transaction)} on the created transaction to + * prevent this, but that should only occur once the transaction has been accepted by the network. This implies + * you cannot have more than one outstanding sending tx at once.

+ * + *

You MUST ensure that the value is not smaller than {@link TransactionOutput#getMinNonDustValue()} or the transaction + * will almost certainly be rejected by the network as dust.

+ * + * @param address The Bitcoin address to send the money to. + * @param value How much currency to send. + * @param allowUnconfirmed Wether to allow spending unconfirmed outputs. + * @return either the created Transaction or null if there are insufficient coins. + * @throws InsufficientMoneyException if the request could not be completed due to not enough balance. + * @throws DustySendRequested if the resultant transaction would violate the dust rules. + * @throws CouldNotAdjustDownwards if emptying the wallet was requested and the output can't be shrunk for fees without violating a protocol rule. + * @throws ExceededMaxTransactionSize if the resultant transaction is too big for Bitcoin to process. + * @throws MultipleOpReturnRequested if there is more than one OP_RETURN output for the resultant transaction. + * @throws BadWalletEncryptionKeyException if the supplied {@link SendRequest#aesKey} is wrong. + */ + public Transaction createSend(Address address, Coin value, boolean allowUnconfirmed) + throws InsufficientMoneyException, BadWalletEncryptionKeyException { SendRequest req = SendRequest.to(address, value); if (params.getId().equals(NetworkParameters.ID_UNITTESTNET)) req.shuffleOutputs = false; + if (allowUnconfirmed) + req.allowUnconfirmed(); completeTx(req); return req.tx; } @@ -4471,7 +4511,7 @@ public class Wallet extends BaseTaggableObject return candidates; } - /** Returns the {@link CoinSelector} object which controls which outputs can be spent by this wallet. */ + /** Returns the default {@link CoinSelector} object that is used by this wallet if no custom selector is specified. */ public CoinSelector getCoinSelector() { lock.lock(); try { @@ -4481,30 +4521,6 @@ public class Wallet extends BaseTaggableObject } } - /** - * A coin selector is responsible for choosing which outputs to spend when creating transactions. The default - * selector implements a policy of spending transactions that appeared in the best chain and pending transactions - * that were created by this wallet, but not others. You can override the coin selector for any given send - * operation by changing {@link SendRequest#coinSelector}. - */ - public void setCoinSelector(CoinSelector coinSelector) { - lock.lock(); - try { - this.coinSelector = checkNotNull(coinSelector); - } finally { - lock.unlock(); - } - } - - /** - * Convenience wrapper for {@code setCoinSelector(Wallet.AllowUnconfirmedCoinSelector.get())}. If this method - * is called on the wallet then transactions will be used for spending regardless of their confidence. This can - * be dangerous - only use this if you absolutely know what you're doing! - */ - public void allowSpendingUnconfirmedTransactions() { - setCoinSelector(AllowUnconfirmedCoinSelector.get()); - } - /** * Get the {@link UTXOProvider}. * @return The UTXO provider. diff --git a/core/src/test/java/org/bitcoinj/core/ChainSplitTest.java b/core/src/test/java/org/bitcoinj/core/ChainSplitTest.java index 0c0e7d0dd..081563df6 100644 --- a/core/src/test/java/org/bitcoinj/core/ChainSplitTest.java +++ b/core/src/test/java/org/bitcoinj/core/ChainSplitTest.java @@ -530,10 +530,9 @@ public class ChainSplitTest { chain.add(b1); // Send a couple of payments one after the other (so the second depends on the change output of the first). - wallet.allowSpendingUnconfirmedTransactions(); - Transaction t2 = checkNotNull(wallet.createSend(LegacyAddress.fromKey(UNITTEST, new ECKey()), CENT)); + Transaction t2 = checkNotNull(wallet.createSend(LegacyAddress.fromKey(UNITTEST, new ECKey()), CENT, true)); wallet.commitTx(t2); - Transaction t3 = checkNotNull(wallet.createSend(LegacyAddress.fromKey(UNITTEST, new ECKey()), CENT)); + Transaction t3 = checkNotNull(wallet.createSend(LegacyAddress.fromKey(UNITTEST, new ECKey()), CENT, true)); wallet.commitTx(t3); chain.add(FakeTxBuilder.makeSolvedTestBlock(b1, t2, t3)); diff --git a/core/src/test/java/org/bitcoinj/core/TransactionInputTest.java b/core/src/test/java/org/bitcoinj/core/TransactionInputTest.java index 291b682d7..fdec79155 100644 --- a/core/src/test/java/org/bitcoinj/core/TransactionInputTest.java +++ b/core/src/test/java/org/bitcoinj/core/TransactionInputTest.java @@ -38,13 +38,14 @@ public class TransactionInputTest { @Test public void testStandardWalletDisconnect() throws Exception { Wallet w = Wallet.createDeterministic(new Context(UNITTEST), Script.ScriptType.P2PKH); - w.setCoinSelector(new AllowUnconfirmedCoinSelector()); Address a = w.currentReceiveAddress(); Transaction tx1 = FakeTxBuilder.createFakeTxWithoutChangeAddress(UNITTEST, Coin.COIN, a); w.receivePending(tx1, null); Transaction tx2 = new Transaction(UNITTEST); tx2.addOutput(Coin.valueOf(99000000), new ECKey()); - w.completeTx(SendRequest.forTx(tx2)); + SendRequest req = SendRequest.forTx(tx2); + req.allowUnconfirmed(); + w.completeTx(req); TransactionInput txInToDisconnect = tx2.getInput(0); diff --git a/core/src/test/java/org/bitcoinj/wallet/WalletTest.java b/core/src/test/java/org/bitcoinj/wallet/WalletTest.java index b1cf8e040..8c4b0c962 100644 --- a/core/src/test/java/org/bitcoinj/wallet/WalletTest.java +++ b/core/src/test/java/org/bitcoinj/wallet/WalletTest.java @@ -423,7 +423,7 @@ public class WalletTest extends TestWithWallet { Threading.waitForUserCode(); final ListenableFuture depthFuture = t1.getConfidence().getDepthFuture(1); assertFalse(depthFuture.isDone()); - assertEquals(ZERO, wallet.getBalance()); + assertEquals(ZERO, wallet.getBalance(Wallet.BalanceType.AVAILABLE)); assertEquals(amount, wallet.getBalance(Wallet.BalanceType.ESTIMATED)); assertFalse(availFuture.isDone()); // Our estimated balance has reached the requested level. @@ -842,12 +842,11 @@ public class WalletTest extends TestWithWallet { buf[43] = 0; // Break the signature: bitcoinj won't check in SPV mode and this is easier than other mutations. send1 = UNITTEST.getDefaultSerializer().makeTransaction(buf); wallet.commitTx(send2); - wallet.allowSpendingUnconfirmedTransactions(); - assertEquals(value, wallet.getBalance(Wallet.BalanceType.ESTIMATED)); + assertEquals(value, wallet.getBalance(BalanceType.ESTIMATED)); // Now spend the change. This transaction should die permanently when the mutant appears in the chain. - Transaction send3 = checkNotNull(wallet.createSend(OTHER_ADDRESS, value)); + Transaction send3 = checkNotNull(wallet.createSend(OTHER_ADDRESS, value, true)); wallet.commitTx(send3); - assertEquals(ZERO, wallet.getBalance()); + assertEquals(ZERO, wallet.getBalance(BalanceType.AVAILABLE)); final LinkedList dead = new LinkedList<>(); final TransactionConfidence.Listener listener = new TransactionConfidence.Listener() { @Override @@ -862,7 +861,7 @@ public class WalletTest extends TestWithWallet { // Double spend! sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, send1); // Back to having one coin. - assertEquals(value, wallet.getBalance()); + assertEquals(value, wallet.getBalance(BalanceType.AVAILABLE)); assertEquals(send2.getTxId(), dead.poll().getTransactionHash()); assertEquals(send3.getTxId(), dead.poll().getTransactionHash()); } @@ -951,180 +950,171 @@ public class WalletTest extends TestWithWallet { // // txA1 is in conflict with txA2 and txA3. txB1 is in conflict with txB2. - CoinSelector originalCoinSelector = wallet.getCoinSelector(); - try { - wallet.allowSpendingUnconfirmedTransactions(); + Transaction txARoot = sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(10, 0)); + SendRequest a1Req = SendRequest.to(OTHER_ADDRESS, valueOf(1, 0)); + a1Req.tx.addInput(txARoot.getOutput(0)); + a1Req.shuffleOutputs = false; + wallet.completeTx(a1Req); + Transaction txA1 = a1Req.tx; + SendRequest a2Req = SendRequest.to(OTHER_ADDRESS, valueOf(2, 0)); + a2Req.tx.addInput(txARoot.getOutput(0)); + a2Req.shuffleOutputs = false; + wallet.completeTx(a2Req); + Transaction txA2 = a2Req.tx; + SendRequest a3Req = SendRequest.to(OTHER_ADDRESS, valueOf(3, 0)); + a3Req.tx.addInput(txARoot.getOutput(0)); + a3Req.shuffleOutputs = false; + wallet.completeTx(a3Req); + Transaction txA3 = a3Req.tx; + wallet.commitTx(txA1); + wallet.commitTx(txA2); + wallet.commitTx(txA3); - Transaction txARoot = sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(10, 0)); - SendRequest a1Req = SendRequest.to(OTHER_ADDRESS, valueOf(1, 0)); - a1Req.tx.addInput(txARoot.getOutput(0)); - a1Req.shuffleOutputs = false; - wallet.completeTx(a1Req); - Transaction txA1 = a1Req.tx; - SendRequest a2Req = SendRequest.to(OTHER_ADDRESS, valueOf(2, 0)); - a2Req.tx.addInput(txARoot.getOutput(0)); - a2Req.shuffleOutputs = false; - wallet.completeTx(a2Req); - Transaction txA2 = a2Req.tx; - SendRequest a3Req = SendRequest.to(OTHER_ADDRESS, valueOf(3, 0)); - a3Req.tx.addInput(txARoot.getOutput(0)); - a3Req.shuffleOutputs = false; - wallet.completeTx(a3Req); - Transaction txA3 = a3Req.tx; - wallet.commitTx(txA1); - wallet.commitTx(txA2); - wallet.commitTx(txA3); + Transaction txBRoot = sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(100, 0)); + SendRequest b1Req = SendRequest.to(OTHER_ADDRESS, valueOf(11, 0)); + b1Req.tx.addInput(txBRoot.getOutput(0)); + b1Req.shuffleOutputs = false; + wallet.completeTx(b1Req); + Transaction txB1 = b1Req.tx; + SendRequest b2Req = SendRequest.to(OTHER_ADDRESS, valueOf(22, 0)); + b2Req.tx.addInput(txBRoot.getOutput(0)); + b2Req.shuffleOutputs = false; + wallet.completeTx(b2Req); + Transaction txB2 = b2Req.tx; + wallet.commitTx(txB1); + wallet.commitTx(txB2); - Transaction txBRoot = sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(100, 0)); - SendRequest b1Req = SendRequest.to(OTHER_ADDRESS, valueOf(11, 0)); - b1Req.tx.addInput(txBRoot.getOutput(0)); - b1Req.shuffleOutputs = false; - wallet.completeTx(b1Req); - Transaction txB1 = b1Req.tx; - SendRequest b2Req = SendRequest.to(OTHER_ADDRESS, valueOf(22, 0)); - b2Req.tx.addInput(txBRoot.getOutput(0)); - b2Req.shuffleOutputs = false; - wallet.completeTx(b2Req); - Transaction txB2 = b2Req.tx; - wallet.commitTx(txB1); - wallet.commitTx(txB2); + SendRequest c1Req = SendRequest.to(OTHER_ADDRESS, valueOf(0, 10)); + c1Req.tx.addInput(txA1.getOutput(1)); + c1Req.tx.addInput(txB1.getOutput(1)); + c1Req.shuffleOutputs = false; + wallet.completeTx(c1Req); + Transaction txC1 = c1Req.tx; + SendRequest c2Req = SendRequest.to(OTHER_ADDRESS, valueOf(0, 20)); + c2Req.tx.addInput(txA2.getOutput(1)); + c2Req.tx.addInput(txB2.getOutput(1)); + c2Req.shuffleOutputs = false; + wallet.completeTx(c2Req); + Transaction txC2 = c2Req.tx; + wallet.commitTx(txC1); + wallet.commitTx(txC2); - SendRequest c1Req = SendRequest.to(OTHER_ADDRESS, valueOf(0, 10)); - c1Req.tx.addInput(txA1.getOutput(1)); - c1Req.tx.addInput(txB1.getOutput(1)); - c1Req.shuffleOutputs = false; - wallet.completeTx(c1Req); - Transaction txC1 = c1Req.tx; - SendRequest c2Req = SendRequest.to(OTHER_ADDRESS, valueOf(0, 20)); - c2Req.tx.addInput(txA2.getOutput(1)); - c2Req.tx.addInput(txB2.getOutput(1)); - c2Req.shuffleOutputs = false; - wallet.completeTx(c2Req); - Transaction txC2 = c2Req.tx; - wallet.commitTx(txC1); - wallet.commitTx(txC2); + SendRequest d1Req = SendRequest.to(OTHER_ADDRESS, valueOf(0, 1)); + d1Req.tx.addInput(txC1.getOutput(1)); + d1Req.shuffleOutputs = false; + wallet.completeTx(d1Req); + Transaction txD1 = d1Req.tx; + SendRequest d2Req = SendRequest.to(OTHER_ADDRESS, valueOf(0, 2)); + d2Req.tx.addInput(txC2.getOutput(1)); + d2Req.shuffleOutputs = false; + wallet.completeTx(d2Req); + Transaction txD2 = d2Req.tx; + wallet.commitTx(txD1); + wallet.commitTx(txD2); - SendRequest d1Req = SendRequest.to(OTHER_ADDRESS, valueOf(0, 1)); - d1Req.tx.addInput(txC1.getOutput(1)); - d1Req.shuffleOutputs = false; - wallet.completeTx(d1Req); - Transaction txD1 = d1Req.tx; - SendRequest d2Req = SendRequest.to(OTHER_ADDRESS, valueOf(0, 2)); - d2Req.tx.addInput(txC2.getOutput(1)); - d2Req.shuffleOutputs = false; - wallet.completeTx(d2Req); - Transaction txD2 = d2Req.tx; - wallet.commitTx(txD1); - wallet.commitTx(txD2); + assertInConflict(txA1); + assertInConflict(txA2); + assertInConflict(txA3); + assertInConflict(txB1); + assertInConflict(txB2); + assertInConflict(txC1); + assertInConflict(txC2); + assertInConflict(txD1); + assertInConflict(txD2); - assertInConflict(txA1); - assertInConflict(txA2); - assertInConflict(txA3); - assertInConflict(txB1); - assertInConflict(txB2); - assertInConflict(txC1); - assertInConflict(txC2); - assertInConflict(txD1); - assertInConflict(txD2); + // Add a block to the block store. The rest of the blocks in this test will be on top of this one. + FakeTxBuilder.BlockPair blockPair0 = createFakeBlock(blockStore, 1); - // Add a block to the block store. The rest of the blocks in this test will be on top of this one. - FakeTxBuilder.BlockPair blockPair0 = createFakeBlock(blockStore, 1); + // A block was mined including txA1 + FakeTxBuilder.BlockPair blockPair1 = createFakeBlock(blockStore, 2, txA1); + wallet.receiveFromBlock(txA1, blockPair1.storedBlock, AbstractBlockChain.NewBlockType.BEST_CHAIN, 0); + wallet.notifyNewBestBlock(blockPair1.storedBlock); + assertSpent(txA1); + assertDead(txA2); + assertDead(txA3); + assertInConflict(txB1); + assertInConflict(txB2); + assertInConflict(txC1); + assertDead(txC2); + assertInConflict(txD1); + assertDead(txD2); - // A block was mined including txA1 - FakeTxBuilder.BlockPair blockPair1 = createFakeBlock(blockStore, 2, txA1); - wallet.receiveFromBlock(txA1, blockPair1.storedBlock, AbstractBlockChain.NewBlockType.BEST_CHAIN, 0); - wallet.notifyNewBestBlock(blockPair1.storedBlock); - assertSpent(txA1); - assertDead(txA2); - assertDead(txA3); - assertInConflict(txB1); - assertInConflict(txB2); - assertInConflict(txC1); - assertDead(txC2); - assertInConflict(txD1); - assertDead(txD2); + // A reorg: previous block "replaced" by new block containing txA1 and txB1 + FakeTxBuilder.BlockPair blockPair2 = createFakeBlock(blockStore, blockPair0.storedBlock, 2, txA1, txB1); + wallet.receiveFromBlock(txA1, blockPair2.storedBlock, AbstractBlockChain.NewBlockType.SIDE_CHAIN, 0); + wallet.receiveFromBlock(txB1, blockPair2.storedBlock, AbstractBlockChain.NewBlockType.SIDE_CHAIN, 1); + wallet.reorganize(blockPair0.storedBlock, Lists.newArrayList(blockPair1.storedBlock), + Lists.newArrayList(blockPair2.storedBlock)); + assertSpent(txA1); + assertDead(txA2); + assertDead(txA3); + assertSpent(txB1); + assertDead(txB2); + assertPending(txC1); + assertDead(txC2); + assertPending(txD1); + assertDead(txD2); - // A reorg: previous block "replaced" by new block containing txA1 and txB1 - FakeTxBuilder.BlockPair blockPair2 = createFakeBlock(blockStore, blockPair0.storedBlock, 2, txA1, txB1); - wallet.receiveFromBlock(txA1, blockPair2.storedBlock, AbstractBlockChain.NewBlockType.SIDE_CHAIN, 0); - wallet.receiveFromBlock(txB1, blockPair2.storedBlock, AbstractBlockChain.NewBlockType.SIDE_CHAIN, 1); - wallet.reorganize(blockPair0.storedBlock, Lists.newArrayList(blockPair1.storedBlock), - Lists.newArrayList(blockPair2.storedBlock)); - assertSpent(txA1); - assertDead(txA2); - assertDead(txA3); - assertSpent(txB1); - assertDead(txB2); - assertPending(txC1); - assertDead(txC2); - assertPending(txD1); - assertDead(txD2); + // A reorg: previous block "replaced" by new block containing txA1, txB1 and txC1 + FakeTxBuilder.BlockPair blockPair3 = createFakeBlock(blockStore, blockPair0.storedBlock, 2, txA1, txB1, txC1); + wallet.receiveFromBlock(txA1, blockPair3.storedBlock, AbstractBlockChain.NewBlockType.SIDE_CHAIN, 0); + wallet.receiveFromBlock(txB1, blockPair3.storedBlock, AbstractBlockChain.NewBlockType.SIDE_CHAIN, 1); + wallet.receiveFromBlock(txC1, blockPair3.storedBlock, AbstractBlockChain.NewBlockType.SIDE_CHAIN, 2); + wallet.reorganize(blockPair0.storedBlock, Lists.newArrayList(blockPair2.storedBlock), + Lists.newArrayList(blockPair3.storedBlock)); + assertSpent(txA1); + assertDead(txA2); + assertDead(txA3); + assertSpent(txB1); + assertDead(txB2); + assertSpent(txC1); + assertDead(txC2); + assertPending(txD1); + assertDead(txD2); - // A reorg: previous block "replaced" by new block containing txA1, txB1 and txC1 - FakeTxBuilder.BlockPair blockPair3 = createFakeBlock(blockStore, blockPair0.storedBlock, 2, txA1, txB1, - txC1); - wallet.receiveFromBlock(txA1, blockPair3.storedBlock, AbstractBlockChain.NewBlockType.SIDE_CHAIN, 0); - wallet.receiveFromBlock(txB1, blockPair3.storedBlock, AbstractBlockChain.NewBlockType.SIDE_CHAIN, 1); - wallet.receiveFromBlock(txC1, blockPair3.storedBlock, AbstractBlockChain.NewBlockType.SIDE_CHAIN, 2); - wallet.reorganize(blockPair0.storedBlock, Lists.newArrayList(blockPair2.storedBlock), - Lists.newArrayList(blockPair3.storedBlock)); - assertSpent(txA1); - assertDead(txA2); - assertDead(txA3); - assertSpent(txB1); - assertDead(txB2); - assertSpent(txC1); - assertDead(txC2); - assertPending(txD1); - assertDead(txD2); + // A reorg: previous block "replaced" by new block containing txB1 + FakeTxBuilder.BlockPair blockPair4 = createFakeBlock(blockStore, blockPair0.storedBlock, 2, txB1); + wallet.receiveFromBlock(txB1, blockPair4.storedBlock, AbstractBlockChain.NewBlockType.SIDE_CHAIN, 0); + wallet.reorganize(blockPair0.storedBlock, Lists.newArrayList(blockPair3.storedBlock), + Lists.newArrayList(blockPair4.storedBlock)); + assertPending(txA1); + assertDead(txA2); + assertDead(txA3); + assertSpent(txB1); + assertDead(txB2); + assertPending(txC1); + assertDead(txC2); + assertPending(txD1); + assertDead(txD2); - // A reorg: previous block "replaced" by new block containing txB1 - FakeTxBuilder.BlockPair blockPair4 = createFakeBlock(blockStore, blockPair0.storedBlock, 2, txB1); - wallet.receiveFromBlock(txB1, blockPair4.storedBlock, AbstractBlockChain.NewBlockType.SIDE_CHAIN, 0); - wallet.reorganize(blockPair0.storedBlock, Lists.newArrayList(blockPair3.storedBlock), - Lists.newArrayList(blockPair4.storedBlock)); - assertPending(txA1); - assertDead(txA2); - assertDead(txA3); - assertSpent(txB1); - assertDead(txB2); - assertPending(txC1); - assertDead(txC2); - assertPending(txD1); - assertDead(txD2); - - // A reorg: previous block "replaced" by new block containing txA2 - FakeTxBuilder.BlockPair blockPair5 = createFakeBlock(blockStore, blockPair0.storedBlock, 2, txA2); - wallet.receiveFromBlock(txA2, blockPair5.storedBlock, AbstractBlockChain.NewBlockType.SIDE_CHAIN, 0); - wallet.reorganize(blockPair0.storedBlock, Lists.newArrayList(blockPair4.storedBlock), - Lists.newArrayList(blockPair5.storedBlock)); - assertDead(txA1); - assertUnspent(txA2); - assertDead(txA3); - assertPending(txB1); - assertDead(txB2); - assertDead(txC1); - assertDead(txC2); - assertDead(txD1); - assertDead(txD2); - - // A reorg: previous block "replaced" by new empty block - FakeTxBuilder.BlockPair blockPair6 = createFakeBlock(blockStore, blockPair0.storedBlock, 2); - wallet.reorganize(blockPair0.storedBlock, Lists.newArrayList(blockPair5.storedBlock), - Lists.newArrayList(blockPair6.storedBlock)); - assertDead(txA1); - assertPending(txA2); - assertDead(txA3); - assertPending(txB1); - assertDead(txB2); - assertDead(txC1); - assertDead(txC2); - assertDead(txD1); - assertDead(txD2); - } finally { - wallet.setCoinSelector(originalCoinSelector); - } + // A reorg: previous block "replaced" by new block containing txA2 + FakeTxBuilder.BlockPair blockPair5 = createFakeBlock(blockStore, blockPair0.storedBlock, 2, txA2); + wallet.receiveFromBlock(txA2, blockPair5.storedBlock, AbstractBlockChain.NewBlockType.SIDE_CHAIN, 0); + wallet.reorganize(blockPair0.storedBlock, Lists.newArrayList(blockPair4.storedBlock), + Lists.newArrayList(blockPair5.storedBlock)); + assertDead(txA1); + assertUnspent(txA2); + assertDead(txA3); + assertPending(txB1); + assertDead(txB2); + assertDead(txC1); + assertDead(txC2); + assertDead(txD1); + assertDead(txD2); + // A reorg: previous block "replaced" by new empty block + FakeTxBuilder.BlockPair blockPair6 = createFakeBlock(blockStore, blockPair0.storedBlock, 2); + wallet.reorganize(blockPair0.storedBlock, Lists.newArrayList(blockPair5.storedBlock), + Lists.newArrayList(blockPair6.storedBlock)); + assertDead(txA1); + assertPending(txA2); + assertDead(txA3); + assertPending(txB1); + assertDead(txB2); + assertDead(txC1); + assertDead(txC2); + assertDead(txD1); + assertDead(txD2); } @Test @@ -1167,51 +1157,35 @@ public class WalletTest extends TestWithWallet { @Test public void doubleSpendForBuildingTx() throws Exception { - CoinSelector originalCoinSelector = wallet.getCoinSelector(); - try { - wallet.allowSpendingUnconfirmedTransactions(); + sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(2, 0)); + Transaction send1 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 0), true)); + Transaction send2 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 20), true)); - sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(2, 0)); - Transaction send1 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 0))); - Transaction send2 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 20))); + sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, send1); + assertUnspent(send1); - sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, send1); - assertUnspent(send1); - - wallet.receivePending(send2, null); - assertUnspent(send1); - assertDead(send2); - - } finally { - wallet.setCoinSelector(originalCoinSelector); - } + wallet.receivePending(send2, null); + assertUnspent(send1); + assertDead(send2); } @Test public void txSpendingDeadTx() throws Exception { - CoinSelector originalCoinSelector = wallet.getCoinSelector(); - try { - wallet.allowSpendingUnconfirmedTransactions(); + sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(2, 0)); + Transaction send1 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 0), true)); + Transaction send2 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 20), true)); + wallet.commitTx(send1); + assertPending(send1); + Transaction send1b = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 50), true)); - sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(2, 0)); - Transaction send1 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 0))); - Transaction send2 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 20))); - wallet.commitTx(send1); - assertPending(send1); - Transaction send1b = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 50))); + sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, send2); + assertDead(send1); + assertUnspent(send2); - sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, send2); - assertDead(send1); - assertUnspent(send2); - - wallet.receivePending(send1b, null); - assertDead(send1); - assertUnspent(send2); - assertDead(send1b); - - } finally { - wallet.setCoinSelector(originalCoinSelector); - } + wallet.receivePending(send1b, null); + assertDead(send1); + assertUnspent(send2); + assertDead(send1b); } private void assertInConflict(Transaction tx) { @@ -1241,88 +1215,76 @@ public class WalletTest extends TestWithWallet { @Test public void testAddTransactionsDependingOn() throws Exception { - CoinSelector originalCoinSelector = wallet.getCoinSelector(); - try { - wallet.allowSpendingUnconfirmedTransactions(); - sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(2, 0)); - Transaction send1 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 0))); - Transaction send2 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 20))); - wallet.commitTx(send1); - Transaction send1b = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 50))); - wallet.commitTx(send1b); - Transaction send1c = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 25))); - wallet.commitTx(send1c); - wallet.commitTx(send2); - Set txns = new HashSet<>(); - txns.add(send1); - wallet.addTransactionsDependingOn(txns, wallet.getTransactions(true)); - assertEquals(3, txns.size()); - assertTrue(txns.contains(send1)); - assertTrue(txns.contains(send1b)); - assertTrue(txns.contains(send1c)); - } finally { - wallet.setCoinSelector(originalCoinSelector); - } + sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(2, 0)); + Transaction send1 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 0), true)); + Transaction send2 = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 20), true)); + wallet.commitTx(send1); + Transaction send1b = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 50), true)); + wallet.commitTx(send1b); + Transaction send1c = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 25), true)); + wallet.commitTx(send1c); + wallet.commitTx(send2); + Set txns = new HashSet<>(); + txns.add(send1); + wallet.addTransactionsDependingOn(txns, wallet.getTransactions(true)); + assertEquals(3, txns.size()); + assertTrue(txns.contains(send1)); + assertTrue(txns.contains(send1b)); + assertTrue(txns.contains(send1c)); } @Test public void sortTxnsByDependency() throws Exception { - CoinSelector originalCoinSelector = wallet.getCoinSelector(); - try { - wallet.allowSpendingUnconfirmedTransactions(); - Transaction send1 = sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(2, 0)); - Transaction send1a = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 0))); - wallet.commitTx(send1a); - Transaction send1b = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 50))); - wallet.commitTx(send1b); - Transaction send1c = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 25))); - wallet.commitTx(send1c); - Transaction send1d = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 12))); - wallet.commitTx(send1d); - Transaction send1e = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 06))); - wallet.commitTx(send1e); + Transaction send1 = sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(2, 0)); + Transaction send1a = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(1, 0), true)); + wallet.commitTx(send1a); + Transaction send1b = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 50), true)); + wallet.commitTx(send1b); + Transaction send1c = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 25), true)); + wallet.commitTx(send1c); + Transaction send1d = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 12), true)); + wallet.commitTx(send1d); + Transaction send1e = checkNotNull(wallet.createSend(OTHER_ADDRESS, valueOf(0, 06), true)); + wallet.commitTx(send1e); - Transaction send2 = sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(200, 0)); + Transaction send2 = sendMoneyToWallet(AbstractBlockChain.NewBlockType.BEST_CHAIN, valueOf(200, 0)); - SendRequest req2a = SendRequest.to(OTHER_ADDRESS, valueOf(100, 0)); - req2a.tx.addInput(send2.getOutput(0)); - req2a.shuffleOutputs = false; - wallet.completeTx(req2a); - Transaction send2a = req2a.tx; + SendRequest req2a = SendRequest.to(OTHER_ADDRESS, valueOf(100, 0)); + req2a.tx.addInput(send2.getOutput(0)); + req2a.shuffleOutputs = false; + wallet.completeTx(req2a); + Transaction send2a = req2a.tx; - SendRequest req2b = SendRequest.to(OTHER_ADDRESS, valueOf(50, 0)); - req2b.tx.addInput(send2a.getOutput(1)); - req2b.shuffleOutputs = false; - wallet.completeTx(req2b); - Transaction send2b = req2b.tx; + SendRequest req2b = SendRequest.to(OTHER_ADDRESS, valueOf(50, 0)); + req2b.tx.addInput(send2a.getOutput(1)); + req2b.shuffleOutputs = false; + wallet.completeTx(req2b); + Transaction send2b = req2b.tx; - SendRequest req2c = SendRequest.to(OTHER_ADDRESS, valueOf(25, 0)); - req2c.tx.addInput(send2b.getOutput(1)); - req2c.shuffleOutputs = false; - wallet.completeTx(req2c); - Transaction send2c = req2c.tx; + SendRequest req2c = SendRequest.to(OTHER_ADDRESS, valueOf(25, 0)); + req2c.tx.addInput(send2b.getOutput(1)); + req2c.shuffleOutputs = false; + wallet.completeTx(req2c); + Transaction send2c = req2c.tx; - Set unsortedTxns = new HashSet<>(); - unsortedTxns.add(send1a); - unsortedTxns.add(send1b); - unsortedTxns.add(send1c); - unsortedTxns.add(send1d); - unsortedTxns.add(send1e); - unsortedTxns.add(send2a); - unsortedTxns.add(send2b); - unsortedTxns.add(send2c); - List sortedTxns = wallet.sortTxnsByDependency(unsortedTxns); + Set unsortedTxns = new HashSet<>(); + unsortedTxns.add(send1a); + unsortedTxns.add(send1b); + unsortedTxns.add(send1c); + unsortedTxns.add(send1d); + unsortedTxns.add(send1e); + unsortedTxns.add(send2a); + unsortedTxns.add(send2b); + unsortedTxns.add(send2c); + List sortedTxns = wallet.sortTxnsByDependency(unsortedTxns); - assertEquals(8, sortedTxns.size()); - assertTrue(sortedTxns.indexOf(send1a) < sortedTxns.indexOf(send1b)); - assertTrue(sortedTxns.indexOf(send1b) < sortedTxns.indexOf(send1c)); - assertTrue(sortedTxns.indexOf(send1c) < sortedTxns.indexOf(send1d)); - assertTrue(sortedTxns.indexOf(send1d) < sortedTxns.indexOf(send1e)); - assertTrue(sortedTxns.indexOf(send2a) < sortedTxns.indexOf(send2b)); - assertTrue(sortedTxns.indexOf(send2b) < sortedTxns.indexOf(send2c)); - } finally { - wallet.setCoinSelector(originalCoinSelector); - } + assertEquals(8, sortedTxns.size()); + assertTrue(sortedTxns.indexOf(send1a) < sortedTxns.indexOf(send1b)); + assertTrue(sortedTxns.indexOf(send1b) < sortedTxns.indexOf(send1c)); + assertTrue(sortedTxns.indexOf(send1c) < sortedTxns.indexOf(send1d)); + assertTrue(sortedTxns.indexOf(send1d) < sortedTxns.indexOf(send1e)); + assertTrue(sortedTxns.indexOf(send2a) < sortedTxns.indexOf(send2b)); + assertTrue(sortedTxns.indexOf(send2b) < sortedTxns.indexOf(send2c)); } @Test diff --git a/tools/src/main/java/org/bitcoinj/tools/WalletTool.java b/tools/src/main/java/org/bitcoinj/tools/WalletTool.java index e621bfcb8..d61329e9a 100644 --- a/tools/src/main/java/org/bitcoinj/tools/WalletTool.java +++ b/tools/src/main/java/org/bitcoinj/tools/WalletTool.java @@ -602,7 +602,7 @@ public class WalletTool { if (feePerVkb != null) req.setFeePerVkb(feePerVkb); if (allowUnconfirmed) { - wallet.allowSpendingUnconfirmedTransactions(); + req.allowUnconfirmed(); } if (password != null) { req.aesKey = passwordToKey(true); diff --git a/wallettemplate/src/main/java/wallettemplate/Main.java b/wallettemplate/src/main/java/wallettemplate/Main.java index 3a2b0a062..5783970e1 100644 --- a/wallettemplate/src/main/java/wallettemplate/Main.java +++ b/wallettemplate/src/main/java/wallettemplate/Main.java @@ -139,9 +139,6 @@ public class Main extends Application { bitcoin = new WalletAppKit(params, PREFERRED_OUTPUT_SCRIPT_TYPE, null, appDataDirectory, WALLET_FILE_NAME) { @Override protected void onSetupCompleted() { - // Don't make the user wait for confirmations for now, as the intention is they're sending it - // their own money! - bitcoin.wallet().allowSpendingUnconfirmedTransactions(); Platform.runLater(controller::onBitcoinSetup); } }; diff --git a/wallettemplate/src/main/java/wallettemplate/SendMoneyController.java b/wallettemplate/src/main/java/wallettemplate/SendMoneyController.java index d995e8549..8f0679b8f 100644 --- a/wallettemplate/src/main/java/wallettemplate/SendMoneyController.java +++ b/wallettemplate/src/main/java/wallettemplate/SendMoneyController.java @@ -78,6 +78,9 @@ public class SendMoneyController { else req = SendRequest.to(destination, amount); req.aesKey = aesKey; + // Don't make the user wait for confirmations for now, as the intention is they're sending it + // their own money! + req.allowUnconfirmed(); sendResult = Main.bitcoin.wallet().sendCoins(req); Futures.addCallback(sendResult.broadcastComplete, new FutureCallback() { @Override