mirrors/bitcoinj
1
0
Fork 0
mirror of https://github.com/bitcoinj/bitcoinj.git synced 2025-01-18 21:32:35 +01:00
Code Issues Packages Projects Releases Wiki Activity

Wallet: properly handle unconnected request inputs in completeTx()

Browse Source 
This adds a test for and fixes issue #2063.

The changes do the following:

1. Build a preliminary list of candidate UTXOs.
2. Use the preliminary list to fix unconnected/valueless inputs before
   calculating `valueNeeded`.
3. Create a new candidate list without the UTXOs that were already present
   in inputs and use it as input for generating the `CoinSelection`.

Test changes:

1. Update existing test to show the improvement (i.e. no wasted fee).
2. Add a new test (based on PR #3055) that shows no duplicate inputs.

This work is based on changes by @michael-swiggs in PR #3055.
This commit is contained in:
Sean Gilligan 2023-05-09 18:23:22 -07:00 committed by Andreas Schildbach
parent 15a9086122
commit dce352f591
2 changed files with 75 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
core/src/main/java/org/bitcoinj/wallet/Wallet.java
View File

@ -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<TransactionOutput> prelimCandidates = calculateAllSpendCandidates(true, req.missingSigsMode == MissingSigsMode.THROW);
// Connect (add a value amount) unconnected inputs
List<TransactionInput> 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<TransactionOutput> candidates = calculateAllSpendCandidates(true, req.missingSigsMode == MissingSigsMode.THROW);
// Filter out candidates that are already included in the transaction inputs
List<TransactionOutput> 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<TransactionInput> connectInputs(List<TransactionOutput> candidates, List<TransactionInput> 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<TransactionInput> inputs, TransactionOutput output) {
return inputs.stream().noneMatch(i -> i.getOutpoint().equals(output.getOutPointFor()));
}
/**
* <p>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.</p>

38
core/src/test/java/org/bitcoinj/wallet/WalletTest.java
View File

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