TransactionInput: make field sequence immutable

Because tweaking is necessary in cases like unit tests, these usages
have been changed to produce new inputs instead and replace them in
transactions as needed.

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

@@ -991,6 +991,19 @@ public TransactionInput addSignedInput(TransactionOutput output, ECKey sigKey, S
         return addSignedInput(output.getOutPointFor(), output.getScriptPubKey(), output.getValue(), sigKey, sigHash, anyoneCanPay);
     }
 
+    /**
+     * Replaces an already added input. This is meant to amend a transaction before it's committed to a wallet.
+     *
+     * @param index index of input to replace
+     * @param input input to replace with
+     */
+    public void replaceInput(int index, TransactionInput input) {
+        TransactionInput oldInput = inputs.remove(index);
+        oldInput.setParent(null);
+        input.setParent(this);
+        inputs.add(index, input);
+    }
+
     /**
      * Removes all the outputs from this transaction.
      * Note that this also invalidates the length attribute
@@ -1202,7 +1215,7 @@ public Sha256Hash hashForSignature(int inputIndex, byte[] connectedScript, byte
                 // The signature isn't broken by new versions of the transaction issued by other parties.
                 for (int i = 0; i < tx.inputs.size(); i++)
                     if (i != inputIndex)
-                        tx.inputs.get(i).setSequenceNumber(0);
+                        tx.replaceInput(i, tx.getInput(i).withSequence(0));
             } else if ((sigHashType & 0x1f) == SigHash.SINGLE.value) {
                 // SIGHASH_SINGLE means only sign the output at the same index as the input (ie, my output).
                 if (inputIndex >= tx.outputs.size()) {
@@ -1224,7 +1237,7 @@ public Sha256Hash hashForSignature(int inputIndex, byte[] connectedScript, byte
                 // The signature isn't broken by new versions of the transaction issued by other parties.
                 for (int i = 0; i < tx.inputs.size(); i++)
                     if (i != inputIndex)
-                        tx.inputs.get(i).setSequenceNumber(0);
+                        tx.replaceInput(i, tx.getInput(i).withSequence(0));
             }
 
             if ((sigHashType & SigHash.ANYONECANPAY.value) == SigHash.ANYONECANPAY.value) {

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

@@ -73,7 +73,7 @@ public class TransactionInput {
     @Nullable private Transaction parent;
 
     // Allows for altering transactions after they were broadcast. Values below NO_SEQUENCE-1 mean it can be altered.
-    private long sequence;
+    private final long sequence;
     // Data needed to connect to the output of the transaction we're gathering coins from.
     private TransactionOutPoint outpoint;
     // The "script bytes" might not actually be a script. In coinbase transactions where new coins are minted there
@@ -115,40 +115,55 @@ public static TransactionInput read(ByteBuffer payload, Transaction parentTransa
         TransactionOutPoint outpoint = TransactionOutPoint.read(payload);
         byte[] scriptBytes = Buffers.readLengthPrefixedBytes(payload);
         long sequence = ByteUtils.readUint32(payload);
-        return new TransactionInput(parentTransaction, scriptBytes, outpoint, sequence, null);
+        return new TransactionInput(parentTransaction, scriptBytes, outpoint, sequence);
     }
 
     public TransactionInput(@Nullable Transaction parentTransaction, byte[] scriptBytes,
                             TransactionOutPoint outpoint) {
-        this(parentTransaction, scriptBytes, outpoint, NO_SEQUENCE, null);
+        this(parentTransaction, null, scriptBytes, outpoint, NO_SEQUENCE, null, null);
+    }
+
+    public TransactionInput(@Nullable Transaction parentTransaction, byte[] scriptBytes,
+                            TransactionOutPoint outpoint, long sequence) {
+        this(parentTransaction, null, scriptBytes, outpoint, sequence, null, null);
     }
 
     public TransactionInput(@Nullable Transaction parentTransaction, byte[] scriptBytes,
                             TransactionOutPoint outpoint, @Nullable Coin value) {
-        this(parentTransaction, scriptBytes, outpoint, NO_SEQUENCE, value);
+        this(parentTransaction, null, scriptBytes, outpoint, NO_SEQUENCE, value, null);
+    }
+
+    /** internal use only */
+    public TransactionInput(Transaction parentTransaction, byte[] scriptBytes, TransactionOutPoint outpoint,
+                            long sequence, @Nullable Coin value) {
+        this(Objects.requireNonNull(parentTransaction), null, scriptBytes, outpoint, sequence, value, null);
     }
 
-    private TransactionInput(@Nullable Transaction parentTransaction, byte[] scriptBytes,
-                            TransactionOutPoint outpoint, long sequence, @Nullable Coin value) {
+    private TransactionInput(@Nullable Transaction parentTransaction, @Nullable Script scriptSig, byte[] scriptBytes,
+                            TransactionOutPoint outpoint, long sequence, @Nullable Coin value,
+                            @Nullable TransactionWitness witness) {
         checkArgument(value == null || value.signum() >= 0, () -> "value out of range: " + value);
         parent = parentTransaction;
-        this.scriptBytes = scriptBytes;
-        this.outpoint = outpoint;
+        this.scriptSig = scriptSig != null ? new WeakReference<>(scriptSig) : null;
+        this.scriptBytes = Objects.requireNonNull(scriptBytes);
+        this.outpoint = Objects.requireNonNull(outpoint);
         this.sequence = sequence;
         this.value = value;
+        this.witness = witness;
     }
 
     /**
      * Creates an UNSIGNED input that links to the given output
      */
     TransactionInput(Transaction parentTransaction, TransactionOutput output) {
         this(parentTransaction,
-                EMPTY_ARRAY,
+                null, EMPTY_ARRAY,
                 output.getParentTransaction() != null ?
                         new TransactionOutPoint(output.getIndex(), output.getParentTransaction()) :
                         new TransactionOutPoint(output),
                 NO_SEQUENCE,
-                output.getValue());
+                output.getValue(),
+                null);
     }
 
     /**
@@ -239,15 +254,22 @@ public long getSequenceNumber() {
     }
 
     /**
+     * Returns a clone of this input, with a given sequence number.
+     * <p>
      * Sequence numbers allow participants in a multi-party transaction signing protocol to create new versions of the
      * transaction independently of each other. Newer versions of a transaction can replace an existing version that's
      * in nodes memory pools if the existing version is time locked. See the Contracts page on the Bitcoin wiki for
      * examples of how you can use this feature to build contract protocols.
+     *
+     * @param sequence sequence number for the clone
+     * @return clone of input, with given sequence number
      */
-    public void setSequenceNumber(long sequence) {
+    public TransactionInput withSequence(long sequence) {
         checkArgument(sequence >= 0 && sequence <= ByteUtils.MAX_UNSIGNED_INTEGER, () ->
                 "sequence out of range: " + sequence);
-        this.sequence = sequence;
+        Script scriptSig = this.scriptSig != null ? this.scriptSig.get() : null;
+        return new TransactionInput(this.parent, scriptSig, this.scriptBytes, this.outpoint, sequence, this.value,
+                this.witness);
     }
 
     /**

core/src/main/java/org/bitcoinj/wallet/WalletProtobufSerializer.java

@@ -655,9 +655,8 @@ private void readTransaction(Protos.Transaction txProto) throws UnreadableWallet
                     byteStringToHash(inputProto.getTransactionOutPointHash())
             );
             Coin value = inputProto.hasValue() ? Coin.valueOf(inputProto.getValue()) : null;
-            TransactionInput input = new TransactionInput(tx, scriptBytes, outpoint, value);
-            if (inputProto.hasSequence())
-                input.setSequenceNumber(0xffffffffL & inputProto.getSequence());
+            long sequence = inputProto.hasSequence() ? 0xffffffffL & inputProto.getSequence() : TransactionInput.NO_SEQUENCE;
+            TransactionInput input = new TransactionInput(tx, scriptBytes, outpoint, sequence, value);
             if (inputProto.hasWitness()) {
                 Protos.ScriptWitness witnessProto = inputProto.getWitness();
                 if (witnessProto.getDataCount() > 0) {

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

@@ -114,7 +114,7 @@ public void testCachedParsing() throws Exception {
         transaction = (Transaction) serializer.deserialize(ByteBuffer.wrap(TRANSACTION_MESSAGE_BYTES));
         assertNotNull(transaction);
 
-        transaction.getInput(0).setSequenceNumber(1);
+        transaction.replaceInput(0, transaction.getInput(0).withSequence(1));
 
         bos = new ByteArrayOutputStream();
         serializer.serialize(transaction, bos);
@@ -131,7 +131,8 @@ public void testCachedParsing() throws Exception {
         transaction = (Transaction) serializer.deserialize(ByteBuffer.wrap(TRANSACTION_MESSAGE_BYTES));
         assertNotNull(transaction);
 
-        transaction.getInput(0).setSequenceNumber(transaction.getInputs().get(0).getSequenceNumber());
+        transaction.replaceInput(0,
+                transaction.getInput(0).withSequence(transaction.getInput(0).getSequenceNumber())); // no-op?
 
         bos = new ByteArrayOutputStream();
         serializer.serialize(transaction, bos);

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

@@ -1193,7 +1193,7 @@ public boolean add(Rule element) {
         NewBlock b63 = createNextBlock(b60, chainHeadHeight + 19, null, null);
         {
             b63.block.getTransactions().get(0).setLockTime(0xffffffffL);
-            b63.block.getTransactions().get(0).getInput(0).setSequenceNumber(0xdeadbeefL);
+            b63.block.getTransactions().get(0).replaceInput(0, b63.block.getTransactions().get(0).getInput(0).withSequence(0xdeadbeefL));
             checkState(!b63.block.getTransactions().get(0).isFinal(chainHeadHeight + 17, b63.block.time()));
         }
         b63.solve();
@@ -1797,7 +1797,7 @@ private void addOnlyInputToTransaction(Transaction t, TransactionOutPointWithVal
 
     private void addOnlyInputToTransaction(Transaction t, TransactionOutPointWithValue prevOut, long sequence) throws ScriptException {
         TransactionInput input = new TransactionInput(t, new byte[]{}, prevOut.outpoint);
-        input.setSequenceNumber(sequence);
+        input = input.withSequence(sequence);
         t.addInput(input);
 
         if (prevOut.scriptPubKey.chunks().get(0).equalsOpCode(OP_TRUE)) {

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

@@ -481,8 +481,7 @@ public void testToString() {
     @Test
     public void testToStringWhenLockTimeIsSpecifiedInBlockHeight() {
         Transaction tx = FakeTxBuilder.createFakeTx(TESTNET.network());
-        TransactionInput input = tx.getInput(0);
-        input.setSequenceNumber(42);
+        tx.replaceInput(0, tx.getInput(0).withSequence(42));
 
         int TEST_LOCK_TIME = 20;
         tx.setLockTime(TEST_LOCK_TIME);
@@ -608,7 +607,7 @@ public void optInFullRBF() {
         Transaction tx = FakeTxBuilder.createFakeTx(TESTNET.network());
         assertFalse(tx.isOptInFullRBF());
 
-        tx.getInput(0).setSequenceNumber(TransactionInput.NO_SEQUENCE - 2);
+        tx.replaceInput(0, tx.getInput(0).withSequence(TransactionInput.NO_SEQUENCE - 2));
         assertTrue(tx.isOptInFullRBF());
     }
 

core/src/test/java/org/bitcoinj/script/ScriptTest.java

@@ -355,7 +355,7 @@ private Transaction buildCreditingTransaction(Script scriptPubKey) {
 
         TransactionInput txInput = new TransactionInput(null,
                 new ScriptBuilder().number(0).number(0).build().program(), TransactionOutPoint.UNCONNECTED);
-        txInput.setSequenceNumber(TransactionInput.NO_SEQUENCE);
+        txInput = txInput.withSequence(TransactionInput.NO_SEQUENCE);
         tx.addInput(txInput);
 
         TransactionOutput txOutput = new TransactionOutput(tx, Coin.ZERO, scriptPubKey.program());
@@ -371,7 +371,7 @@ private Transaction buildSpendingTransaction(Transaction creditingTransaction, S
 
         TransactionInput txInput = new TransactionInput(creditingTransaction, scriptSig.program(),
                 TransactionOutPoint.UNCONNECTED);
-        txInput.setSequenceNumber(TransactionInput.NO_SEQUENCE);
+        txInput = txInput.withSequence(TransactionInput.NO_SEQUENCE);
         tx.addInput(txInput);
 
         TransactionOutput txOutput = new TransactionOutput(tx, creditingTransaction.getOutput(0).getValue(),

core/src/test/java/org/bitcoinj/store/WalletProtobufSerializerTest.java

@@ -246,10 +246,10 @@ public void testLastBlockSeenHash() throws Exception {
     public void testSequenceNumber() throws Exception {
         Wallet wallet = Wallet.createDeterministic(BitcoinNetwork.TESTNET, ScriptType.P2PKH);
         Transaction tx1 = createFakeTx(TESTNET.network(), Coin.COIN, wallet.currentReceiveAddress());
-        tx1.getInput(0).setSequenceNumber(TransactionInput.NO_SEQUENCE);
+        tx1.replaceInput(0, tx1.getInput(0).withSequence(TransactionInput.NO_SEQUENCE));
         wallet.receivePending(tx1, null);
         Transaction tx2 = createFakeTx(TESTNET.network(), Coin.COIN, wallet.currentReceiveAddress());
-        tx2.getInput(0).setSequenceNumber(TransactionInput.NO_SEQUENCE - 1);
+        tx2.replaceInput(0, tx2.getInput(0).withSequence(TransactionInput.NO_SEQUENCE - 1));
         wallet.receivePending(tx2, null);
         Wallet walletCopy = roundTrip(wallet);
         Transaction tx1copy = Objects.requireNonNull(walletCopy.getTransaction(tx1.getTxId()));

core/src/test/java/org/bitcoinj/wallet/DefaultRiskAnalysisTest.java

@@ -90,7 +90,7 @@ public void nonFinal() {
         assertNull(analysis.getNonFinal());
 
         // Set a sequence number on the input to make it genuinely non-final. Verify it's risky.
-        input.setSequenceNumber(TransactionInput.NO_SEQUENCE - 1);
+        tx.replaceInput(0, input.withSequence(TransactionInput.NO_SEQUENCE - 1));
         analysis = DefaultRiskAnalysis.FACTORY.create(wallet, tx, NO_DEPS);
         assertEquals(RiskAnalysis.Result.NON_FINAL, analysis.analyze());
         assertEquals(tx, analysis.getNonFinal());
@@ -104,7 +104,9 @@ public void nonFinal() {
     @Test
     public void selfCreatedAreNotRisky() {
         Transaction tx = new Transaction();
-        tx.addInput(MAINNET.getGenesisBlock().getTransactions().get(0).getOutput(0)).setSequenceNumber(1);
+        TransactionInput input =
+                tx.addInput(MAINNET.getGenesisBlock().getTransactions().get(0).getOutput(0)).withSequence(1);
+        tx.replaceInput(0, input);
         tx.addOutput(COIN, key1);
         tx.setLockTime(TIMESTAMP + 86400);
 
@@ -125,7 +127,9 @@ public void selfCreatedAreNotRisky() {
     public void nonFinalDependency() {
         // Final tx has a dependency that is non-final.
         Transaction tx1 = new Transaction();
-        tx1.addInput(MAINNET.getGenesisBlock().getTransactions().get(0).getOutput(0)).setSequenceNumber(1);
+        TransactionInput input1 =
+                tx1.addInput(MAINNET.getGenesisBlock().getTransactions().get(0).getOutput(0)).withSequence(1);
+        tx1.replaceInput(0, input1);
         TransactionOutput output = tx1.addOutput(COIN, key1);
         tx1.setLockTime(TIMESTAMP + 86400);
         Transaction tx2 = new Transaction();
@@ -239,7 +243,7 @@ public void standardOutputs() {
     @Test
     public void optInFullRBF() {
         Transaction tx = FakeTxBuilder.createFakeTx(MAINNET.network());
-        tx.getInput(0).setSequenceNumber(TransactionInput.NO_SEQUENCE - 2);
+        tx.replaceInput(0, tx.getInput(0).withSequence(TransactionInput.NO_SEQUENCE - 2));
         DefaultRiskAnalysis analysis = DefaultRiskAnalysis.FACTORY.create(wallet, tx, NO_DEPS);
         assertEquals(RiskAnalysis.Result.NON_FINAL, analysis.analyze());
         assertEquals(tx, analysis.getNonFinal());
@@ -251,11 +255,11 @@ public void relativeLockTime() {
         tx.setVersion(2);
         checkState(!tx.hasRelativeLockTime());
 
-        tx.getInput(0).setSequenceNumber(TransactionInput.NO_SEQUENCE);
+        tx.replaceInput(0, tx.getInput(0).withSequence(TransactionInput.NO_SEQUENCE));
         DefaultRiskAnalysis analysis = DefaultRiskAnalysis.FACTORY.create(wallet, tx, NO_DEPS);
         assertEquals(RiskAnalysis.Result.OK, analysis.analyze());
 
-        tx.getInput(0).setSequenceNumber(0);
+        tx.replaceInput(0, tx.getInput(0).withSequence(0));
         analysis = DefaultRiskAnalysis.FACTORY.create(wallet, tx, NO_DEPS);
         assertEquals(RiskAnalysis.Result.NON_FINAL, analysis.analyze());
         assertEquals(tx, analysis.getNonFinal());

integration-test/src/test/java/org/bitcoinj/core/PeerTest.java

@@ -732,8 +732,7 @@ private void checkTimeLockedDependency(boolean shouldAccept) throws Exception {
         t2.setLockTime(999999);
         // Add a fake input to t3 that goes nowhere.
         Sha256Hash t3 = Sha256Hash.of("abc".getBytes(StandardCharsets.UTF_8));
-        t2.addInput(new TransactionInput(t2, new byte[]{}, new TransactionOutPoint(0, t3)));
-        t2.getInput(0).setSequenceNumber(0xDEADBEEF);
+        t2.addInput(new TransactionInput(t2, new byte[] {}, new TransactionOutPoint(0, t3), 0xDEADBEEF));
         t2.addOutput(COIN, new ECKey());
         Transaction t1 = new Transaction();
         t1.addInput(t2.getOutput(0));

wallettool/src/main/java/org/bitcoinj/wallettool/WalletTool.java

@@ -664,7 +664,7 @@ private void send(CoinSelector coinSelector, List<String> outputs, Coin feePerVk
             if (lockTimeStr != null) {
                 tx.setLockTime(parseLockTimeStr(lockTimeStr));
                 // For lock times to take effect, at least one output must have a non-final sequence number.
-                tx.getInput(0).setSequenceNumber(0);
+                tx.replaceInput(0, tx.getInput(0).withSequence(0));
                 // And because we modified the transaction after it was completed, we must re-sign the inputs.
                 wallet.signTransaction(req);
             }