Transaction: Make zero inputs/outputs transactions parsable.

Switch to the bitcoin core reference segwit parsing algorithm because
some no-segwit transactions with no inputs are incorrectly parsed as
segwit.

This change is to be 100% compatible with the reference implementation
and to be compatible with the PSBT implementation.

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

@@ -335,10 +335,14 @@ protected long readVarInt(int offset) throws ProtocolException {
         }
     }
 
-    protected byte[] readBytes(int length) throws ProtocolException {
+    private void checkReadLength(int length) throws ProtocolException {
         if ((length > MAX_SIZE) || (cursor + length > payload.length)) {
             throw new ProtocolException("Claimed value length too large: " + length);
         }
+    }
+
+    protected byte[] readBytes(int length) throws ProtocolException {
+        checkReadLength(length);
         try {
             byte[] b = new byte[length];
             System.arraycopy(payload, cursor, b, 0, length);
@@ -349,6 +353,11 @@ protected byte[] readBytes(int length) throws ProtocolException {
         }
     }
 
+    protected byte readByte() throws ProtocolException {
+        checkReadLength(1);
+        return payload[cursor++];
+    }
+
     protected byte[] readByteArray() throws ProtocolException {
         long len = readVarInt();
         return readBytes((int)len);

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

@@ -42,6 +42,7 @@
 import java.io.*;
 import java.util.*;
 
+import static org.bitcoinj.core.NetworkParameters.ProtocolVersion.WITNESS_VERSION;
 import static org.bitcoinj.core.Utils.*;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
@@ -95,6 +96,12 @@ public int compare(final Transaction tx1, final Transaction tx2) {
     };
     private static final Logger log = LoggerFactory.getLogger(Transaction.class);
 
+    /**
+     * When this bit is set in protocolVersion, do not include witness. The actual value is the same as in Bitcoin Core
+     * for consistency.
+     */
+    public static final int SERIALIZE_TRANSACTION_NO_WITNESS = 0x40000000;
+
     /** Threshold for lockTime: below this value it is interpreted as block number, otherwise as timestamp. **/
     public static final int LOCKTIME_THRESHOLD = 500000000; // Tue Nov  5 00:53:20 1985 UTC
     /** Same but as a BigInteger for CHECKLOCKTIMEVERIFY */
@@ -283,6 +290,15 @@ public Sha256Hash getTxId() {
         return cachedTxId;
     }
 
+    /**
+     * Returns if tx witnesses are allowed based on the protocol version
+     */
+    private boolean allowWitness() {
+        int protocolVersion = serializer.getProtocolVersion();
+        return (protocolVersion & SERIALIZE_TRANSACTION_NO_WITNESS) == 0
+                && protocolVersion >= WITNESS_VERSION.getBitcoinProtocolVersion();
+    }
+
     /**
      * Returns the witness transaction id (aka witness id) as per BIP144. For transactions without witness, this is the
      * same as {@link #getTxId()}.
@@ -619,26 +635,46 @@ protected static int calcLength(byte[] buf, int offset) {
      */
     @Override
     protected void parse() throws ProtocolException {
+        boolean allowWitness = allowWitness();
+
         cursor = offset;
         optimalEncodingMessageSize = 4;
 
         // version
         version = readUint32();
-        // peek at marker
-        byte marker = payload[cursor];
-        boolean useSegwit = marker == 0;
-        // marker, flag
-        if (useSegwit) {
-            readBytes(2);
+        byte flags = 0;
+        // Try to parse the inputs. In case the dummy is there, this will be read as an empty array list.
+        parseInputs();
+        if (inputs.size() == 0 && allowWitness) {
+            // We read a dummy or an empty input
+            flags = readByte();
             optimalEncodingMessageSize += 2;
+
+            if (flags != 0) {
+                parseInputs();
+                parseOutputs();
+            } else {
+                outputs = new ArrayList<>(0);
+            }
+        } else {
+            // We read non-empty inputs. Assume normal outputs follows.
+            parseOutputs();
         }
-        // txin_count, txins
-        parseInputs();
-        // txout_count, txouts
-        parseOutputs();
-        // script_witnesses
-        if (useSegwit)
+
+        if (((flags & 1) != 0) && allowWitness) {
+            // The witness flag is present, and we support witnesses.
+            flags ^= 1;
+            // script_witnesses
             parseWitnesses();
+            if (!hasWitnesses()) {
+                // It's illegal to encode witnesses when all witness stacks are empty.
+                throw new ProtocolException("Superfluous witness record");
+            }
+        }
+        if (flags != 0) {
+            // Unknown flag in the serialization
+            throw new ProtocolException("Unknown transaction optional data");
+        }
         // lock_time
         lockTime = readUint32();
         optimalEncodingMessageSize += 4;
@@ -1421,8 +1457,7 @@ public synchronized Sha256Hash hashForWitnessSignature(
 
     @Override
     protected void bitcoinSerializeToStream(OutputStream stream) throws IOException {
-        boolean useSegwit = hasWitnesses()
-                && serializer.getProtocolVersion() >= NetworkParameters.ProtocolVersion.WITNESS_VERSION.getBitcoinProtocolVersion();
+        boolean useSegwit = hasWitnesses() && allowWitness();
         bitcoinSerializeToStream(stream, useSegwit);
     }
 

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

@@ -665,4 +665,23 @@ public void getWeightAndVsize() {
         assertEquals(542, tx.getWeight());
         assertEquals(136, tx.getVsize());
     }
+
+    @Test
+    public void nonSegwitZeroInputZeroOutputTx() {
+        // Non SegWit tx with zero input and outputs
+        String txHex = "010000000000f1f2f3f4";
+        Transaction tx = UNITTEST.getDefaultSerializer().makeTransaction(HEX.decode(txHex));
+        assertEquals(txHex, HEX.encode(tx.bitcoinSerialize()));
+    }
+
+    @Test
+    public void nonSegwitZeroInputOneOutputTx() {
+        // Non SegWit tx with zero input and one output that has an amount of `0100000000000000` that could confuse
+        // a naive segwit parser. This can only be read with SegWit disabled
+        MessageSerializer serializer = UNITTEST.getDefaultSerializer();
+        String txHex = "0100000000010100000000000000016af1f2f3f4";
+        int protoVersionNoWitness = serializer.getProtocolVersion() | Transaction.SERIALIZE_TRANSACTION_NO_WITNESS;
+        tx = serializer.withProtocolVersion(protoVersionNoWitness).makeTransaction(HEX.decode(txHex));
+        assertEquals(txHex, HEX.encode(tx.bitcoinSerialize()));
+    }
 }

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

@@ -41,6 +41,7 @@
 import java.nio.charset.StandardCharsets;
 import java.util.*;
 
+import static org.bitcoinj.core.Transaction.SERIALIZE_TRANSACTION_NO_WITNESS;
 import static org.bitcoinj.core.Utils.HEX;
 import static org.bitcoinj.script.ScriptOpCodes.OP_0;
 import static org.bitcoinj.script.ScriptOpCodes.OP_INVALIDOPCODE;
@@ -394,7 +395,17 @@ public void dataDrivenInvalidTransactions() throws Exception {
             if (test.isArray() && test.size() == 1 && test.get(0).isTextual())
                 continue; // This is a comment.
             Map<TransactionOutPoint, Script> scriptPubKeys = parseScriptPubKeys(test.get(0));
-            Transaction transaction = TESTNET.getDefaultSerializer().makeTransaction(HEX.decode(test.get(1).asText().toLowerCase()));
+            byte[] txBytes = HEX.decode(test.get(1).asText().toLowerCase());
+            MessageSerializer serializer = TESTNET.getDefaultSerializer();
+            Transaction transaction;
+            try {
+                transaction = serializer.makeTransaction(txBytes);
+            } catch (ProtocolException ignore) {
+                // Try to parse as a no-witness transaction because some vectors are 0-input, 1-output txs that fail
+                // to correctly parse as witness transactions.
+                int protoVersionNoWitness = serializer.getProtocolVersion() | SERIALIZE_TRANSACTION_NO_WITNESS;
+                transaction = serializer.withProtocolVersion(protoVersionNoWitness).makeTransaction(txBytes);
+            }
             Set<VerifyFlag> verifyFlags = parseVerifyFlags(test.get(2).asText());
 
             boolean valid = true;