From 90be66777954ec04c51358ba74c907c39c8ca750 Mon Sep 17 00:00:00 2001 From: Andreas Schildbach Date: Mon, 3 Apr 2023 22:54:05 +0200 Subject: [PATCH] TransactionInput: divorce from `Message` It is never sent on its own, so it doesn't need to be a `Message`. * A static constructur `read()` replaces the native constructor that deserialized from a payload. * A `write()` helper replaces `bitcoinSerializeToStream()`. * A `serialize()` helper replaces `bitcoinSerialize()`. This comes with a test. --- .../java/org/bitcoinj/core/Transaction.java | 4 +- .../org/bitcoinj/core/TransactionInput.java | 95 ++++++++++++------- .../main/java/org/bitcoinj/wallet/Wallet.java | 2 +- .../bitcoinj/core/TransactionInputTest.java | 33 +++++++ .../org/bitcoinj/core/TransactionTest.java | 2 +- 5 files changed, 98 insertions(+), 38 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/core/Transaction.java b/core/src/main/java/org/bitcoinj/core/Transaction.java index 731112f13..b6307361d 100644 --- a/core/src/main/java/org/bitcoinj/core/Transaction.java +++ b/core/src/main/java/org/bitcoinj/core/Transaction.java @@ -679,7 +679,7 @@ public class Transaction extends Message { int numInputs = numInputsVarInt.intValue(); inputs = new ArrayList<>(Math.min((int) numInputs, Utils.MAX_INITIAL_ARRAY_LENGTH)); for (long i = 0; i < numInputs; i++) { - TransactionInput input = new TransactionInput(this, payload.slice()); + TransactionInput input = TransactionInput.read(payload.slice(), this); inputs.add(input); // intentionally read again, due to the slice above Buffers.skipBytes(payload, TransactionOutPoint.BYTES); @@ -1525,7 +1525,7 @@ public class Transaction extends Message { // txin_count, txins stream.write(VarInt.of(inputs.size()).serialize()); for (TransactionInput in : inputs) - in.bitcoinSerializeToStream(stream); + stream.write(in.serialize()); // txout_count, txouts stream.write(VarInt.of(outputs.size()).serialize()); for (TransactionOutput out : outputs) diff --git a/core/src/main/java/org/bitcoinj/core/TransactionInput.java b/core/src/main/java/org/bitcoinj/core/TransactionInput.java index 4fd9d219b..b4d740018 100644 --- a/core/src/main/java/org/bitcoinj/core/TransactionInput.java +++ b/core/src/main/java/org/bitcoinj/core/TransactionInput.java @@ -30,9 +30,8 @@ import org.bitcoinj.wallet.KeyBag; import org.bitcoinj.wallet.RedeemData; import javax.annotation.Nullable; -import java.io.IOException; -import java.io.OutputStream; import java.lang.ref.WeakReference; +import java.nio.BufferOverflowException; import java.nio.BufferUnderflowException; import java.nio.ByteBuffer; import java.util.Arrays; @@ -49,7 +48,7 @@ import static org.bitcoinj.base.internal.Preconditions.checkArgument; * *

Instances of this class are not safe for use by multiple threads.

*/ -public class TransactionInput extends Message { +public class TransactionInput { /** Magic sequence number that indicates there is no sequence number. */ public static final long NO_SEQUENCE = 0xFFFFFFFFL; /** @@ -103,17 +102,37 @@ public class TransactionInput extends Message { return new TransactionInput(parentTransaction, scriptBytes, TransactionOutPoint.UNCONNECTED); } - public TransactionInput(@Nullable Transaction parentTransaction, byte[] scriptBytes, - TransactionOutPoint outpoint) { - this(parentTransaction, scriptBytes, outpoint, null); + /** + * Deserialize this transaction input from a given payload. + * + * @param payload payload to deserialize from + * @param parentTransaction parent transaction of the input + * @return read message + * @throws BufferUnderflowException if the read message extends beyond the remaining bytes of the payload + */ + public static TransactionInput read(ByteBuffer payload, Transaction parentTransaction) throws BufferUnderflowException, ProtocolException { + Objects.requireNonNull(parentTransaction); + TransactionOutPoint outpoint = TransactionOutPoint.read(payload); + byte[] scriptBytes = Buffers.readLengthPrefixedBytes(payload); + long sequence = ByteUtils.readUint32(payload); + return new TransactionInput(parentTransaction, scriptBytes, outpoint, sequence, null); } public TransactionInput(@Nullable Transaction parentTransaction, byte[] scriptBytes, - TransactionOutPoint outpoint, @Nullable Coin value) { - super(); + TransactionOutPoint outpoint) { + this(parentTransaction, scriptBytes, outpoint, NO_SEQUENCE, null); + } + + public TransactionInput(@Nullable Transaction parentTransaction, byte[] scriptBytes, + TransactionOutPoint outpoint, @Nullable Coin value) { + this(parentTransaction, scriptBytes, outpoint, NO_SEQUENCE, value); + } + + private TransactionInput(@Nullable Transaction parentTransaction, byte[] scriptBytes, + TransactionOutPoint outpoint, long sequence, @Nullable Coin value) { this.scriptBytes = scriptBytes; this.outpoint = outpoint; - this.sequence = NO_SEQUENCE; + this.sequence = sequence; this.value = value; setParent(parentTransaction); } @@ -135,17 +154,6 @@ public class TransactionInput extends Message { this.value = output.getValue(); } - /** - * Deserializes an input message. This is usually part of a transaction message. - * @param payload Bitcoin protocol formatted byte array containing message content. - * @throws ProtocolException - */ - public TransactionInput(@Nullable Transaction parentTransaction, ByteBuffer payload) throws ProtocolException { - super(payload); - setParent(parentTransaction); - this.value = null; - } - /** * Gets the index of this input in the parent transaction, or throws if this input is freestanding. Iterates * over the parents list to discover this. @@ -157,14 +165,41 @@ public class TransactionInput extends Message { return myIndex; } - @Override - protected void parse(ByteBuffer payload) throws BufferUnderflowException, ProtocolException { - outpoint = TransactionOutPoint.read(payload); - scriptBytes = Buffers.readLengthPrefixedBytes(payload); - sequence = ByteUtils.readUint32(payload); + /** + * Write this transaction input into the given buffer. + * + * @param buf buffer to write into + * @return the buffer + * @throws BufferOverflowException if the input doesn't fit the remaining buffer + */ + public ByteBuffer write(ByteBuffer buf) throws BufferOverflowException { + buf.put(outpoint.serialize()); + Buffers.writeLengthPrefixedBytes(buf, scriptBytes); + ByteUtils.writeInt32LE(sequence, buf); + return buf; } - @Override + /** + * Allocates a byte array and writes this transaction input into it. + * + * @return byte array containing the transaction input + */ + public byte[] serialize() { + return write(ByteBuffer.allocate(getMessageSize())).array(); + } + + /** @deprecated use {@link #serialize()} */ + @Deprecated + public byte[] bitcoinSerialize() { + return serialize(); + } + + /** + * Return the size of the serialized message. Note that if the message was deserialized from a payload, this + * size can differ from the size of the original payload. + * + * @return size of the serialized message in bytes + */ public int getMessageSize() { int size = TransactionOutPoint.BYTES; size += VarInt.sizeOf(scriptBytes.length) + scriptBytes.length; @@ -172,14 +207,6 @@ public class TransactionInput extends Message { return size; } - @Override - protected void bitcoinSerializeToStream(OutputStream stream) throws IOException { - stream.write(outpoint.serialize()); - stream.write(VarInt.of(scriptBytes.length).serialize()); - stream.write(scriptBytes); - ByteUtils.writeInt32LE(sequence, stream); - } - /** * Coinbase transactions have special inputs with hashes of zero. If this is such an input, returns true. */ diff --git a/core/src/main/java/org/bitcoinj/wallet/Wallet.java b/core/src/main/java/org/bitcoinj/wallet/Wallet.java index f51c7e80d..0dafb52b7 100644 --- a/core/src/main/java/org/bitcoinj/wallet/Wallet.java +++ b/core/src/main/java/org/bitcoinj/wallet/Wallet.java @@ -5372,7 +5372,7 @@ public class Wallet extends BaseTaggableObject private void addSuppliedInputs(Transaction tx, List originalInputs) { for (TransactionInput input : originalInputs) - tx.addInput(new TransactionInput(tx, ByteBuffer.wrap(input.bitcoinSerialize()))); + tx.addInput(TransactionInput.read(ByteBuffer.wrap(input.bitcoinSerialize()), tx)); } private int estimateVirtualBytesForSigning(CoinSelection selection) { diff --git a/core/src/test/java/org/bitcoinj/core/TransactionInputTest.java b/core/src/test/java/org/bitcoinj/core/TransactionInputTest.java index 5d3ffed0d..d14dd4190 100644 --- a/core/src/test/java/org/bitcoinj/core/TransactionInputTest.java +++ b/core/src/test/java/org/bitcoinj/core/TransactionInputTest.java @@ -17,6 +17,8 @@ package org.bitcoinj.core; import com.google.common.collect.Lists; +import junitparams.JUnitParamsRunner; +import junitparams.Parameters; import org.bitcoinj.base.Address; import org.bitcoinj.base.BitcoinNetwork; import org.bitcoinj.base.Coin; @@ -31,13 +33,21 @@ import org.bitcoinj.wallet.SendRequest; import org.bitcoinj.wallet.Wallet; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; +import java.nio.Buffer; +import java.nio.ByteBuffer; +import java.util.Iterator; import java.util.List; +import java.util.Random; +import java.util.stream.Stream; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +@RunWith(JUnitParamsRunner.class) public class TransactionInputTest { private static final NetworkParameters TESTNET = TestNet3Params.get(); @@ -112,4 +122,27 @@ public class TransactionInputTest { TransactionInput coinbaseInput = TransactionInput.coinbaseInput(new Transaction(), new byte[2]); assertTrue(coinbaseInput.isCoinBase()); } + + @Test + @Parameters(method = "randomInputs") + public void readAndWrite(TransactionInput input) { + ByteBuffer buf = ByteBuffer.allocate(input.getMessageSize()); + input.write(buf); + assertFalse(buf.hasRemaining()); + ((Buffer) buf).rewind(); + TransactionInput inputCopy = TransactionInput.read(buf, input.getParentTransaction()); + assertFalse(buf.hasRemaining()); + assertEquals(input, inputCopy); + } + + private Iterator randomInputs() { + Random random = new Random(); + Transaction parent = new Transaction(); + return Stream.generate(() -> { + byte[] randomBytes = new byte[100]; + random.nextBytes(randomBytes); + return new TransactionInput(parent, randomBytes, TransactionOutPoint.UNCONNECTED, + Coin.ofSat(random.nextLong())); + }).limit(10).iterator(); + } } diff --git a/core/src/test/java/org/bitcoinj/core/TransactionTest.java b/core/src/test/java/org/bitcoinj/core/TransactionTest.java index 4e5df31bb..de7d7e8d5 100644 --- a/core/src/test/java/org/bitcoinj/core/TransactionTest.java +++ b/core/src/test/java/org/bitcoinj/core/TransactionTest.java @@ -736,7 +736,7 @@ public class TransactionTest { long inputsSize = hackInputsSize ? Integer.MAX_VALUE : getInputs().size(); stream.write(VarInt.of(inputsSize).serialize()); for (TransactionInput in : getInputs()) - in.bitcoinSerializeToStream(stream); + stream.write(in.serialize()); // txout_count, txouts long outputsSize = hackOutputsSize ? Integer.MAX_VALUE : getOutputs().size(); stream.write(VarInt.of(outputsSize).serialize());