diff --git a/core/src/main/java/org/bitcoinj/core/Transaction.java b/core/src/main/java/org/bitcoinj/core/Transaction.java index 6b71e64fb14..890b69c21ce 100644 --- a/core/src/main/java/org/bitcoinj/core/Transaction.java +++ b/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) { diff --git a/core/src/main/java/org/bitcoinj/core/TransactionInput.java b/core/src/main/java/org/bitcoinj/core/TransactionInput.java index 3ce34e0ab0d..fcb3768764d 100644 --- a/core/src/main/java/org/bitcoinj/core/TransactionInput.java +++ b/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,27 +115,41 @@ 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; } /** @@ -143,12 +157,13 @@ private TransactionInput(@Nullable Transaction parentTransaction, byte[] scriptB */ 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. + *

* 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); } /** diff --git a/core/src/main/java/org/bitcoinj/wallet/WalletProtobufSerializer.java b/core/src/main/java/org/bitcoinj/wallet/WalletProtobufSerializer.java index 60f7d961546..1807896a242 100644 --- a/core/src/main/java/org/bitcoinj/wallet/WalletProtobufSerializer.java +++ b/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) { diff --git a/core/src/test/java/org/bitcoinj/core/BitcoinSerializerTest.java b/core/src/test/java/org/bitcoinj/core/BitcoinSerializerTest.java index e52621304d5..ed8e99f8413 100644 --- a/core/src/test/java/org/bitcoinj/core/BitcoinSerializerTest.java +++ b/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); diff --git a/core/src/test/java/org/bitcoinj/core/FullBlockTestGenerator.java b/core/src/test/java/org/bitcoinj/core/FullBlockTestGenerator.java index a4a05f094a8..0394ddc8002 100644 --- a/core/src/test/java/org/bitcoinj/core/FullBlockTestGenerator.java +++ b/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)) { diff --git a/core/src/test/java/org/bitcoinj/core/TransactionTest.java b/core/src/test/java/org/bitcoinj/core/TransactionTest.java index a25eba41423..73666d87631 100644 --- a/core/src/test/java/org/bitcoinj/core/TransactionTest.java +++ b/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()); } diff --git a/core/src/test/java/org/bitcoinj/script/ScriptTest.java b/core/src/test/java/org/bitcoinj/script/ScriptTest.java index 28a67fe283f..fd93e21a844 100644 --- a/core/src/test/java/org/bitcoinj/script/ScriptTest.java +++ b/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(), diff --git a/core/src/test/java/org/bitcoinj/store/WalletProtobufSerializerTest.java b/core/src/test/java/org/bitcoinj/store/WalletProtobufSerializerTest.java index ca0fae09cd5..757b0593547 100644 --- a/core/src/test/java/org/bitcoinj/store/WalletProtobufSerializerTest.java +++ b/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())); diff --git a/core/src/test/java/org/bitcoinj/wallet/DefaultRiskAnalysisTest.java b/core/src/test/java/org/bitcoinj/wallet/DefaultRiskAnalysisTest.java index 4b1c3032030..ac2811f4b4f 100644 --- a/core/src/test/java/org/bitcoinj/wallet/DefaultRiskAnalysisTest.java +++ b/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()); diff --git a/integration-test/src/test/java/org/bitcoinj/core/PeerTest.java b/integration-test/src/test/java/org/bitcoinj/core/PeerTest.java index c77bc744c15..1342c52937a 100644 --- a/integration-test/src/test/java/org/bitcoinj/core/PeerTest.java +++ b/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)); diff --git a/wallettool/src/main/java/org/bitcoinj/wallettool/WalletTool.java b/wallettool/src/main/java/org/bitcoinj/wallettool/WalletTool.java index 8cc7c56e5dd..be037e138d8 100644 --- a/wallettool/src/main/java/org/bitcoinj/wallettool/WalletTool.java +++ b/wallettool/src/main/java/org/bitcoinj/wallettool/WalletTool.java @@ -664,7 +664,7 @@ private void send(CoinSelector coinSelector, List 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); }