diff --git a/core/src/main/java/org/bitcoinj/wallet/Wallet.java b/core/src/main/java/org/bitcoinj/wallet/Wallet.java index 04756a168..08826282e 100644 --- a/core/src/main/java/org/bitcoinj/wallet/Wallet.java +++ b/core/src/main/java/org/bitcoinj/wallet/Wallet.java @@ -4540,7 +4540,18 @@ public class Wallet extends BaseTaggableObject log.info("Completing send tx with {} outputs totalling {} and a fee of {}/vkB", req.tx.getOutputs().size(), req.tx.getOutputSum().toFriendlyString(), req.feePerKb.toFriendlyString()); - // Warn if there are unconnected inputs whose value we do not know + // Calculate a list of ALL potential candidates for spending and then ask a coin selector to provide us + // with the actual outputs that'll be used to gather the required amount of value. In this way, users + // can customize coin selection policies. The call below will ignore immature coinbases and outputs + // we don't have the keys for. + List prelimCandidates = calculateAllSpendCandidates(true, req.missingSigsMode == MissingSigsMode.THROW); + + // Connect (add a value amount) unconnected inputs + List inputs = connectInputs(prelimCandidates, req.tx.getInputs()); + req.tx.clearInputs(); + inputs.forEach(req.tx::addInput); + + // Warn if there are remaining unconnected inputs whose value we do not know // TODO: Consider throwing if there are inputs that we don't have a value for if (req.tx.getInputs().stream() .map(TransactionInput::getValue) @@ -4564,11 +4575,10 @@ public class Wallet extends BaseTaggableObject throw new DustySendRequested(); } - // Calculate a list of ALL potential candidates for spending and then ask a coin selector to provide us - // with the actual outputs that'll be used to gather the required amount of value. In this way, users - // can customize coin selection policies. The call below will ignore immature coinbases and outputs - // we don't have the keys for. - List candidates = calculateAllSpendCandidates(true, req.missingSigsMode == MissingSigsMode.THROW); + // Filter out candidates that are already included in the transaction inputs + List candidates = prelimCandidates.stream() + .filter(output -> alreadyIncluded(req.tx.getInputs(), output)) + .collect(StreamUtils.toUnmodifiableList()); CoinSelection bestCoinSelection; TransactionOutput bestChangeOutput = null; @@ -4641,6 +4651,33 @@ public class Wallet extends BaseTaggableObject } } + /** + * Connect unconnected inputs with outputs from the wallet + * @param candidates A list of spend candidates from a Wallet + * @param inputs a list of possibly unconnected/unvalued inputs (e.g. from a spend request) + * @return a list of the same inputs, but connected/valued if not previously valued and found in wallet + */ + @VisibleForTesting + static List connectInputs(List candidates, List inputs) { + return inputs.stream() + .map(in -> candidates.stream() + .filter(utxo -> utxo.getOutPointFor().equals(in.getOutpoint())) + .findFirst() + .map(o -> new TransactionInput(o.getParentTransaction(), o.getScriptPubKey().program(), o.getOutPointFor(), o.getValue())) + .orElse(in)) + .collect(StreamUtils.toUnmodifiableList()); + } + + /** + * Is a UTXO already included (to be spent) in a list of transaction inputs? + * @param inputs the list of inputs to check + * @param output the transaction output + * @return true if it is already included, false otherwise + */ + private boolean alreadyIncluded(List inputs, TransactionOutput output) { + return inputs.stream().noneMatch(i -> i.getOutpoint().equals(output.getOutPointFor())); + } + /** *

Given a send request containing transaction, attempts to sign it's inputs. This method expects transaction * to have all necessary inputs connected or they will be ignored.

diff --git a/core/src/test/java/org/bitcoinj/wallet/WalletTest.java b/core/src/test/java/org/bitcoinj/wallet/WalletTest.java index b2ebe1d2c..980b74c1a 100644 --- a/core/src/test/java/org/bitcoinj/wallet/WalletTest.java +++ b/core/src/test/java/org/bitcoinj/wallet/WalletTest.java @@ -2718,17 +2718,15 @@ public class WalletTest extends TestWithWallet { request2.tx.getInput(0).getScriptSig().correctlySpends( request2.tx, 0, null, null, tx3.getOutput(0).getScriptPubKey(), Script.ALL_VERIFY_FLAGS); - // However, if there is no connected output, we will grab a COIN output anyway and add the CENT to fee + // However, if there is no connected output, we connect it SendRequest request3 = SendRequest.to(OTHER_ADDRESS, CENT); request3.tx.addInput(new TransactionInput(request3.tx, new byte[] {}, new TransactionOutPoint(0, tx3.getTxId()))); - // Now completeTx will result in two inputs, two outputs and a fee of a CENT - // Note that it is simply assumed that the inputs are correctly signed, though in fact the first is not + // Now completeTx will find the matching UTXO from the wallet and add its value to the unconnected input request3.shuffleOutputs = false; wallet.completeTx(request3); - assertEquals(2, request3.tx.getInputs().size()); - assertEquals(2, request3.tx.getOutputs().size()); + assertEquals(1, request3.tx.getInputs().size()); + assertEquals(1, request3.tx.getOutputs().size()); assertEquals(CENT, request3.tx.getOutput(0).getValue()); - assertEquals(COIN.subtract(CENT), request3.tx.getOutput(1).getValue()); SendRequest request4 = SendRequest.to(OTHER_ADDRESS, CENT); request4.tx.addInput(tx3.getOutput(0)); @@ -2742,6 +2740,34 @@ public class WalletTest extends TestWithWallet { assertArrayEquals(scriptSig, request4.tx.getInput(0).getScriptBytes()); } + @Test + public void testCompleteTxWithUnconnectedExistingInput() throws Exception { + // Test calling completeTx with a SendRequest that has an unconnected input (i.e. an input with an unconnected outpoint) + + // Generate an output to us + StoredBlock block = new StoredBlock(makeSolvedTestBlock(blockStore, OTHER_ADDRESS), BigInteger.ONE, 1); + Transaction tx = createFakeTx(TESTNET.network(), COIN, myAddress); + wallet.receiveFromBlock(tx, block, AbstractBlockChain.NewBlockType.BEST_CHAIN, 0); + + // SendRequest using that output as an unconnected input + SendRequest request = SendRequest.to(OTHER_ADDRESS, COIN); + request.tx.addInput(new TransactionInput(request.tx, new byte[] {}, new TransactionOutPoint(0, tx.getTxId()))); + + // Complete the transaction + wallet.completeTx(request); + + // Make sure it has no duplicate inputs + assertEquals(uniqueOutPoints(request.tx.getInputs()), request.tx.getInputs().size()); + } + + // Count unique TransactionOutPoints in a list of TransactionInputs + private long uniqueOutPoints(List inputs) { + return inputs.stream() + .map(TransactionInput::getOutpoint) + .distinct() + .count(); + } + // There is a test for spending a coinbase transaction as it matures in BlockChainTest#coinbaseTransactionAvailability // Support for offline spending is tested in PeerGroupTest