diff --git a/core/src/main/java/org/bitcoinj/core/Transaction.java b/core/src/main/java/org/bitcoinj/core/Transaction.java index bcc98b38d67..4d34e64f588 100644 --- a/core/src/main/java/org/bitcoinj/core/Transaction.java +++ b/core/src/main/java/org/bitcoinj/core/Transaction.java @@ -1011,6 +1011,20 @@ public TransactionOutput addOutput(TransactionOutput to) { return to; } + /** + * Replaces an already added output. + * + * @param index index of output to replace + * @param output output to replace with + */ + public void replaceOutput(int index, TransactionOutput output) { + TransactionOutput oldOutput = outputs.remove(index); + checkState(oldOutput.isAvailableForSpending()); + oldOutput.setParent(null); + output.setParent(this); + outputs.add(index, output); + } + /** * Creates an output based on the given address and value, adds it to this transaction, and returns the new output. */ diff --git a/core/src/main/java/org/bitcoinj/core/TransactionOutput.java b/core/src/main/java/org/bitcoinj/core/TransactionOutput.java index 772b6619a83..2c85e9562de 100644 --- a/core/src/main/java/org/bitcoinj/core/TransactionOutput.java +++ b/core/src/main/java/org/bitcoinj/core/TransactionOutput.java @@ -57,7 +57,7 @@ public class TransactionOutput { @Nullable protected Transaction parent; // The output's value is kept as a native type in order to save class instances. - private long value; + private final long value; // A transaction output has a script used for authenticating that the redeemer is allowed to spend // this output. @@ -169,14 +169,14 @@ public Coin getValue() { } /** - * Sets the value of this output. + * Returns a clone of this output, with given value. The typical use case is fee calculation. + * + * @param value value for the clone + * @return clone of output, with given value */ - public void setValue(Coin value) { + public TransactionOutput withValue(Coin value) { Objects.requireNonNull(value); - // Negative values obviously make no sense, except for -1 which is used as a sentinel value when calculating - // SIGHASH_SINGLE signatures, so unfortunately we have to allow that here. - checkArgument(value.signum() >= 0 || value.equals(Coin.NEGATIVE_SATOSHI), () -> "value out of range: " + value); - this.value = value.value; + return new TransactionOutput(this.parent, value, this.scriptBytes); } /** diff --git a/core/src/main/java/org/bitcoinj/wallet/Wallet.java b/core/src/main/java/org/bitcoinj/wallet/Wallet.java index 2aa7d510456..cb9e4f6a16c 100644 --- a/core/src/main/java/org/bitcoinj/wallet/Wallet.java +++ b/core/src/main/java/org/bitcoinj/wallet/Wallet.java @@ -4439,7 +4439,7 @@ public void completeTx(SendRequest req) throws InsufficientMoneyException, Trans CoinSelector selector = req.coinSelector == null ? coinSelector : req.coinSelector; bestCoinSelection = selector.select((Coin) network.maxMoney(), candidates); candidates = null; // Selector took ownership and might have changed candidates. Don't access again. - req.tx.getOutput(0).setValue(bestCoinSelection.totalValue()); + req.tx.replaceOutput(0, req.tx.getOutput(0).withValue(bestCoinSelection.totalValue())); log.info(" emptying {}", bestCoinSelection.totalValue().toFriendlyString()); } @@ -4453,7 +4453,7 @@ public void completeTx(SendRequest req) throws InsufficientMoneyException, Trans if (updatedOutputValues != null) { for (int i = 0; i < updatedOutputValues.size(); i++) { - req.tx.getOutput(i).setValue(updatedOutputValues.get(i)); + req.tx.replaceOutput(i, req.tx.getOutput(i).withValue(updatedOutputValues.get(i))); } } @@ -4591,7 +4591,8 @@ private boolean adjustOutputDownwardsForFee(Transaction tx, CoinSelection coinSe boolean ensureMinRequiredFee) { Coin fee = estimateFees(tx, coinSelection, feePerKb, ensureMinRequiredFee); TransactionOutput output = tx.getOutput(0); - output.setValue(output.getValue().subtract(fee)); + output = output.withValue(output.getValue().subtract(fee)); + tx.replaceOutput(0, output); return !output.isDust(); } @@ -5269,11 +5270,12 @@ private FeeCalculation calculateFee(SendRequest req, Coin value, boolean needAtL ByteBuffer.wrap(req.tx.getOutput(i).serialize()), tx); if (req.recipientsPayFees) { // Subtract fee equally from each selected recipient - output.setValue(output.getValue().subtract(fee.divide(req.tx.getOutputs().size()))); + output = output.withValue(output.getValue().subtract(fee.divide(req.tx.getOutputs().size()))); // first receiver pays the remainder not divisible by output count if (i == 0) { - output.setValue( - output.getValue().subtract(fee.divideAndRemainder(req.tx.getOutputs().size())[1])); // Subtract fee equally from each selected recipient + // Subtract fee equally from each selected recipient + Coin feeRemainder = fee.divideAndRemainder(req.tx.getOutputs().size())[1]; + output = output.withValue(output.getValue().subtract(feeRemainder)); } result.updatedOutputValues.add(output.getValue()); Coin nonDustValue = output.getMinNonDustValue(); @@ -5304,9 +5306,10 @@ private FeeCalculation calculateFee(SendRequest req, Coin value, boolean needAtL // This would be against the purpose of the all-inclusive feature. // So instead we raise the change and deduct from the first recipient. Coin missingToNotBeDust = changeOutput.getMinNonDustValue().subtract(changeOutput.getValue()); - changeOutput.setValue(changeOutput.getValue().add(missingToNotBeDust)); + changeOutput = changeOutput.withValue(changeOutput.getValue().add(missingToNotBeDust)); TransactionOutput firstOutput = tx.getOutput(0); - firstOutput.setValue(firstOutput.getValue().subtract(missingToNotBeDust)); + firstOutput = firstOutput.withValue(firstOutput.getValue().subtract(missingToNotBeDust)); + tx.replaceOutput(0, firstOutput); result.updatedOutputValues.set(0, firstOutput.getValue()); if (firstOutput.isDust()) { throw new CouldNotAdjustDownwards(); diff --git a/core/src/test/java/org/bitcoinj/core/TransactionTest.java b/core/src/test/java/org/bitcoinj/core/TransactionTest.java index a25eba41423..cfa81ff9802 100644 --- a/core/src/test/java/org/bitcoinj/core/TransactionTest.java +++ b/core/src/test/java/org/bitcoinj/core/TransactionTest.java @@ -116,7 +116,7 @@ public void duplicateOutPoint() { @Test(expected = VerificationException.NegativeValueOutput.class) public void negativeOutput() { Transaction tx = FakeTxBuilder.createFakeTx(TESTNET.network()); - tx.getOutput(0).setValue(Coin.NEGATIVE_SATOSHI); + tx.replaceOutput(0, tx.getOutput(0).withValue(Coin.NEGATIVE_SATOSHI)); Transaction.verify(TESTNET.network(), tx); } @@ -124,7 +124,7 @@ public void negativeOutput() { public void exceedsMaxMoney2() { Transaction tx = FakeTxBuilder.createFakeTx(TESTNET.network()); Coin half = BitcoinNetwork.MAX_MONEY.divide(2).add(Coin.SATOSHI); - tx.getOutput(0).setValue(half); + tx.replaceOutput(0, tx.getOutput(0).withValue(half)); tx.addOutput(half, ADDRESS); Transaction.verify(TESTNET.network(), tx); }