From ec15117aca1c478a88fae104846839de6acbd40f Mon Sep 17 00:00:00 2001 From: Zannick Date: Mon, 18 Oct 2021 18:41:19 -0700 Subject: [PATCH] Correctly track and test RingCT spends. 1. When creating a transaction, the record has dummy inputs, but we want to mark the actual txo's as pending spend so later transactions don't attempt to spend them. 2. When verifying that we aren't double-spending, if we're only testing that a transaction can be accepted, don't track the inputs as though we're trying to commit the transaction. This way we can test accepting a transaction and later commit it (e.g. as part of a batch). --- src/consensus/tx_verify.cpp | 8 ++++++-- src/consensus/tx_verify.h | 4 +++- src/validation.cpp | 2 +- src/veil/ringct/anonwallet.cpp | 16 +++++++++++----- src/veil/ringct/anonwallet.h | 2 +- 5 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/consensus/tx_verify.cpp b/src/consensus/tx_verify.cpp index 132bfb4b2f..b2ca860791 100644 --- a/src/consensus/tx_verify.cpp +++ b/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 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 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 diff --git a/src/consensus/tx_verify.h b/src/consensus/tx_verify.h index 25f33cc6b5..c1b606b479 100644 --- a/src/consensus/tx_verify.h +++ b/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) */ diff --git a/src/validation.cpp b/src/validation.cpp index 4bbdb728f9..acbc12dc47 100644 --- a/src/validation.cpp +++ b/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)); } diff --git a/src/veil/ringct/anonwallet.cpp b/src/veil/ringct/anonwallet.cpp index 1e082390f5..b92e4edf31 100644 --- a/src/veil/ringct/anonwallet.cpp +++ b/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& 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 spends; + spends.reserve(setCoins.size()); + std::transform(setCoins.begin(), setCoins.end(), std::back_inserter(spends), + [](std::pair coin) -> COutPoint { return COutPoint(coin.first->first, coin.second); }); + MarkInputsAsPendingSpend(spends); uint256 txid = txNew.GetHash(); if (nValueOutZerocoin) diff --git a/src/veil/ringct/anonwallet.h b/src/veil/ringct/anonwallet.h index 17220e2509..1f36046c0d 100644 --- a/src/veil/ringct/anonwallet.h +++ b/src/veil/ringct/anonwallet.h @@ -213,7 +213,7 @@ class AnonWallet void AddOutputRecordMetaData(CTransactionRecord &rtx, std::vector &vecSend); bool ExpandTempRecipients(std::vector &vecSend, std::string &sError); - void MarkInputsAsPendingSpend(CTransactionRecord &rtx); + void MarkInputsAsPendingSpend(const std::vector& rtxvin); bool AddCTData(CTxOutBase *txout, CTempRecipient &r, std::string &sError);