diff --git a/src/net_processing.cpp b/src/net_processing.cpp index fd29c706ba20e..e0d337b5882ba 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -24,7 +24,7 @@ int64_t nTimeBestReceived = 0; // Used only to inform the wallet of when we las static const uint64_t RANDOMIZER_ID_ADDRESS_RELAY = 0x3cac0035b5866b90ULL; // SHA256("main address relay")[0:8] struct COrphanTx { - CTransaction tx; + CTransactionRef tx; NodeId fromPeer; }; @@ -511,9 +511,9 @@ void UnregisterNodeSignals(CNodeSignals& nodeSignals) // mapOrphanTransactions // -bool AddOrphanTx(const CTransaction& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +bool AddOrphanTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { - uint256 hash = tx.GetHash(); + const uint256& hash = tx->GetHash(); if (mapOrphanTransactions.count(hash)) return false; @@ -524,15 +524,15 @@ bool AddOrphanTx(const CTransaction& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(c // have been mined or received. // 10,000 orphans, each of which is at most 5,000 bytes big is // at most 500 megabytes of orphans: - unsigned int sz = GetSerializeSize(tx, SER_NETWORK, CTransaction::CURRENT_VERSION); + unsigned int sz = GetSerializeSize(*tx, SER_NETWORK, CTransaction::CURRENT_VERSION); if (sz > 5000) { LogPrint(BCLog::MEMPOOL, "ignoring large orphan tx (size: %u, hash: %s)\n", sz, hash.ToString()); return false; } - mapOrphanTransactions[hash].tx = tx; - mapOrphanTransactions[hash].fromPeer = peer; - for (const CTxIn& txin : tx.vin) + auto ret = mapOrphanTransactions.emplace(hash, COrphanTx{tx, peer}); + assert(ret.second); + for (const CTxIn& txin : tx->vin) mapOrphanTransactionsByPrev[txin.prevout.hash].insert(hash); LogPrint(BCLog::MEMPOOL, "stored orphan tx %s (mapsz %u prevsz %u)\n", hash.ToString(), @@ -545,7 +545,7 @@ void static EraseOrphanTx(uint256 hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) std::map::iterator it = mapOrphanTransactions.find(hash); if (it == mapOrphanTransactions.end()) return; - for (const CTxIn& txin : it->second.tx.vin) { + for (const CTxIn& txin : it->second.tx->vin) { std::map >::iterator itPrev = mapOrphanTransactionsByPrev.find(txin.prevout.hash); if (itPrev == mapOrphanTransactionsByPrev.end()) continue; @@ -563,7 +563,7 @@ void EraseOrphansFor(NodeId peer) EXCLUSIVE_LOCKS_REQUIRED(cs_main) while (iter != mapOrphanTransactions.end()) { std::map::iterator maybeErase = iter++; // increment to avoid iterator becoming invalid if (maybeErase->second.fromPeer == peer) { - EraseOrphanTx(maybeErase->second.tx.GetHash()); + EraseOrphanTx(maybeErase->second.tx->GetHash()); ++nErased; } } @@ -1401,8 +1401,9 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR else if (strCommand == NetMsgType::TX) { std::vector vWorkQueue; std::vector vEraseQueue; - CTransaction tx; - vRecv >> tx; + CTransactionRef ptx; + vRecv >> ptx; + const CTransaction& tx = *ptx; CInv inv(MSG_TX, tx.GetHash()); pfrom->AddInventoryKnown(inv); @@ -1416,7 +1417,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR mapAlreadyAskedFor.erase(inv); - if (!tx.HasZerocoinSpendInputs() && AcceptToMemoryPool(mempool, state, tx, true, &fMissingInputs, false, ignoreFees)) { + if (!tx.HasZerocoinSpendInputs() && AcceptToMemoryPool(mempool, state, ptx, true, &fMissingInputs, false, ignoreFees)) { mempool.check(pcoinsTip); RelayTransaction(tx, connman); vWorkQueue.push_back(inv.hash); @@ -1435,7 +1436,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR mi != itByPrev->second.end(); ++mi) { const uint256 &orphanHash = *mi; - const CTransaction &orphanTx = mapOrphanTransactions[orphanHash].tx; + const auto &orphanTx = mapOrphanTransactions[orphanHash].tx; NodeId fromPeer = mapOrphanTransactions[orphanHash].fromPeer; bool fMissingInputs2 = false; // Use a dummy CValidationState so someone can't setup nodes to counter-DoS based on orphan @@ -1448,7 +1449,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR continue; if(AcceptToMemoryPool(mempool, stateDummy, orphanTx, true, &fMissingInputs2)) { LogPrint(BCLog::MEMPOOL, " accepted orphan tx %s\n", orphanHash.ToString()); - RelayTransaction(orphanTx, connman); + RelayTransaction(*orphanTx, connman); vWorkQueue.push_back(orphanHash); vEraseQueue.push_back(orphanHash); } else if(!fMissingInputs2) { @@ -1472,7 +1473,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR for (uint256& hash : vEraseQueue) EraseOrphanTx(hash); - } else if (tx.HasZerocoinSpendInputs() && AcceptToMemoryPool(mempool, state, tx, true, &fMissingZerocoinInputs, false, false, ignoreFees)) { + } else if (tx.HasZerocoinSpendInputs() && AcceptToMemoryPool(mempool, state, ptx, true, &fMissingZerocoinInputs, false, false, ignoreFees)) { //Presstab: ZCoin has a bunch of code commented out here. Is this something that should have more going on? //Also there is nothing that handles fMissingZerocoinInputs. Does there need to be? RelayTransaction(tx, connman); @@ -1481,7 +1482,7 @@ bool static ProcessMessage(CNode* pfrom, std::string strCommand, CDataStream& vR tx.GetHash().ToString(), mempool.mapTx.size()); } else if (fMissingInputs) { - AddOrphanTx(tx, pfrom->GetId()); + AddOrphanTx(ptx, pfrom->GetId()); // DoS prevention: do not allow mapOrphanTransactions to grow unbounded unsigned int nMaxOrphanTx = (unsigned int)std::max((int64_t)0, GetArg("-maxorphantx", DEFAULT_MAX_ORPHAN_TRANSACTIONS)); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 23d72283a55aa..5fae8c834bdd5 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -863,7 +863,7 @@ UniValue sendrawtransaction(const JSONRPCRequest& request) } CValidationState state; bool fMissingInputs; - if (!AcceptToMemoryPool(mempool, state, tx, false, &fMissingInputs, false, !fOverrideFees)) { + if (!AcceptToMemoryPool(mempool, state, MakeTransactionRef(std::move(tx)), false, &fMissingInputs, false, !fOverrideFees)) { if (state.IsInvalid()) { throw JSONRPCError(RPC_TRANSACTION_REJECTED, strprintf("%i: %s", state.GetRejectCode(), state.GetRejectReason())); } else { diff --git a/src/swifttx.cpp b/src/swifttx.cpp index e32524108de18..b6561967fed3d 100644 --- a/src/swifttx.cpp +++ b/src/swifttx.cpp @@ -44,8 +44,9 @@ void ProcessMessageSwiftTX(CNode* pfrom, std::string& strCommand, CDataStream& v if (strCommand == NetMsgType::IX) { //LogPrintf("ProcessMessageSwiftTX::ix\n"); CDataStream vMsg(vRecv); - CTransaction tx; - vRecv >> tx; + CTransactionRef ptx; + vRecv >> ptx; + const CTransaction& tx = *ptx; CInv inv(MSG_TXLOCK_REQUEST, tx.GetHash()); pfrom->AddInventoryKnown(inv); @@ -75,7 +76,7 @@ void ProcessMessageSwiftTX(CNode* pfrom, std::string& strCommand, CDataStream& v bool fAccepted = false; { LOCK(cs_main); - fAccepted = AcceptToMemoryPool(mempool, state, tx, true, &fMissingInputs); + fAccepted = AcceptToMemoryPool(mempool, state, ptx, true, &fMissingInputs); } if (fAccepted) { g_connman->RelayInv(inv); @@ -260,7 +261,7 @@ int64_t CreateNewLock(CTransaction tx) } // check if we need to vote on this transaction -void DoConsensusVote(CTransaction& tx, int64_t nBlockHeight) +void DoConsensusVote(const CTransaction& tx, int64_t nBlockHeight) { if (!fMasterNode) return; @@ -400,7 +401,7 @@ bool ProcessConsensusVote(CNode* pnode, CConsensusVote& ctx) return false; } -bool CheckForConflictingLocks(CTransaction& tx) +bool CheckForConflictingLocks(const CTransaction& tx) { /* It's possible (very unlikely though) to get 2 conflicting transaction locks approved by the network. diff --git a/src/swifttx.h b/src/swifttx.h index ffaabe9f9dfa3..d9ff34cec5670 100644 --- a/src/swifttx.h +++ b/src/swifttx.h @@ -45,12 +45,12 @@ int64_t CreateNewLock(CTransaction tx); bool IsIXTXValid(const CTransaction& txCollateral); // if two conflicting locks are approved by the network, they will cancel out -bool CheckForConflictingLocks(CTransaction& tx); +bool CheckForConflictingLocks(const CTransaction& tx); void ProcessMessageSwiftTX(CNode* pfrom, std::string& strCommand, CDataStream& vRecv); //check if we need to vote on this transaction -void DoConsensusVote(CTransaction& tx, int64_t nBlockHeight); +void DoConsensusVote(const CTransaction& tx, int64_t nBlockHeight); //process consensus vote message bool ProcessConsensusVote(CNode* pnode, CConsensusVote& ctx); diff --git a/src/test/DoS_tests.cpp b/src/test/DoS_tests.cpp index 3f5e933cd949e..9863008d7a9ce 100644 --- a/src/test/DoS_tests.cpp +++ b/src/test/DoS_tests.cpp @@ -25,11 +25,11 @@ #include // Tests this internal-to-validation.cpp method: -extern bool AddOrphanTx(const CTransaction& tx, NodeId peer); +extern bool AddOrphanTx(const CTransactionRef& tx, NodeId peer); extern void EraseOrphansFor(NodeId peer); extern unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans); struct COrphanTx { - CTransaction tx; + CTransactionRef tx; NodeId fromPeer; }; extern std::map mapOrphanTransactions; @@ -132,7 +132,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime) BOOST_CHECK(!connman->IsBanned(addr)); } -CTransaction RandomOrphan() +CTransactionRef RandomOrphan() { std::map::iterator it; it = mapOrphanTransactions.lower_bound(InsecureRand256()); @@ -160,30 +160,30 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans) tx.vout[0].nValue = 1*CENT; tx.vout[0].scriptPubKey = GetScriptForDestination(key.GetPubKey().GetID()); - AddOrphanTx(tx, i); + AddOrphanTx(MakeTransactionRef(tx), i); } // ... and 50 that depend on other orphans: for (int i = 0; i < 50; i++) { - CTransaction txPrev = RandomOrphan(); + CTransactionRef txPrev = RandomOrphan(); CMutableTransaction tx; tx.vin.resize(1); tx.vin[0].prevout.n = 0; - tx.vin[0].prevout.hash = txPrev.GetHash(); + tx.vin[0].prevout.hash = txPrev->GetHash(); tx.vout.resize(1); tx.vout[0].nValue = 1*CENT; tx.vout[0].scriptPubKey = GetScriptForDestination(key.GetPubKey().GetID()); - SignSignature(keystore, txPrev, tx, 0, SIGHASH_ALL); + SignSignature(keystore, *txPrev, tx, 0, SIGHASH_ALL); - AddOrphanTx(tx, i); + AddOrphanTx(MakeTransactionRef(tx), i); } // This really-big orphan should be ignored: for (int i = 0; i < 10; i++) { - CTransaction txPrev = RandomOrphan(); + CTransactionRef txPrev = RandomOrphan(); CMutableTransaction tx; tx.vout.resize(1); @@ -193,15 +193,15 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans) for (unsigned int j = 0; j < tx.vin.size(); j++) { tx.vin[j].prevout.n = j; - tx.vin[j].prevout.hash = txPrev.GetHash(); + tx.vin[j].prevout.hash = txPrev->GetHash(); } - SignSignature(keystore, txPrev, tx, 0, SIGHASH_ALL); + SignSignature(keystore, *txPrev, tx, 0, SIGHASH_ALL); // Re-use same signature for other inputs // (they don't have to be valid for this test) for (unsigned int j = 1; j < tx.vin.size(); j++) tx.vin[j].scriptSig = tx.vin[0].scriptSig; - BOOST_CHECK(!AddOrphanTx(tx, i)); + BOOST_CHECK(!AddOrphanTx(MakeTransactionRef(tx), i)); } // Test EraseOrphansFor: diff --git a/src/test/test_pivx.cpp b/src/test/test_pivx.cpp index af46f6a1e16f5..e0a7a4b3aa6f2 100644 --- a/src/test/test_pivx.cpp +++ b/src/test/test_pivx.cpp @@ -78,10 +78,10 @@ TestingSetup::~TestingSetup() } CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(CMutableTransaction &tx, CTxMemPool *pool) { - CTransaction txn(tx); + const CTransactionRef txn = MakeTransactionRef(tx); // todo: move to the caller side.. bool hasNoDependencies = pool ? pool->HasNoInputsOf(tx) : hadNoDependencies; // Hack to assume either its completely dependent on other mempool txs or not at all - CAmount inChainValue = hasNoDependencies ? txn.GetValueOut() : 0; + CAmount inChainValue = hasNoDependencies ? txn->GetValueOut() : 0; return CTxMemPoolEntry(txn, nFee, nTime, dPriority, nHeight, hasNoDependencies, inChainValue, spendsCoinbaseOrCoinstake, sigOpCount); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 7f9d6f6f605f3..0cda0fbd22dad 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -19,22 +19,22 @@ #include -CTxMemPoolEntry::CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, +CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee, int64_t _nTime, double _entryPriority, unsigned int _entryHeight, bool poolHasNoInputsOf, CAmount _inChainInputValue, bool _spendsCoinbaseOrCoinstake, unsigned int _sigOps) : tx(MakeTransactionRef(_tx)), nFee(_nFee), nTime(_nTime), entryPriority(_entryPriority), entryHeight(_entryHeight), hadNoDependencies(poolHasNoInputsOf), inChainInputValue(_inChainInputValue), spendsCoinbaseOrCoinstake(_spendsCoinbaseOrCoinstake), sigOpCount(_sigOps) { - nTxSize = ::GetSerializeSize(_tx, SER_NETWORK, PROTOCOL_VERSION); - nModSize = _tx.CalculateModifiedSize(nTxSize); - nUsageSize = _tx.DynamicMemoryUsage(); - hasZerocoins = _tx.ContainsZerocoins(); - m_isShielded = tx->hasSaplingData(); + nTxSize = ::GetSerializeSize(*_tx, SER_NETWORK, PROTOCOL_VERSION); + nModSize = _tx->CalculateModifiedSize(nTxSize); + nUsageSize = _tx->DynamicMemoryUsage(); + hasZerocoins = _tx->ContainsZerocoins(); + m_isShielded = _tx->hasSaplingData(); nCountWithDescendants = 1; nSizeWithDescendants = nTxSize; nFeesWithDescendants = nFee; - CAmount nValueIn = _tx.GetValueOut()+nFee; + CAmount nValueIn = _tx->GetValueOut()+nFee; assert(inChainInputValue <= nValueIn); feeDelta = 0; diff --git a/src/txmempool.h b/src/txmempool.h index 6f7f7cfdaabd3..79f4b4d389e2c 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -87,7 +87,7 @@ class CTxMemPoolEntry CAmount nFeesWithDescendants; //! ... and total fees (all including us) public: - CTxMemPoolEntry(const CTransaction& _tx, const CAmount& _nFee, + CTxMemPoolEntry(const CTransactionRef& _tx, const CAmount& _nFee, int64_t _nTime, double _entryPriority, unsigned int _entryHeight, bool poolHasNoInputsOf, CAmount _inChainInputValue, bool _spendsCoinbaseOrCoinstake, unsigned int nSigOps); diff --git a/src/validation.cpp b/src/validation.cpp index 541db0b899b75..c3ea1f3db5826 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -303,11 +303,12 @@ std::string FormatStateMessage(const CValidationState &state) state.GetRejectCode()); } -bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const CTransaction &tx, bool fLimitFree, +bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const CTransactionRef& _tx, bool fLimitFree, bool* pfMissingInputs, bool fOverrideMempoolLimit, bool fRejectAbsurdFee, bool ignoreFees, std::vector& coins_to_uncache) { AssertLockHeld(cs_main); + const CTransaction& tx = *_tx; if (pfMissingInputs) *pfMissingInputs = false; @@ -469,7 +470,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C } } - CTxMemPoolEntry entry(tx, nFees, GetTime(), dPriority, chainHeight, pool.HasNoInputsOf(tx), inChainInputValue, fSpendsCoinbaseOrCoinstake, nSigOps); + CTxMemPoolEntry entry(_tx, nFees, GetTime(), dPriority, chainHeight, pool.HasNoInputsOf(tx), inChainInputValue, fSpendsCoinbaseOrCoinstake, nSigOps); unsigned int nSize = entry.GetTxSize(); // Don't accept it if it can't get into a block @@ -581,7 +582,7 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C return true; } -bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransaction &tx, bool fLimitFree, +bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransactionRef& tx, bool fLimitFree, bool* pfMissingInputs, bool fOverrideMempoolLimit, bool fRejectAbsurdFee, bool fIgnoreFees) { std::vector coins_to_uncache; @@ -1889,15 +1890,14 @@ bool static DisconnectTip(CValidationState& state) return false; // Resurrect mempool transactions from the disconnected block. std::vector vHashUpdate; - for (const auto& txIn : block.vtx) { - const CTransaction& tx = *txIn; + for (const auto& tx : block.vtx) { // ignore validation errors in resurrected transactions std::list removed; CValidationState stateDummy; - if (tx.IsCoinBase() || tx.IsCoinStake() || !AcceptToMemoryPool(mempool, stateDummy, tx, false, nullptr, true)) { - mempool.remove(tx, removed, true); - } else if (mempool.exists(tx.GetHash())) { - vHashUpdate.push_back(tx.GetHash()); + if (tx->IsCoinBase() || tx->IsCoinStake() || !AcceptToMemoryPool(mempool, stateDummy, tx, false, nullptr, true)) { + mempool.remove(*tx, removed, true); + } else if (mempool.exists(tx->GetHash())) { + vHashUpdate.push_back(tx->GetHash()); } } // AcceptToMemoryPool/addUnchecked all assume that new mempool entries have diff --git a/src/validation.h b/src/validation.h index f055aff2df0c2..49efa892d1973 100644 --- a/src/validation.h +++ b/src/validation.h @@ -217,7 +217,7 @@ void FlushStateToDisk(); /** (try to) add transaction to memory pool **/ -bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransaction& tx, bool fLimitFree, bool* pfMissingInputs, bool fOverrideMempoolLimit = false, bool fRejectInsaneFee = false, bool ignoreFees = false); +bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransactionRef& tx, bool fLimitFree, bool* pfMissingInputs, bool fOverrideMempoolLimit = false, bool fRejectInsaneFee = false, bool ignoreFees = false); int GetIXConfirmations(uint256 nTXHash); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index fd0fde8878ee6..34838a18dc86d 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -337,7 +337,7 @@ CBlockIndex* SimpleFakeMine(CWalletTx& wtx, CBlockIndex* pprev = nullptr) return fakeIndex; } -void fakeMempoolInsertion(const CWalletTx& wtxCredit) +void fakeMempoolInsertion(const CTransactionRef& wtxCredit) { CTxMemPoolEntry entry(wtxCredit, 0, 0, 0, 0, false, 0, false, 0); LOCK(mempool.cs); @@ -435,7 +435,7 @@ BOOST_AUTO_TEST_CASE(cached_balances_tests) ); // GetUnconfirmedBalance requires tx in mempool. - fakeMempoolInsertion(wtxCredit); + fakeMempoolInsertion(MakeTransactionRef(wtxCredit)); BOOST_CHECK_EQUAL(wallet.GetUnconfirmedBalance(), nCredit); // 2) Confirm tx and verify diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index a3900cbc0586e..fa23c24532e04 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2870,7 +2870,7 @@ CWallet::CommitResult CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& // Try ATMP. This must not fail. The transaction has already been signed and recorded. CValidationState state; - if (!AcceptToMemoryPool(mempool, state, wtxNew, false, nullptr, false, true, false)) { + if (!AcceptToMemoryPool(mempool, state, MakeTransactionRef(std::move(wtxNew)), false, nullptr, false, true, false)) { res.state = state; // Abandon the transaction if (AbandonTransaction(res.hashTx)) { @@ -4035,7 +4035,7 @@ bool CMerkleTx::IsInMainChainImmature() const bool CMerkleTx::AcceptToMemoryPool(bool fLimitFree, bool fRejectInsaneFee, bool ignoreFees) { CValidationState state; - bool fAccepted = ::AcceptToMemoryPool(mempool, state, *this, fLimitFree, nullptr, false, fRejectInsaneFee, ignoreFees); + bool fAccepted = ::AcceptToMemoryPool(mempool, state, MakeTransactionRef(*this), fLimitFree, nullptr, false, fRejectInsaneFee, ignoreFees); if (!fAccepted) LogPrintf("%s : %s\n", __func__, state.GetRejectReason()); return fAccepted;