Implement new fields for the type and version of transaction

Signed-off-by: Dmitry Saveliev <[email protected]>

Author: dsaveliev

src/bench/verify_script.cpp

@@ -18,7 +18,7 @@
 static CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey)
 {
     CMutableTransaction txCredit;
-    txCredit.nVersion = 1;
+    txCredit.version = 1;
     txCredit.nLockTime = 0;
     txCredit.vin.resize(1);
     txCredit.vout.resize(1);
@@ -35,7 +35,7 @@ static CMutableTransaction BuildCreditingTransaction(const CScript& scriptPubKey
 static CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CMutableTransaction& txCredit)
 {
     CMutableTransaction txSpend;
-    txSpend.nVersion = 1;
+    txSpend.version = 1;
     txSpend.nLockTime = 0;
     txSpend.vin.resize(1);
     txSpend.vout.resize(1);

src/compressor.h

@@ -38,8 +38,8 @@ class CScriptCompressor
 private:
     /**
      * make this static for now (there are only 6 special scripts defined)
-     * this can potentially be extended together with a new nVersion for
-     * transactions, in which case this value becomes dependent on nVersion
+     * this can potentially be extended together with a new version for
+     * transactions, in which case this value becomes dependent on version
      * and nHeight of the enclosing transaction.
      */
     static const unsigned int nSpecialScripts = 6;

src/consensus/tx_verify.cpp

@@ -42,11 +42,10 @@ std::pair<int, int64_t> CalculateSequenceLocks(const CTransaction &tx, int flags
     int nMinHeight = -1;
     int64_t nMinTime = -1;
 
-    // tx.nVersion is signed integer so requires cast to unsigned otherwise
-    // we would be doing a signed comparison and half the range of nVersion
+    // tx.version is signed integer so requires cast to unsigned otherwise
+    // we would be doing a signed comparison and half the range of version
     // wouldn't support BIP 68.
-    bool fEnforceBIP68 = static_cast<uint32_t>(tx.nVersion) >= 2
-                      && flags & LOCKTIME_VERIFY_SEQUENCE;
+    bool fEnforceBIP68 = tx.version >= 2 && flags & LOCKTIME_VERIFY_SEQUENCE;
 
     // Do not enforce sequence numbers as a relative lock time
     // unless we have been instructed to

src/esperanza/walletextension.cpp

@@ -248,8 +248,7 @@ bool WalletExtension::SetMasterKeyFromSeed(const key::mnemonic::Seed &seed,
 }
 
 bool WalletExtension::BackupWallet() {
-  const std::string wallet_file_name = m_enclosing_wallet.GetName().empty() ?
-          "wallet.dat" : m_enclosing_wallet.GetName();
+  const std::string wallet_file_name = m_enclosing_wallet.GetName().empty() ? "wallet.dat" : m_enclosing_wallet.GetName();
   const int64_t current_time = GetTime();
 
   const std::string backup_wallet_filename =
@@ -612,7 +611,6 @@ void WalletExtension::VoteIfNeeded() {
     return;
   }
 
-
   const CWalletTx *prev_tx = m_enclosing_wallet.GetWalletTx(validator->m_last_transaction_hash);
   assert(prev_tx);
 

src/policy/policy.cpp

@@ -93,7 +93,7 @@ bool IsStandardTx(const CTransaction& tx, std::string& reason)
         return true;
     }
 
-    if (tx.nVersion > CTransaction::MAX_STANDARD_VERSION || tx.nVersion < 1) {
+    if (tx.version > CTransaction::MAX_STANDARD_VERSION || tx.version < 1) {
         reason = "version";
         return false;
     }

src/policy/policy.h

@@ -22,8 +22,8 @@ static const unsigned int DEFAULT_BLOCK_MAX_WEIGHT = MAX_BLOCK_WEIGHT - 4000;
 static const unsigned int DEFAULT_BLOCK_MIN_TX_FEE = 1000;
 /** The maximum weight for transactions we're willing to relay/mine */
 static const unsigned int MAX_STANDARD_TX_WEIGHT = 400000;
-/** The minimum non-witness size for transactions we're willing to relay/mine (1 segwit input + 1 P2WPKH output = 82 bytes) */
-static const unsigned int MIN_STANDARD_TX_NONWITNESS_SIZE = 82;
+/** The minimum non-witness size for transactions we're willing to relay/mine (1 segwit input + 1 P2WPKH output = 80 bytes) */
+static const unsigned int MIN_STANDARD_TX_NONWITNESS_SIZE = 80;
 /** Maximum number of signature check operations in an IsStandard() P2SH script */
 static const unsigned int MAX_P2SH_SIGOPS = 15;
 /** The maximum number of sigops we're willing to relay/mine in a single tx */

src/primitives/transaction.cpp

@@ -55,15 +55,15 @@ std::string CTxOut::ToString() const
     return strprintf("CTxOut(nValue=%d.%08d, scriptPubKey=%s)", nValue / UNIT, nValue % UNIT, HexStr(scriptPubKey).substr(0, 30));
 }
 
-CMutableTransaction::CMutableTransaction() : nVersion(CTransaction::CURRENT_VERSION), nLockTime(0) {}
-CMutableTransaction::CMutableTransaction(const CTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nLockTime(tx.nLockTime) {}
+CMutableTransaction::CMutableTransaction() : version(CTransaction::CURRENT_VERSION), type(TxType::REGULAR), nLockTime(0) {}
+CMutableTransaction::CMutableTransaction(const CTransaction& tx) : vin(tx.vin), vout(tx.vout), version(tx.version), type(tx.type), nLockTime(tx.nLockTime) {}
 
 void CMutableTransaction::SetType(TxType type) {
-  nVersion = (nVersion & 0x0000FFFF) | (static_cast<uint16_t>(type) << 16);
+  this->type = static_cast<uint8_t>(type);
 }
 
-void CMutableTransaction::SetVersion(uint16_t type) {
-  nVersion = (nVersion & 0xFFFF0000) | (static_cast<uint16_t>(type));
+void CMutableTransaction::SetVersion(uint8_t version) {
+  this->version = version;
 }
 
 uint256 CMutableTransaction::GetHash() const
@@ -85,9 +85,9 @@ uint256 CTransaction::ComputeWitnessHash() const
 }
 
 /* For backward compatibility, the hash is initialized to 0. TODO: remove the need for this default constructor entirely. */
-CTransaction::CTransaction() : vin(), vout(), nVersion(CTransaction::CURRENT_VERSION), nLockTime(0), hash{}, m_witness_hash{} {}
-CTransaction::CTransaction(const CMutableTransaction& tx) : vin(tx.vin), vout(tx.vout), nVersion(tx.nVersion), nLockTime(tx.nLockTime), hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}
-CTransaction::CTransaction(CMutableTransaction&& tx) : vin(std::move(tx.vin)), vout(std::move(tx.vout)), nVersion(tx.nVersion), nLockTime(tx.nLockTime), hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}
+CTransaction::CTransaction() : vin(), vout(), version(CTransaction::CURRENT_VERSION), type(TxType::REGULAR), nLockTime(0), hash{}, m_witness_hash{} {}
+CTransaction::CTransaction(const CMutableTransaction& tx) : vin(tx.vin), vout(tx.vout), version(tx.version), type(tx.type), nLockTime(tx.nLockTime), hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}
+CTransaction::CTransaction(CMutableTransaction&& tx) : vin(std::move(tx.vin)), vout(std::move(tx.vout)), version(tx.version), type(tx.type), nLockTime(tx.nLockTime), hash{ComputeHash()}, m_witness_hash{ComputeWitnessHash()} {}
 
 CAmount CTransaction::GetValueOut() const
 {
@@ -110,7 +110,7 @@ std::string CTransaction::ToString() const
     std::string str;
     str += strprintf("CTransaction(hash=%s, ver=%d, vin.size=%u, vout.size=%u, nLockTime=%u)\n",
         GetHash().ToString().substr(0,10),
-        nVersion,
+        version,
         vin.size(),
         vout.size(),
         nLockTime);

src/primitives/transaction.h

@@ -180,13 +180,15 @@ struct CMutableTransaction;
 
 /**
  * Basic transaction serialization format:
- * - int32_t nVersion
+ * - uint8_t type
+ * - uint8_t version
  * - std::vector<CTxIn> vin
  * - std::vector<CTxOut> vout
  * - uint32_t nLockTime
  *
  * Extended transaction serialization format:
- * - int32_t nVersion
+ * - uint8_t type
+ * - uint8_t version
  * - unsigned char dummy = 0x00
  * - unsigned char flags (!= 0)
  * - std::vector<CTxIn> vin
@@ -199,7 +201,8 @@ template<typename Stream, typename TxType>
 inline void UnserializeTransaction(TxType& tx, Stream& s) {
     const bool fAllowWitness = !(s.GetVersion() & SERIALIZE_TRANSACTION_NO_WITNESS);
 
-    s >> tx.nVersion;
+    s >> tx.type;
+    s >> tx.version;
     unsigned char flags = 0;
     tx.vin.clear();
     tx.vout.clear();
@@ -234,7 +237,8 @@ template<typename Stream, typename TxType>
 inline void SerializeTransaction(const TxType& tx, Stream& s) {
     const bool fAllowWitness = !(s.GetVersion() & SERIALIZE_TRANSACTION_NO_WITNESS);
 
-    s << tx.nVersion;
+    s << tx.type;
+    s << tx.version;
     unsigned char flags = 0;
     // Consistency check
     if (fAllowWitness) {
@@ -264,14 +268,12 @@ class TransactionBase
 {
 public:
 
-    //! Returns the version of this transaction (considers only the two lower bytes of nVersion).
-    uint16_t GetVersion() const {
-        return static_cast<uint16_t>(GetDerived()->nVersion);
+    uint8_t GetVersion() const {
+        return GetDerived()->version;
     }
 
-    //! Returns the transaction type (TxType) of this transaction (stored in the two upper bytes of the nVersion field).
     TxType GetType() const {
-        return TxType::_from_index_unchecked(GetDerived()->nVersion >> 16);
+        return TxType::_from_index_unchecked(GetDerived()->type);
     }
 
     bool IsRegular() const {
@@ -345,13 +347,13 @@ class CTransaction : public TransactionBase<CTransaction>
 {
 public:
     // Default transaction version.
-    static const int32_t CURRENT_VERSION=2;
+    static const uint8_t CURRENT_VERSION=2;
 
     // Changing the default transaction version requires a two step process: first
     // adapting relay policy by bumping MAX_STANDARD_VERSION, and then later date
     // bumping the default CURRENT_VERSION at which point both CURRENT_VERSION and
     // MAX_STANDARD_VERSION will be equal.
-    static const int32_t MAX_STANDARD_VERSION=2;
+    static const uint8_t MAX_STANDARD_VERSION=2;
 
     // The local variables are made const to prevent unintended modification
     // without updating the cached hash value. However, CTransaction is not
@@ -369,7 +371,8 @@ class CTransaction : public TransactionBase<CTransaction>
      * Thus the upper two bytes are used to piggy back that type (also uint16_t) onto
      * the version field.
      */
-    const uint32_t nVersion;
+    const uint8_t version;
+    const uint8_t type;
     const uint32_t nLockTime;
 
 private:
@@ -435,7 +438,8 @@ struct CMutableTransaction : public TransactionBase<CMutableTransaction>
 {
     std::vector<CTxIn> vin;
     std::vector<CTxOut> vout;
-    uint32_t nVersion;
+    uint8_t version;
+    uint8_t type;
     uint32_t nLockTime;
 
     CMutableTransaction();
@@ -457,7 +461,7 @@ struct CMutableTransaction : public TransactionBase<CMutableTransaction>
         Unserialize(s);
     }
 
-    void SetVersion(uint16_t version);
+    void SetVersion(uint8_t version);
 
     void SetType(TxType type);
 

src/proposer/proposer.cpp

@@ -120,8 +120,8 @@ class ProposerImpl : public Proposer {
             continue;
           }
           const EligibleCoin &coin = winning_ticket.get();
-          LogPrint(BCLog::PROPOSING, "Proposing... (wallet=%s, coin=%s)\n",
-                   wallet_name, util::to_string(coin.utxo));
+          LogPrint(BCLog::PROPOSING, "Proposing... (wallet=%s, coin=%d)\n",
+                   wallet_name, coin.utxo.GetOutputIndex());
           staking::TransactionPicker::PickTransactionsParameters parameters{};
           staking::TransactionPicker::PickTransactionsResult result =
               m_transaction_picker->PickTransactions(parameters);

src/script/interpreter.cpp

@@ -1333,8 +1333,10 @@ class CTransactionSignatureSerializer
     /** Serialize txTo */
     template<typename S>
     void Serialize(S &s) const {
-        // Serialize nVersion
-        ::Serialize(s, txTo.nVersion);
+        // Serialize type
+        ::Serialize(s, txTo.type);
+        // Serialize version
+        ::Serialize(s, txTo.version);
         // Serialize vin
         unsigned int nInputs = fAnyoneCanPay ? 1 : txTo.vin.size();
         ::WriteCompactSize(s, nInputs);
@@ -1433,8 +1435,10 @@ uint256 SignatureHash(const CScript& scriptCode, const T& txTo, unsigned int nIn
         }
 
         CHashWriter ss(SER_GETHASH, 0);
+        // Type
+        ss << txTo.type;
         // Version
-        ss << txTo.nVersion;
+        ss << txTo.version;
         // Input prevouts/nSequence (none/all, depending on flags)
         ss << hashPrevouts;
         ss << hashSequence;
@@ -1549,7 +1553,7 @@ bool GenericTransactionSignatureChecker<T>::CheckSequence(const CScriptNum& nSeq
 
     // Fail if the transaction's version number is not set high
     // enough to trigger BIP 68 rules.
-    if (static_cast<uint32_t>(txTo->nVersion) < 2)
+    if (txTo->version < 2)
         return false;
 
     // Sequence numbers with their most significant bit set are not