Skip to content

Commit

Permalink
TransactionInput: make field scriptBytes immutable
Browse files Browse the repository at this point in the history
Because tweaking is necessary for transaction signing, these usages
have been changed to produce new inputs instead and replace them in
transactions as needed.
  • Loading branch information
schildbach committed Feb 8, 2025
1 parent ce68909 commit fb40b17
Show file tree
Hide file tree
Showing 12 changed files with 84 additions and 54 deletions.
13 changes: 7 additions & 6 deletions core/src/main/java/org/bitcoinj/core/Transaction.java
Original file line number Diff line number Diff line change
Expand Up @@ -902,20 +902,20 @@ public TransactionInput addSignedInput(TransactionOutPoint prevOut, Script scrip
if (ScriptPattern.isP2PK(scriptPubKey)) {
TransactionSignature signature = calculateSignature(inputIndex, sigKey, scriptPubKey, sigHash,
anyoneCanPay);
input.setScriptSig(ScriptBuilder.createInputScript(signature));
input = input.withScriptSig(ScriptBuilder.createInputScript(signature));
input = input.withoutWitness();
replaceInput(inputIndex, input);
} else if (ScriptPattern.isP2PKH(scriptPubKey)) {
TransactionSignature signature = calculateSignature(inputIndex, sigKey, scriptPubKey, sigHash,
anyoneCanPay);
input.setScriptSig(ScriptBuilder.createInputScript(signature, sigKey));
input = input.withScriptSig(ScriptBuilder.createInputScript(signature, sigKey));
input = input.withoutWitness();
replaceInput(inputIndex, input);
} else if (ScriptPattern.isP2WPKH(scriptPubKey)) {
Script scriptCode = ScriptBuilder.createP2PKHOutputScript(sigKey);
TransactionSignature signature = calculateWitnessSignature(inputIndex, sigKey, scriptCode, input.getValue(),
sigHash, anyoneCanPay);
input.setScriptSig(ScriptBuilder.createEmpty());
input = input.withScriptSig(ScriptBuilder.createEmpty());
input = input.withWitness(TransactionWitness.redeemP2WPKH(signature, sigKey));
replaceInput(inputIndex, input);
} else {
Expand Down Expand Up @@ -1195,7 +1195,7 @@ public Sha256Hash hashForSignature(int inputIndex, byte[] connectedScript, byte
// EC math so we'll do it anyway.
for (int i = 0; i < tx.inputs.size(); i++) {
TransactionInput input = tx.getInput(i);
input.clearScriptBytes();
input = input.withoutScriptBytes();
input = input.withoutWitness();
tx.replaceInput(i, input);
}
Expand All @@ -1212,8 +1212,9 @@ public Sha256Hash hashForSignature(int inputIndex, byte[] connectedScript, byte
// Set the input to the script of its output. Bitcoin Core does this but the step has no obvious purpose as
// the signature covers the hash of the prevout transaction which obviously includes the output script
// already. Perhaps it felt safer to him in some way, or is another leftover from how the code was written.
TransactionInput input = tx.inputs.get(inputIndex);
input.setScriptBytes(connectedScript);
TransactionInput input = tx.getInput(inputIndex);
input = input.withScriptBytes(connectedScript);
tx.replaceInput(inputIndex, input);

if ((sigHashType & 0x1f) == SigHash.NONE.value) {
// SIGHASH_NONE means no outputs are signed at all - the signature is effectively for a "blank cheque".
Expand Down
42 changes: 28 additions & 14 deletions core/src/main/java/org/bitcoinj/core/TransactionInput.java
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public class TransactionInput {
// The "script bytes" might not actually be a script. In coinbase transactions where new coins are minted there
// is no input transaction, so instead the scriptBytes contains some extra stuff (like a rollover nonce) that we
// don't care about much. The bytes are turned into a Script object (cached below) on demand via a getter.
private byte[] scriptBytes;
private final byte[] scriptBytes;
// The Script object obtained from parsing scriptBytes. Only filled in on demand and if the transaction is not
// coinbase.
private WeakReference<Script> scriptSig;
Expand Down Expand Up @@ -226,11 +226,16 @@ public Script getScriptSig() throws ScriptException {
return script;
}

/** Set the given program as the scriptSig that is supposed to satisfy the connected output script. */
public void setScriptSig(Script scriptSig) {
this.scriptSig = new WeakReference<>(Objects.requireNonNull(scriptSig));
// TODO: This should all be cleaned up so we have a consistent internal representation.
setScriptBytes(scriptSig.program());
/**
* Returns a clone of this input, with given scriptSig. The typical use case is transaction signing.
*
* @param scriptSig scriptSig for the clone
* @return clone of input, with given scriptSig
*/
public TransactionInput withScriptSig(Script scriptSig) {
Objects.requireNonNull(scriptSig);
return new TransactionInput(this.parent, scriptSig, scriptSig.program(), this.outpoint, this.sequence,
this.value, this.witness);
}

/**
Expand Down Expand Up @@ -277,20 +282,29 @@ public TransactionOutPoint getOutpoint() {
* @return the scriptBytes
*/
public byte[] getScriptBytes() {
return scriptBytes;
return Arrays.copyOf(scriptBytes, scriptBytes.length);
}

/** Clear input scripts, e.g. in preparation for signing. */
public void clearScriptBytes() {
setScriptBytes(TransactionInput.EMPTY_ARRAY);
/**
* Returns a clone of this input, without script bytes. The typical use case is transaction signing.
*
* @return clone of input, without script bytes
*/
public TransactionInput withoutScriptBytes() {
return new TransactionInput(this.parent, null, TransactionInput.EMPTY_ARRAY, this.outpoint, this.sequence,
this.value, this.witness);
}

/**
* @param scriptBytes the scriptBytes to set
* Returns a clone of this input, with given script bytes. The typical use case is transaction signing.
*
* @param scriptBytes script bytes for the clone
* @return clone of input, with given script bytes
*/
void setScriptBytes(byte[] scriptBytes) {
this.scriptSig = null;
this.scriptBytes = scriptBytes;
public TransactionInput withScriptBytes(byte[] scriptBytes) {
Objects.requireNonNull(scriptBytes);
return new TransactionInput(this.parent, null, scriptBytes, this.outpoint, this.sequence, this.value,
this.witness);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ public boolean signInputs(ProposedTransaction propTx, KeyBag keyBag) {
TransactionSignature txSig = new TransactionSignature(sigKey.sig, Transaction.SigHash.ALL, false);
int sigIndex = inputScript.getSigInsertionIndex(sighash, sigKey.pubKey);
inputScript = scriptPubKey.getScriptSigWithSignature(inputScript, txSig.encodeToBitcoin(), sigIndex);
txIn.setScriptSig(inputScript);
txIn = txIn.withScriptSig(inputScript);
tx.replaceInput(i, txIn);
}
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,14 @@ public boolean signInputs(ProposedTransaction propTx, KeyBag keyBag) {
int sigIndex = 0;
inputScript = scriptPubKey.getScriptSigWithSignature(inputScript, signature.encodeToBitcoin(),
sigIndex);
txIn.setScriptSig(inputScript);
txIn = txIn.withScriptSig(inputScript);
txIn = txIn.withoutWitness();
} else if (ScriptPattern.isP2WPKH(scriptPubKey)) {
Script scriptCode = ScriptBuilder.createP2PKHOutputScript(key);
Coin value = txIn.getValue();
TransactionSignature signature = tx.calculateWitnessSignature(i, key, scriptCode, value,
Transaction.SigHash.ALL, false);
txIn.setScriptSig(ScriptBuilder.createEmpty());
txIn = txIn.withScriptSig(ScriptBuilder.createEmpty());
txIn = txIn.withWitness(TransactionWitness.redeemP2WPKH(signature, key));
} else {
throw new IllegalStateException(script.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public boolean signInputs(ProposedTransaction propTx, KeyBag keyBag) {
if (missingSigsMode == Wallet.MissingSigsMode.THROW) {
throw new MissingSignatureException();
} else if (missingSigsMode == Wallet.MissingSigsMode.USE_DUMMY_SIG) {
txIn.setScriptSig(scriptPubKey.getScriptSigWithSignature(inputScript, dummySig, j - 1));
txIn = txIn.withScriptSig(scriptPubKey.getScriptSigWithSignature(inputScript, dummySig, j - 1));
}
}
}
Expand All @@ -89,7 +89,7 @@ public boolean signInputs(ProposedTransaction propTx, KeyBag keyBag) {
if (missingSigsMode == Wallet.MissingSigsMode.THROW) {
throw new ECKey.MissingPrivateKeyException();
} else if (missingSigsMode == Wallet.MissingSigsMode.USE_DUMMY_SIG) {
txIn.setScriptSig(scriptPubKey.getScriptSigWithSignature(inputScript, dummySig, 0));
txIn = txIn.withScriptSig(scriptPubKey.getScriptSigWithSignature(inputScript, dummySig, 0));
}
}
} else if (ScriptPattern.isP2WPKH(scriptPubKey)) {
Expand Down
12 changes: 9 additions & 3 deletions core/src/main/java/org/bitcoinj/testing/FakeTxBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ public static Transaction createFakeTxWithChangeAddress(Coin value, Address to,
TransactionOutput prevOut = new TransactionOutput(prevTx, value, to);
prevTx.addOutput(prevOut);
// Connect it.
t.addInput(prevOut).setScriptSig(ScriptBuilder.createInputScript(TransactionSignature.dummy()));
TransactionInput in = t.addInput(prevOut);
in = in.withScriptSig(ScriptBuilder.createInputScript(TransactionSignature.dummy()));
t.replaceInput(t.getInputs().size() - 1, in);
// Fake signature.
// Serialize/deserialize to ensure internal state is stripped, as if it had been read from the wire.
return roundTripTransaction(t);
Expand Down Expand Up @@ -126,14 +128,18 @@ public static Transaction createFakeTxWithoutChangeAddress(Coin value, Address t
TransactionOutput prevOut1 = new TransactionOutput(prevTx1, Coin.valueOf(split), to);
prevTx1.addOutput(prevOut1);
// Connect it.
t.addInput(prevOut1).setScriptSig(ScriptBuilder.createInputScript(TransactionSignature.dummy()));
TransactionInput in1 = t.addInput(prevOut1);
in1 = in1.withScriptSig(ScriptBuilder.createInputScript(TransactionSignature.dummy()));
t.replaceInput(t.getInputs().size() - 1, in1);
// Fake signature.

// Do it again
Transaction prevTx2 = new Transaction();
TransactionOutput prevOut2 = new TransactionOutput(prevTx2, Coin.valueOf(value.getValue() - split), to);
prevTx2.addOutput(prevOut2);
t.addInput(prevOut2).setScriptSig(ScriptBuilder.createInputScript(TransactionSignature.dummy()));
TransactionInput in2 = t.addInput(prevOut2);
in2 = in2.withScriptSig(ScriptBuilder.createInputScript(TransactionSignature.dummy()));
t.replaceInput(t.getInputs().size() - 1, in2);

// Serialize/deserialize to ensure internal state is stripped, as if it had been read from the wire.
return roundTripTransaction(t);
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/org/bitcoinj/wallet/Wallet.java
Original file line number Diff line number Diff line change
Expand Up @@ -4564,7 +4564,8 @@ public void signTransaction(SendRequest req) throws BadWalletEncryptionKeyExcept
RedeemData redeemData = txIn.getConnectedRedeemData(maybeDecryptingKeyBag);
Objects.requireNonNull(redeemData, () ->
"Transaction exists in wallet that we cannot redeem: " + txIn.getOutpoint().hash());
txIn.setScriptSig(scriptPubKey.createEmptyInputScript(redeemData.keys.get(0), redeemData.redeemScript));
tx.replaceInput(i, txIn.withScriptSig(scriptPubKey.createEmptyInputScript(redeemData.keys.get(0),
redeemData.redeemScript)));
}

TransactionSigner.ProposedTransaction proposal = new TransactionSigner.ProposedTransaction(tx);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,8 @@ public void skipScripts() throws Exception {
t.addOutput(new TransactionOutput(t, FIFTY_COINS, new byte[] {}));
TransactionInput input = t.addInput(spendableOutput);
// Invalid script.
input.clearScriptBytes();
input = input.withoutScriptBytes();
t.replaceInput(t.getInputs().size() - 1, input);
rollingBlock.addTransaction(t);
rollingBlock.solve();
chain.setRunScripts(false);
Expand Down
23 changes: 13 additions & 10 deletions core/src/test/java/org/bitcoinj/core/FullBlockTestGenerator.java
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ public boolean add(Rule element) {
//
NewBlock b26 = createNextBlock(b15, chainHeadHeight + 7, out6, null);
// 1 is too small, but we already generate every other block with 2, so that is tested
b26.block.getTransactions().get(0).getInput(0).clearScriptBytes();
b26.block.getTransactions().get(0).replaceInput(0, b26.block.getTransactions().get(0).getInput(0).withoutScriptBytes());
b26.block.setMerkleRoot(null);
b26.solve();
blocks.add(new BlockAndValidity(b26, false, true, b23.getHash(), chainHeadHeight + 7, "b26"));
Expand All @@ -496,7 +496,7 @@ public boolean add(Rule element) {
{
byte[] coinbase = new byte[101];
Arrays.fill(coinbase, (byte)0);
b28.block.getTransactions().get(0).getInput(0).setScriptBytes(coinbase);
b28.block.getTransactions().get(0).replaceInput(0, b28.block.getTransactions().get(0).getInput(0).withScriptBytes(coinbase));
}
b28.block.setMerkleRoot(null);
b28.solve();
Expand All @@ -510,7 +510,7 @@ public boolean add(Rule element) {
{
byte[] coinbase = new byte[100];
Arrays.fill(coinbase, (byte)0);
b30.block.getTransactions().get(0).getInput(0).setScriptBytes(coinbase);
b30.block.getTransactions().get(0).replaceInput(0, b30.block.getTransactions().get(0).getInput(0).withScriptBytes(coinbase));
}
b30.block.setMerkleRoot(null);
b30.solve();
Expand Down Expand Up @@ -761,6 +761,7 @@ public boolean add(Rule element) {
TransactionInput input = new TransactionInput(tx, new byte[]{},
new TransactionOutPoint(0, b39.block.getTransactions().get(i).getTxId()));
tx.addInput(input);
int inputIndex = tx.getInputs().size() - 1;

if (scriptSig == null) {
// Exploit the SigHash.SINGLE bug to avoid having to make more than one signature
Expand All @@ -784,7 +785,7 @@ public boolean add(Rule element) {
}
}

input.setScriptBytes(scriptSig);
tx.replaceInput(inputIndex, tx.getInput(inputIndex).withScriptBytes(scriptSig));

lastOutPoint = new TransactionOutPoint(0, tx.getTxId());

Expand Down Expand Up @@ -828,6 +829,7 @@ public boolean add(Rule element) {
TransactionInput input = new TransactionInput(tx, new byte[] {},
new TransactionOutPoint(0, b39.block.getTransactions().get(i).getTxId()));
tx.addInput(input);
int inputIndex = tx.getInputs().size() - 1;

if (scriptSig == null) {
// Exploit the SigHash.SINGLE bug to avoid having to make more than one signature
Expand Down Expand Up @@ -857,7 +859,7 @@ public boolean add(Rule element) {
}
}

input.setScriptBytes(scriptSig);
tx.replaceInput(inputIndex, tx.getInput(inputIndex).withScriptBytes(scriptSig));

lastOutPoint = new TransactionOutPoint(0,
tx.getTxId());
Expand Down Expand Up @@ -1163,7 +1165,7 @@ public boolean add(Rule element) {

NewBlock b61 = createNextBlock(b60, chainHeadHeight + 19, out18, null);
{
b61.block.getTransactions().get(0).getInput(0).setScriptBytes(b60.block.getTransactions().get(0).getInput(0).getScriptBytes());
b61.block.getTransactions().get(0).replaceInput(0, b61.block.getTransactions().get(0).getInput(0).withScriptBytes(b60.block.getTransactions().get(0).getInput(0).getScriptBytes()));
b61.block.unCache();
checkState(b61.block.getTransactions().get(0).equals(b60.block.getTransactions().get(0)));
}
Expand Down Expand Up @@ -1799,16 +1801,17 @@ private void addOnlyInputToTransaction(Transaction t, TransactionOutPointWithVal
TransactionInput input = new TransactionInput(t, new byte[]{}, prevOut.outpoint);
input = input.withSequence(sequence);
t.addInput(input);
int inputIndex = t.getInputs().size() - 1;

if (prevOut.scriptPubKey.chunks().get(0).equalsOpCode(OP_TRUE)) {
input.setScriptSig(new ScriptBuilder().op(OP_1).build());
t.replaceInput(inputIndex, input.withScriptSig(new ScriptBuilder().op(OP_1).build()));
} else {
// Sign input
checkState(ScriptPattern.isP2PK(prevOut.scriptPubKey));
Sha256Hash hash = t.hashForSignature(0, prevOut.scriptPubKey, SigHash.ALL, false);
input.setScriptSig(ScriptBuilder.createInputScript(
new TransactionSignature(coinbaseOutKey.sign(hash), SigHash.ALL, false))
);
t.replaceInput(inputIndex, input.withScriptSig(ScriptBuilder.createInputScript(
new TransactionSignature(coinbaseOutKey.sign(hash), SigHash.ALL, false))
));
}
}

Expand Down
15 changes: 8 additions & 7 deletions core/src/test/java/org/bitcoinj/core/TransactionTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,15 @@ public void emptyInputs() {
@Test(expected = VerificationException.LargerThanMaxBlockSize.class)
public void tooHuge() {
Transaction tx = FakeTxBuilder.createFakeTx(TESTNET.network());
tx.getInput(0).setScriptBytes(new byte[Block.MAX_BLOCK_SIZE]);
tx.replaceInput(0, tx.getInput(0).withScriptBytes(new byte[Block.MAX_BLOCK_SIZE]));
Transaction.verify(TESTNET.network(), tx);
}

@Test(expected = VerificationException.DuplicatedOutPoint.class)
public void duplicateOutPoint() {
Transaction tx = FakeTxBuilder.createFakeTx(TESTNET.network());
TransactionInput input = tx.getInput(0);
input.setScriptBytes(new byte[1]);
input = input.withScriptBytes(new byte[1]);
tx.addInput(input);
Transaction.verify(TESTNET.network(), tx);
}
Expand Down Expand Up @@ -336,7 +336,8 @@ public void testWitnessSignatureP2WPKH() {
ByteUtils.formatHex(txSig1.encodeToBitcoin()));

assertFalse(correctlySpends(txIn0, scriptPubKey0, 0));
txIn0.setScriptSig(new ScriptBuilder().data(txSig0.encodeToBitcoin()).build());
txIn0 = txIn0.withScriptSig(new ScriptBuilder().data(txSig0.encodeToBitcoin()).build());
tx.replaceInput(0, txIn0);
assertTrue(correctlySpends(txIn0, scriptPubKey0, 0));

assertFalse(correctlySpends(txIn1, scriptPubKey1, 1));
Expand Down Expand Up @@ -415,8 +416,8 @@ public void testWitnessSignatureP2SH_P2WPKH() {

assertFalse(correctlySpends(txIn, scriptPubKey, 0));
txIn = txIn.withWitness(TransactionWitness.redeemP2WPKH(txSig, key));
txIn = txIn.withScriptSig(new ScriptBuilder().data(redeemScript.program()).build());
tx.replaceInput(0, txIn);
txIn.setScriptSig(new ScriptBuilder().data(redeemScript.program()).build());
assertTrue(correctlySpends(txIn, scriptPubKey, 0));

String signedTxHex = "01000000" // version
Expand Down Expand Up @@ -571,11 +572,11 @@ public void testPrioSizeCalc() {
int size1 = tx1.messageSize();
int size2 = tx1.getMessageSizeForPriorityCalc();
assertEquals(113, size1 - size2);
tx1.getInput(0).setScriptSig(Script.parse(new byte[109]));
tx1.replaceInput(0, tx1.getInput(0).withScriptSig(Script.parse(new byte[109])));
assertEquals(78, tx1.getMessageSizeForPriorityCalc());
tx1.getInput(0).setScriptSig(Script.parse(new byte[110]));
tx1.replaceInput(0, tx1.getInput(0).withScriptSig(Script.parse(new byte[110])));
assertEquals(78, tx1.getMessageSizeForPriorityCalc());
tx1.getInput(0).setScriptSig(Script.parse(new byte[111]));
tx1.replaceInput(0, tx1.getInput(0).withScriptSig(Script.parse(new byte[111])));
assertEquals(79, tx1.getMessageSizeForPriorityCalc());
}

Expand Down
Loading

0 comments on commit fb40b17

Please sign in to comment.