TransactionOutput: make field value immutable

Because tweaking is necessary in fee calculation logic, these usages
have been changed to produce new TransactionOutputs instead and
replace them in transactions as needed.

core/src/main/java/org/bitcoinj/core/Transaction.java

@@ -1011,6 +1011,15 @@ public TransactionOutput addOutput(TransactionOutput to) {
         return to;
     }
 
+    public TransactionOutput replaceOutput(int index, Coin newValue) {
+        TransactionOutput oldOutput = outputs.remove(index);
+        checkState(oldOutput.isAvailableForSpending());
+        oldOutput.setParent(null);
+        TransactionOutput newOutput = new TransactionOutput(this, newValue, oldOutput.getScriptBytes());
+        outputs.add(index, newOutput);
+        return newOutput;
+    }
+
     /**
      * Creates an output based on the given address and value, adds it to this transaction, and returns the new output.
      */

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.
@@ -168,17 +168,6 @@ public Coin getValue() {
         return Coin.valueOf(value);
     }
 
-    /**
-     * Sets the value of this output.
-     */
-    public void setValue(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;
-    }
-
     /**
      * Gets the index of this output in the parent transaction, or throws if this output is freestanding. Iterates
      * over the parents list to discover this.

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, 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, updatedOutputValues.get(i));
                 }
             }
 
@@ -4591,7 +4591,7 @@ 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 = tx.replaceOutput(0, output.getValue().subtract(fee));
         return !output.isDust();
     }
 
@@ -5269,11 +5269,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 = new TransactionOutput(null, output.getValue().subtract(fee.divide(req.tx.getOutputs().size())), output.getScriptBytes());
                     // 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 = new TransactionOutput(null, output.getValue().subtract(feeRemainder), output.getScriptBytes());
                     }
                     result.updatedOutputValues.add(output.getValue());
                     Coin nonDustValue = output.getMinNonDustValue();
@@ -5304,9 +5305,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 = new TransactionOutput(tx, changeOutput.getValue().add(missingToNotBeDust), changeAddress);
+                    Coin firstOutputValue = tx.getOutput(0).getValue();
+                    tx.replaceOutput(0, firstOutputValue.subtract(missingToNotBeDust));
                     TransactionOutput firstOutput = tx.getOutput(0);
-                    firstOutput.setValue(firstOutput.getValue().subtract(missingToNotBeDust));
                     result.updatedOutputValues.set(0, firstOutput.getValue());
                     if (firstOutput.isDust()) {
                         throw new CouldNotAdjustDownwards();

core/src/test/java/org/bitcoinj/core/TransactionTest.java

@@ -116,15 +116,15 @@ 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, Coin.NEGATIVE_SATOSHI);
         Transaction.verify(TESTNET.network(), tx);
     }
 
     @Test(expected = VerificationException.ExcessiveValue.class)
     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, half);
         tx.addOutput(half, ADDRESS);
         Transaction.verify(TESTNET.network(), tx);
     }