mirrors/bitcoinj
1
0
Fork 0
mirror of https://github.com/bitcoinj/bitcoinj.git synced 2025-02-22 14:22:45 +01:00
Code Issues Projects Releases Packages Wiki Activity

Wallet.completeTransaction: throw if inputs contain unconnected outputs

Browse source 
Rather than (if not segwit) simply add those unconnected outputs to the miner's fee,
throw an IllegalArgumentException.

In the case of segwit, the transaction can't be properly signed without knowing the
input values.

TODO:
1. Consider adding a checked exception for this new error case
2. Rename some of the variables in the unit test

# Conflicts:
#	core/src/main/java/org/bitcoinj/wallet/Wallet.java
#	core/src/test/java/org/bitcoinj/wallet/WalletTest.java
This commit is contained in:
Sean Gilligan 2023-05-03 17:29:10 -07:00
parent 3ebb1714de
commit be4e954b6f
2 changed files with 16 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
core/src/main/java/org/bitcoinj/wallet/Wallet.java
View file

@ -4362,11 +4362,13 @@ public class Wallet extends BaseTaggableObject
/**
* Given a spend request containing an incomplete transaction, makes it valid by adding outputs and signed inputs
* according to the instructions in the request. The transaction in the request is modified by this method.
* according to the instructions in the request. The transaction in the request is modified by this method. All inputs
* in the incomplete transaction must contain connected unspent outputs.
*
* @param req a SendRequest that contains the incomplete transaction and details for how to make it valid.
* @throws InsufficientMoneyException if the request could not be completed due to not enough balance.
* @throws IllegalArgumentException if you try and complete the same SendRequest twice
* @throws IllegalArgumentException if you provide a transaction with inputs containing unconnected outputs
* or try to complete the same SendRequest twice
* @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.
@ -4393,12 +4395,11 @@ public class Wallet extends BaseTaggableObject
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
// Throw an exception if there are remaining unconnected inputs whose value we do not know
if (req.tx.getInputs().stream()
.map(TransactionInput::getValue)
.anyMatch(Objects::isNull))
log.warn("SendRequest transaction already has inputs but we don't know how much they are worth - they will be added to fee.");
throw new IllegalArgumentException("SendRequest transaction has inputs and we don't know how much they are worth.");
// If any inputs have already been added, we don't need to get their value from wallet
Coin totalInput = req.tx.getInputSum();

20
core/src/test/java/org/bitcoinj/wallet/WalletTest.java
View file

@ -2719,16 +2719,6 @@ 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 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 find the matching UTXO from the wallet and add its value to the unconnected input
request3.shuffleOutputs = false;
wallet.completeTx(request3);
assertEquals(1, request3.tx.getInputs().size());
assertEquals(1, request3.tx.getOutputs().size());
assertEquals(CENT, request3.tx.getOutput(0).getValue());
SendRequest request4 = SendRequest.to(OTHER_ADDRESS, CENT);
request4.tx.addInput(tx3.getOutput(0));
// Now if we manually sign it, completeTx will not replace our signature
@ -2741,6 +2731,16 @@ public class WalletTest extends TestWithWallet {
assertArrayEquals(scriptSig, request4.tx.getInput(0).getScriptBytes());
}
@Test(expected = IllegalArgumentException.class)
public void testCompleteTxWithInputsWithUnconnectedOutput() throws Exception {
// If there is an input with no connected output, we should throw an IllegalArgumentException
SendRequest request3 = SendRequest.to(OTHER_ADDRESS, CENT);
request3.tx.addInput(new TransactionInput(request3.tx, new byte[] {}, new TransactionOutPoint(0, Sha256Hash.ZERO_HASH)));
// Now completeTx will throw rather than add an unconnected output to the fee.
request3.shuffleOutputs = false;
wallet.completeTx(request3);
}
@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)