Merge #215: [Consensus] Change Elements serialization format

d3a48d5 changed elements serialization to new style with no marker 0x00 byte and witness data at the end (tdudz)

Author: instagibbs

qa/rpc-tests/test_framework/mininode.py

@@ -598,25 +598,17 @@ def __init__(self, tx=None):
 
     def deserialize(self, f):
         self.nVersion = struct.unpack("<i", f.read(4))[0]
+        flags = struct.unpack("<B", f.read(1))[0]
         self.vin = deser_vector(f, CTxIn)
-        flags = 0
-        if len(self.vin) == 0:
-            flags = struct.unpack("<B", f.read(1))[0]
-            # Not sure why flags can't be zero, but this
-            # matches the implementation in bitcoind
-            if (flags != 0):
-                self.vin = deser_vector(f, CTxIn)
-                self.vout = deser_vector(f, CTxOut)
-        else:
-            self.vout = deser_vector(f, CTxOut)
+        self.vout = deser_vector(f, CTxOut)
+        self.nLockTime = struct.unpack("<I", f.read(4))[0]
         if flags & 1 > 0:
             self.wit.vtxinwit = [CTxInWitness() for i in range(len(self.vin))]
             self.wit.vtxoutwit = [CTxOutWitness() for i in range(len(self.vout))]
             self.wit.deserialize(f)
-
         if flags > 1:
             raise TypeError('Extra witness flags:' + str(flags))
-        self.nLockTime = struct.unpack("<I", f.read(4))[0]
+        
         self.sha256 = None
         self.hash = None
 
@@ -636,12 +628,10 @@ def serialize_with_witness(self):
             flags |= 1
         r = b""
         r += struct.pack("<i", self.nVersion)
-        if flags:
-            dummy = []
-            r += ser_vector(dummy)
-            r += struct.pack("<B", flags)
+        r += struct.pack("<B", flags)
         r += ser_vector(self.vin)
         r += ser_vector(self.vout)
+        r += struct.pack("<I", self.nLockTime)
         if flags & 1:
             if (len(self.wit.vtxinwit) != len(self.vin)):
                 # vtxinwit must have the same length as vin
@@ -652,7 +642,6 @@ def serialize_with_witness(self):
                 for i in range(len(self.wit.vtxoutwit), len(self.vout)):
                     self.wit.vtxoutwit.append(CTxInWitness())
             r += self.wit.serialize()
-        r += struct.pack("<I", self.nLockTime)
         return r
 
     def serialize(self):

src/bitcoin-tx.cpp

@@ -906,7 +906,7 @@ static int CommandLineRawTx(int argc, char* argv[])
             if (strHexTx == "-")                 // "-" implies standard input
                 strHexTx = readStdin();
 
-            if (!DecodeHexTx(tx, strHexTx, true))
+            if (!DecodeHexTx(tx, strHexTx))
                 throw std::runtime_error("invalid transaction encoding");
 
             startArg = 2;

src/core_io.h

@@ -18,7 +18,7 @@ class UniValue;
 // core_read.cpp
 CScript ParseScript(const std::string& s);
 std::string ScriptToAsmStr(const CScript& script, const bool fAttemptSighashDecode = false);
-bool DecodeHexTx(CMutableTransaction& tx, const std::string& strHexTx, bool fTryNoWitness = false);
+bool DecodeHexTx(CMutableTransaction& tx, const std::string& strHexTx);
 bool DecodeHexBlk(CBlock&, const std::string& strHexBlk);
 uint256 ParseHashUV(const UniValue& v, const std::string& strName);
 uint256 ParseHashStr(const std::string&, const std::string& strName);

src/core_read.cpp

@@ -88,26 +88,13 @@ CScript ParseScript(const std::string& s)
     return result;
 }
 
-bool DecodeHexTx(CMutableTransaction& tx, const std::string& strHexTx, bool fTryNoWitness)
+bool DecodeHexTx(CMutableTransaction& tx, const std::string& strHexTx)
 {
     if (!IsHex(strHexTx))
         return false;
 
     std::vector<unsigned char> txData(ParseHex(strHexTx));
 
-    if (fTryNoWitness) {
-        CDataStream ssData(txData, SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS);
-        try {
-            ssData >> tx;
-            if (ssData.eof()) {
-                return true;
-            }
-        }
-        catch (const std::exception&) {
-            // Fall through.
-        }
-    }
-
     CDataStream ssData(txData, SER_NETWORK, PROTOCOL_VERSION);
     try {
         ssData >> tx;

src/primitives/transaction.h

@@ -615,45 +615,31 @@ class CTxWitness
 struct CMutableTransaction;
 
 /**
- * Basic transaction serialization format:
+ * Elements transaction serialization format:
  * - int32_t nVersion
+ * - unsigned char flags
+ *     - bit 1: witness data
  * - std::vector<CTxIn> vin
  * - std::vector<CTxOut> vout
  * - uint32_t nLockTime
- *
- * Extended transaction serialization format:
- * - int32_t nVersion
- * - unsigned char dummy = 0x00
- * - unsigned char flags (!= 0)
- * - std::vector<CTxIn> vin
- * - std::vector<CTxOut> vout
  * - if (flags & 1):
  *   - CTxWitness wit;
- * - uint32_t nLockTime
  */
 template<typename Stream, typename TxType>
 inline void UnserializeTransaction(TxType& tx, Stream& s) {
-    const bool fAllowWitness = !(s.GetVersion() & SERIALIZE_TRANSACTION_NO_WITNESS);
     s >> tx.nVersion;
     unsigned char flags = 0;
     tx.vin.clear();
     tx.vout.clear();
     tx.wit.SetNull();
-    /* Try to read the vin. In case the dummy is there, this will be read as an empty vector. */
+
+    s >> flags;
     s >> tx.vin;
-    if (tx.vin.size() == 0 && fAllowWitness) {
-        /* We read a dummy or an empty vin. */
-        s >> flags;
-        if (flags != 0) {
-            s >> tx.vin;
-            s >> tx.vout;
-        }
-    } else {
-        /* We read a non-empty vin. Assume a normal vout follows. */
-        s >> tx.vout;
-    }
-    if ((flags & 1) && fAllowWitness) {
-        /* The witness flag is present, and we support witnesses. */
+    s >> tx.vout;
+    s >> tx.nLockTime;
+
+    if (flags & 1) {
+        /* The witness flag is present. */
         flags ^= 1;
         const_cast<CTxWitness*>(&tx.wit)->vtxinwit.resize(tx.vin.size());
         const_cast<CTxWitness*>(&tx.wit)->vtxoutwit.resize(tx.vout.size());
@@ -663,43 +649,35 @@ inline void UnserializeTransaction(TxType& tx, Stream& s) {
         /* Unknown flag in the serialization */
         throw std::ios_base::failure("Unknown transaction optional data");
     }
-    s >> tx.nLockTime;
 }
 
 template<typename Stream, typename TxType>
 inline void SerializeTransaction(const TxType& tx, Stream& s) {
-    const bool fAllowWitness = !(s.GetVersion() & SERIALIZE_TRANSACTION_NO_WITNESS);
-
+	const bool fAllowWitness = !(s.GetVersion() & SERIALIZE_TRANSACTION_NO_WITNESS);
     s << tx.nVersion;
-
     unsigned char flags = 0;
     // Consistency check
     assert(tx.wit.vtxoutwit.size() <= tx.vout.size());
-    if (fAllowWitness) {
-        /* Check whether witnesses need to be serialized. */
-        if (tx.HasWitness()) {
-            flags |= 1;
-        }
-    }
-    if (flags) {
-        /* Use extended format in case witnesses are to be serialized. */
-        std::vector<CTxIn> vinDummy;
-        s << vinDummy;
-        s << flags;
+
+    /* Check whether witnesses need to be serialized. */
+    if (fAllowWitness && tx.HasWitness()) {
+        flags |= 1;
     }
+
+    s << flags;
     s << tx.vin;
     s << tx.vout;
+    s << tx.nLockTime;
+
     if (flags & 1) {
         const_cast<CTxWitness*>(&tx.wit)->vtxinwit.resize(tx.vin.size());
         const_cast<CTxWitness*>(&tx.wit)->vtxoutwit.resize(tx.vout.size());
         s << tx.wit;
     }
-    s << tx.nLockTime;
 }
 
-
 /** The basic transaction that is broadcasted on the network and contained in
- * blocks.  A transaction can contain multiple inputs and outputs.
+ * blocks. A transaction can contain multiple inputs and outputs.
  */
 class CTransaction
 {

src/rpc/rawtransaction.cpp

@@ -1075,7 +1075,7 @@ UniValue decoderawtransaction(const JSONRPCRequest& request)
 
     CMutableTransaction mtx;
 
-    if (!DecodeHexTx(mtx, request.params[0].get_str(), true))
+    if (!DecodeHexTx(mtx, request.params[0].get_str()))
         throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "TX decode failed");
 
     UniValue result(UniValue::VOBJ);