Segwit light for spending coins.

Handle segwit-light properly when spending coins, either for staking
or normal transactions in the wallet.  Depending on when a transaction
was confirmed, we need to make sure to include the right outpoint (with
txid or bare txid) in the transaction spending an output.

Author: domob1812

divi/src/BlockMemoryPoolTransactionCollector.cpp

@@ -202,7 +202,7 @@ std::vector<TxPriority> BlockMemoryPoolTransactionCollector::PrioritizeMempoolTr
             // Read prev transaction
             if (!view.HaveCoins(txin.prevout.hash)) {
                 CTransaction prevTx;
-                if(!mempool_.lookup(txin.prevout.hash, prevTx)) {
+                if(!mempool_.lookupOutpoint(txin.prevout.hash, prevTx)) {
                     // This should never happen; all transactions in the memory
                     // pool should connect to either transactions in the chain
                     // or other transactions in the memory pool.

divi/src/PoSTransactionCreator.cpp

@@ -133,8 +133,8 @@ bool PoSTransactionCreator::SetSuportedStakingScript(
     const StakableCoin& stakableCoin,
     CMutableTransaction& txNew)
 {
-    txNew.vin.push_back(CTxIn(stakableCoin.tx->GetHash(), stakableCoin.outputIndex));
-    txNew.vout.push_back(CTxOut(0, stakableCoin.tx->vout[stakableCoin.outputIndex].scriptPubKey ));
+    txNew.vin.emplace_back(stakableCoin.tx->GetHashForSpendingOutput(), stakableCoin.outputIndex);
+    txNew.vout.emplace_back(0, stakableCoin.tx->vout[stakableCoin.outputIndex].scriptPubKey);
 
     return true;
 }
@@ -150,7 +150,7 @@ void PoSTransactionCreator::CombineUtxos(
     for(const StakableCoin& pcoin : stakedCoins_->asSet())
     {
         if(pcoin.tx->vout[pcoin.outputIndex].scriptPubKey == txNew.vout[1].scriptPubKey &&
-            pcoin.tx->GetHash() != txNew.vin[0].prevout.hash)
+            pcoin.tx->GetHashForSpendingOutput() != txNew.vin[0].prevout.hash)
         {
             if(pcoin.tx->vout[pcoin.outputIndex].nValue + nCredit > nCombineThreshold)
                 continue;
@@ -171,7 +171,7 @@ void PoSTransactionCreator::CombineUtxos(
             nCredit + pcoin.tx->vout[pcoin.outputIndex].nValue > nCombineThreshold)
             break;
 
-        txNew.vin.push_back(CTxIn(pcoin.tx->GetHash(), pcoin.outputIndex));
+        txNew.vin.emplace_back(pcoin.tx->GetHashForSpendingOutput(), pcoin.outputIndex);
         nCredit += pcoin.tx->vout[pcoin.outputIndex].nValue;
         walletTransactions.push_back(pcoin.tx);
     }
@@ -340,4 +340,4 @@ bool PoSTransactionCreator::CreateProofOfStake(
 
     stakedCoins_->resetTimestamp(); //this will trigger stake set to repopulate next round
     return true;
-}
+}

divi/src/StakableCoin.h

@@ -1,10 +1,10 @@
 #ifndef STAKABLE_COIN_H
 #define STAKABLE_COIN_H
+#include <merkletx.h>
 #include <uint256.h>
-#include <primitives/transaction.h>
 struct StakableCoin
 {
-    const CTransaction* tx;
+    const CMerkleTx* tx;
     unsigned outputIndex;
     uint256 blockHashOfFirstConfirmation;
 
@@ -16,7 +16,7 @@ struct StakableCoin
     }
 
     StakableCoin(
-        const CTransaction* txIn,
+        const CMerkleTx* txIn,
         unsigned outputIndexIn,
         uint256 blockHashIn
         ): tx(txIn)
@@ -29,4 +29,4 @@ struct StakableCoin
         return blockHashOfFirstConfirmation < other.blockHashOfFirstConfirmation;
     }
 };
-#endif//STAKABLE_COIN_H
+#endif//STAKABLE_COIN_H

divi/src/kernel.cpp

@@ -254,7 +254,7 @@ bool CheckProofOfStakeContextAndRecoverStakingData(
     uint256 hashBlock;
     CTransaction txPrev;
     if (!GetTransaction(txin.prevout.hash, txPrev, hashBlock, true))
-        return error("CheckProofOfStake() : INFO: read txPrev failed");
+        return error("CheckProofOfStake() : INFO: read txPrev failed for %s", txin.prevout.hash.GetHex());
 
     const CScript &kernelScript = txPrev.vout[txin.prevout.n].scriptPubKey;
 

divi/src/merkletx.cpp

@@ -3,6 +3,7 @@
 #include <primitives/block.h>
 #include <chainparams.h>
 #include <chain.h>
+#include <ForkActivation.h>
 #include <txmempool.h>
 #include <ValidationState.h>
 #include <swifttx.h>
@@ -182,4 +183,23 @@ bool CMerkleTx::IsTransactionLockTimedOut() const
     }
 
     return false;
-}
+}
+
+uint256 CMerkleTx::GetHashForSpendingOutput() const
+{
+    const CBlockIndex* pindexBlock = nullptr;
+    const int depth = GetNumberOfBlockConfirmationsINTERNAL(pindexBlock);
+
+    /* If the transaction is not yet confirmed in a block, we use the current
+       tip to determine the segwit-light activation status.  This is not
+       perfect around the activation time, but there is nothing we can do
+       in that case anyway.  Mempool and wallet discourage spending unconfirmed
+       outputs around the segwit-light fork anyway.  */
+    if (depth <= 0)
+        pindexBlock = chainActive.Tip();
+
+    assert(pindexBlock != nullptr);
+    const ActivationState as(pindexBlock);
+
+    return as.IsActive(Fork::SegwitLight) ? GetBareTxid() : GetHash();
+}

divi/src/merkletx.h

@@ -57,5 +57,11 @@ class CMerkleTx : public CTransaction
     bool AcceptToMemoryPool(bool fLimitFree = true, bool fRejectInsaneFee = true, bool ignoreFees = false);
     int GetTransactionLockSignatures() const;
     bool IsTransactionLockTimedOut() const;
+
+    /** Returns the prevout hash that should be used for spending outputs
+     *  created by this transaction.  This takes the activation of segwit-light
+     *  at the time of confirmation into account.  */
+    uint256 GetHashForSpendingOutput() const;
+
 };
-#endif// MERKLE_TX_H
+#endif// MERKLE_TX_H

divi/src/rpcrawtransaction.cpp

@@ -322,6 +322,8 @@ Value listunspent(const Array& params, bool fHelp)
             "[                   (array of json object)\n"
             "  {\n"
             "    \"txid\" : \"txid\",        (string) the transaction id \n"
+            "    \"baretxid\" : \"baretxid\", (string) The bare txid (without signatures)\n"
+            "    \"outputhash\" :  \"outputhash\", (string) The hash (txid or bare txid) that should be used for spending\n"
             "    \"vout\" : n,               (numeric) the vout value\n"
             "    \"address\" : \"address\",  (string) the divi address\n"
             "    \"account\" : \"account\",  (string) The associated account, or \"\" for the default account\n"
@@ -379,6 +381,8 @@ Value listunspent(const Array& params, bool fHelp)
         const CScript& pk = out.tx->vout[out.i].scriptPubKey;
         Object entry;
         entry.push_back(Pair("txid", out.tx->GetHash().GetHex()));
+        entry.push_back(Pair("baretxid", out.tx->GetBareTxid().GetHex()));
+        entry.push_back(Pair("outputhash", out.tx->GetHashForSpendingOutput().GetHex()));
         entry.push_back(Pair("vout", out.i));
         CTxDestination address;
         if (ExtractDestination(out.tx->vout[out.i].scriptPubKey, address)) {

divi/src/txmempool.cpp

@@ -696,12 +696,7 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const
 
     CCoinsViewCache mempoolDuplicate(const_cast<CCoinsViewCache*>(pcoins));
 
-    /* We use an activation state based on the current tip for determining
-       the state of segwit-light.  Spending of unconfirmed outputs in the
-       mempool is not allowed in a time window "around" the fork, so that
-       this should be good enough.  */
     const ActivationState as(chainActive.Tip());
-    const bool segwitLight = as.IsActive(Fork::SegwitLight);
 
     LOCK(cs);
     list<const CTxMemPoolEntry*> waitingOnDependants;
@@ -713,8 +708,7 @@ void CTxMemPool::check(const CCoinsViewCache* pcoins) const
         for (const auto& txin : tx.vin) {
             // Check that every mempool transaction's inputs refer to available coins, or other mempool tx's.
             CTransaction tx2;
-            if ((segwitLight && lookupBareTxid(txin.prevout.hash, tx2))
-                  || (!segwitLight && lookup(txin.prevout.hash, tx2))) {
+            if (lookupOutpoint(txin.prevout.hash, tx2)) {
                 assert(tx2.vout.size() > txin.prevout.n && !tx2.vout[txin.prevout.n].IsNull());
                 fDependsWait = true;
             } else {
@@ -794,6 +788,18 @@ bool CTxMemPool::lookupBareTxid(const uint256& btxid, CTransaction& result) cons
     return true;
 }
 
+bool CTxMemPool::lookupOutpoint(const uint256& hash, CTransaction& result) const
+{
+    /* We use an activation state based on the current tip for determining
+       the state of segwit-light.  Spending of unconfirmed outputs in the
+       mempool is not allowed in a time window "around" the fork, so that
+       this should be good enough.  */
+    const ActivationState as(chainActive.Tip());
+    return as.IsActive(Fork::SegwitLight)
+              ? lookupBareTxid(hash, result)
+              : lookup(hash, result);
+}
+
 CFeeRate CTxMemPool::estimateFee(int nBlocks) const
 {
     LOCK(cs);
@@ -873,10 +879,7 @@ bool CCoinsViewMemPool::GetCoins(const uint256& txid, CCoins& coins) const
     // conflict with the underlying cache, and it cannot have pruned entries (as it contains full)
     // transactions. First checking the underlying cache risks returning a pruned entry instead.
     CTransaction tx;
-    const ActivationState as(chainActive.Tip());
-    const bool segwitLight = as.IsActive(Fork::SegwitLight);
-    if ((segwitLight && mempool.lookupBareTxid(txid, tx))
-          || (!segwitLight && mempool.lookup(txid, tx))) {
+    if (mempool.lookupOutpoint(txid, tx)) {
         coins = CCoins(tx, MEMPOOL_HEIGHT);
         return true;
     }

divi/src/txmempool.h

@@ -192,6 +192,11 @@ class CTxMemPool
     bool lookup(const uint256& hash, CTransaction& result) const;
     bool lookupBareTxid(const uint256& btxid, CTransaction& result) const;
 
+    /** Looks up a transaction by its outpoint for spending.  This takes the
+     *  state of segwit light activation into account for deciding whether to
+     *  use the normal txid or the bare txid map.  */
+    bool lookupOutpoint(const uint256& hash, CTransaction& result) const;
+
     /** Estimate fee rate needed to get into the next nBlocks */
     CFeeRate estimateFee(int nBlocks) const;
 

divi/src/wallet.cpp

@@ -354,7 +354,7 @@ CAmount CWallet::GetDebit(const CWalletTx& tx, const isminefilter& filter) const
 CAmount CWallet::ComputeCredit(const CWalletTx& tx, const isminefilter& filter, int creditFilterFlags) const
 {
     CAmount nCredit = 0;
-    uint256 hash = tx.GetHash();
+    const uint256 hash = tx.GetHashForSpendingOutput();
     for (unsigned int i = 0; i < tx.vout.size(); i++) {
         if( (creditFilterFlags & REQUIRE_UNSPENT) && IsSpent(tx,i)) continue;
         if( (creditFilterFlags & REQUIRE_UNLOCKED) && IsLockedCoin(hash,i)) continue;
@@ -952,7 +952,7 @@ set<uint256> CWallet::GetConflicts(const uint256& txid) const
  */
 bool CWallet::IsSpent(const CWalletTx& wtx, unsigned int n) const
 {
-    return outputTracker_->IsSpent(wtx.GetHash(), n);
+    return outputTracker_->IsSpent(wtx.GetHashForSpendingOutput(), n);
 }
 
 bool CWallet::GetMasternodeVinAndKeys(CTxIn& txinRet, CPubKey& pubKeyRet, CKey& keyRet, std::string strTxHash, std::string strOutputIndex)
@@ -997,7 +997,7 @@ bool CWallet::GetVinAndKeysFromOutput(COutput out, CTxIn& txinRet, CPubKey& pubK
 
     CScript pubScript;
 
-    txinRet = CTxIn(out.tx->GetHash(), out.i);
+    txinRet = CTxIn(out.tx->GetHashForSpendingOutput(), out.i);
     pubScript = out.tx->vout[out.i].scriptPubKey; // the inputs PubKey
 
     CTxDestination address1;
@@ -1723,7 +1723,7 @@ bool CWallet::IsAvailableForSpending(const CWalletTx* pcoin, unsigned int i, con
         }
     }
 
-    const uint256 hash = pcoin->GetHash();
+    const uint256 hash = pcoin->GetHashForSpendingOutput();
 
     if (IsSpent(*pcoin, i))
         return false;
@@ -2242,8 +2242,8 @@ bool CWallet::CreateTransaction(const vector<pair<CScript, CAmount> >& vecSend,
                 }
 
                 // Fill vin
-                BOOST_FOREACH (const PAIRTYPE(const CWalletTx*, unsigned int) & coin, setCoins)
-                        txNew.vin.push_back(CTxIn(coin.first->GetHash(), coin.second));
+                for (const auto& coin : setCoins)
+                    txNew.vin.emplace_back(coin.first->GetHashForSpendingOutput(), coin.second);
 
                 // Sign
                 int nIn = 0;