[Consensus][RingCT] Correctly track and test RingCT spends.

src/consensus/tx_verify.cpp

@@ -444,7 +444,7 @@ bool CheckTransaction(const CTransaction& tx, CValidationState &state, bool fSki
 }
 
 bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& inputs,
-                              int nSpendHeight, CAmount& txfee, CAmount& nValueIn, CAmount& nValueOut)
+                              int nSpendHeight, CAmount& txfee, CAmount& nValueIn, CAmount& nValueOut, bool test_accept)
 {
     // reset per tx
     state.fHasAnonOutput = false;
@@ -463,6 +463,8 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, c
     std::vector<const secp256k1_pedersen_commitment*> vpCommitsIn, vpCommitsOut;
     size_t nBasecoin = 0, nCt = 0, nRingCT = 0, nZerocoin = 0;
     nValueIn = 0;
+    // keyimages for just this tx, used when test_accept=true
+    std::set<CCmpPubKey> txHaveKI;
     for (unsigned int i = 0; i < tx.vin.size(); ++i) {
 
         uint32_t nInputs, nRingSize;
@@ -473,7 +475,9 @@ bool Consensus::CheckTxInputs(const CTransaction& tx, CValidationState& state, c
             for (size_t k = 0; k < nInputs; ++k) {
                 const CCmpPubKey &ki = *((CCmpPubKey*)&vKeyImages[k*33]);
 
-                if (!state.m_setHaveKI.insert(ki).second) {
+                if (test_accept
+                        ? state.m_setHaveKI.find(ki) != state.m_setHaveKI.end() && !txHaveKI.insert(ki).second
+                        : !state.m_setHaveKI.insert(ki).second) {
                     if (::Params().CheckKIenforced(nSpendHeight))
                         return state.DoS(100, false, REJECT_INVALID, "bad-anonin-dup-ki-tx-double");
                     else

src/consensus/tx_verify.h

@@ -28,11 +28,13 @@ namespace Consensus {
 /**
  * Check whether all inputs of this transaction are valid (no double spends and amounts)
  * This does not modify the UTXO set. This does not check scripts and sigs.
+ * When test_accept is false, keyimages are collected in state.m_setHaveKI to check for double spends;
+ * when test_accept is true, compares keyimages to state.m_setHaveKI but does not add this txn's keyimages.
  * @param[out] txfee Set to the transaction fee if successful.
  * Preconditions: tx.IsCoinBase() is false.
  */
 bool CheckTxInputs(const CTransaction& tx, CValidationState& state, const CCoinsViewCache& inputs, int nSpendHeight,
-                   CAmount& txfee, CAmount& nValueIn, CAmount& nValueOut);
+                   CAmount& txfee, CAmount& nValueIn, CAmount& nValueOut, bool test_accept=false);
 } // namespace Consensus
 
 /** Auxiliary functions for transaction validation (ideally should not be exposed) */

src/validation.cpp

@@ -900,7 +900,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
             return state.DoS(0, false, REJECT_NONSTANDARD, "non-BIP68-final");
 
         CAmount nFees, nValueIn, nValueOut;
-        if (!Consensus::CheckTxInputs(tx, state, view, GetSpendHeight(view), nFees, nValueIn, nValueOut)) {
+        if (!Consensus::CheckTxInputs(tx, state, view, GetSpendHeight(view), nFees, nValueIn, nValueOut, test_accept)) {
             return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state));
         }
 

src/veil/ringct/anonwallet.cpp

@@ -1254,9 +1254,9 @@ void AnonWallet::ParseAddressForMetaData(const CTxDestination &addr, COutputReco
     return;
 }
 
-void AnonWallet::MarkInputsAsPendingSpend(CTransactionRecord &rtx)
+void AnonWallet::MarkInputsAsPendingSpend(const std::vector<COutPoint>& vin)
 {
-    for (const COutPoint& outpointIn : rtx.vin) {
+    for (const COutPoint& outpointIn : vin) {
         auto it = mapRecords.find(outpointIn.hash);
         if (it == mapRecords.end())
             continue;
@@ -2295,7 +2295,7 @@ int AnonWallet::AddStandardInputs_Inner(CWalletTx &wtx, CTransactionRecord &rtx,
         rtx.nFlags |= ORF_FROM; //Set from me
         rtx.nFlags |= ORF_BASECOIN_IN;
         AddOutputRecordMetaData(rtx, vecSend);
-        MarkInputsAsPendingSpend(rtx);
+        MarkInputsAsPendingSpend(rtx.vin);
 
         uint256 txid = txNew.GetHash();
         if (fZerocoinInputs)
@@ -2845,7 +2845,7 @@ int AnonWallet::AddBlindedInputs_Inner(CWalletTx &wtx, CTransactionRecord &rtx,
         AddOutputRecordMetaData(rtx, vecSend);
         for (auto txin : txNew.vin)
             rtx.vin.emplace_back(txin.prevout);
-        MarkInputsAsPendingSpend(rtx);
+        MarkInputsAsPendingSpend(rtx.vin);
 
         uint256 txid = txNew.GetHash();
         if (nValueOutZerocoin)
@@ -3723,7 +3723,13 @@ bool AnonWallet::AddAnonInputs_Inner(CWalletTx &wtx, CTransactionRecord &rtx, st
 
         for (auto txin : txNew.vin)
             rtx.vin.emplace_back(txin.prevout);
-        MarkInputsAsPendingSpend(rtx);
+
+        // Convert the real inputs (setCoins) into COutPoints that we can mark as pending spends
+        std::vector<COutPoint> spends;
+        spends.reserve(setCoins.size());
+        std::transform(setCoins.begin(), setCoins.end(), std::back_inserter(spends),
+                [](std::pair<MapRecords_t::const_iterator, unsigned int> coin) -> COutPoint { return COutPoint(coin.first->first, coin.second); });
+        MarkInputsAsPendingSpend(spends);
 
         uint256 txid = txNew.GetHash();
         if (nValueOutZerocoin)

src/veil/ringct/anonwallet.h

@@ -213,7 +213,7 @@ class AnonWallet
 
     void AddOutputRecordMetaData(CTransactionRecord &rtx, std::vector<CTempRecipient> &vecSend);
     bool ExpandTempRecipients(std::vector<CTempRecipient> &vecSend, std::string &sError);
-    void MarkInputsAsPendingSpend(CTransactionRecord &rtx);
+    void MarkInputsAsPendingSpend(const std::vector<COutPoint>& rtxvin);
 
     bool AddCTData(CTxOutBase *txout, CTempRecipient &r, std::string &sError);