diff --git a/src/consensus/validation.h b/src/consensus/validation.h index 47447c453ba12..01cfe37d1ba62 100644 --- a/src/consensus/validation.h +++ b/src/consensus/validation.h @@ -30,11 +30,15 @@ enum class TxValidationResult { TX_MISSING_INPUTS, //!< transaction was missing some of its inputs TX_PREMATURE_SPEND, //!< transaction spends a coinbase too early, or violates locktime/sequence locks /** - * Transaction might be missing a witness, have a witness prior to SegWit + * Transaction might have a witness prior to SegWit * activation, or witness may have been malleated (which includes * non-standard witnesses). */ TX_WITNESS_MUTATED, + /** + * Transaction is missing a witness. + */ + TX_WITNESS_STRIPPED, /** * Tx already in mempool or conflicts with a tx in the chain * (if it conflicts with another tx in mempool, we use MEMPOOL_POLICY as it failed to reach the RBF threshold) diff --git a/src/net.h b/src/net.h index 51387e96fafd7..6c50ae936784d 100644 --- a/src/net.h +++ b/src/net.h @@ -1115,11 +1115,11 @@ class CNode } - void AddInventoryKnown(const CInv& inv) + void AddKnownTx(const uint256& hash) { if (m_tx_relay != nullptr) { LOCK(m_tx_relay->cs_tx_inventory); - m_tx_relay->filterInventoryKnown.insert(inv.hash); + m_tx_relay->filterInventoryKnown.insert(hash); } } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b8ac5dfe99d6e..85e68054e4757 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -88,6 +88,8 @@ static const unsigned int MAX_LOCATOR_SZ = 101; static constexpr int32_t MAX_PEER_TX_IN_FLIGHT = 100; /** Maximum number of announced transactions from a peer */ static constexpr int32_t MAX_PEER_TX_ANNOUNCEMENTS = 2 * MAX_INV_SZ; +/** How many microseconds to delay requesting transactions via txids, if we have wtxid-relaying peers */ +static constexpr std::chrono::microseconds TXID_RELAY_DELAY{std::chrono::seconds{2}}; /** How many microseconds to delay requesting transactions from inbound peers */ static constexpr std::chrono::microseconds INBOUND_PEER_TX_DELAY{std::chrono::seconds{2}}; /** How long to wait (in microseconds) before downloading a transaction from an additional peer */ @@ -176,6 +178,7 @@ struct COrphanTx { }; RecursiveMutex g_cs_orphans; std::map mapOrphanTransactions GUARDED_BY(g_cs_orphans); +std::map::iterator> g_orphans_by_wtxid GUARDED_BY(g_cs_orphans); void EraseOrphansFor(NodeId peer); @@ -212,6 +215,15 @@ namespace { * million to make it highly unlikely for users to have issues with this * filter. * + * We only need to add wtxids to this filter. For non-segwit + * transactions, the txid == wtxid, so this only prevents us from + * re-downloading non-segwit transactions when communicating with + * non-wtxidrelay peers -- which is important for avoiding malleation + * attacks that could otherwise interfere with transaction relay from + * non-wtxidrelay peers. For communicating with wtxidrelay peers, having + * the reject filter store wtxids is exactly what we want to avoid + * redownload of a rejected transaction. + * * Memory used: 1.3 MB */ std::unique_ptr recentRejects GUARDED_BY(cs_main); @@ -243,6 +255,9 @@ namespace { /** Number of peers from which we're downloading blocks. */ int nPeersWithValidatedDownloads GUARDED_BY(cs_main) = 0; + /** Number of peers with wtxid relay. */ + int g_wtxid_relay_peers GUARDED_BY(cs_main) = 0; + /** Number of outbound peers with m_chain_sync.m_protect. */ int g_outbound_peers_with_protect_from_disconnect GUARDED_BY(cs_main) = 0; @@ -435,6 +450,9 @@ struct CNodeState { //! A rolling bloom filter of all announced tx CInvs to this peer. CRollingBloomFilter m_recently_announced_invs = CRollingBloomFilter{INVENTORY_MAX_RECENT_RELAY, 0.000001}; + //! Whether this peer relays txs via wtxid + bool m_wtxid_relay{false}; + CNodeState(CAddress addrIn, std::string addrNameIn, bool is_inbound, bool is_manual) : address(addrIn), name(std::move(addrNameIn)), m_is_inbound(is_inbound), m_is_manual_connection (is_manual) @@ -813,12 +831,12 @@ std::chrono::microseconds GetObjectExpiryInterval(int invType) } std::chrono::microseconds GetObjectRandomDelay(int invType) { - if (invType == MSG_TX) { + if (invType == MSG_TX || invType == MSG_WTX) { return GetRandMicros(MAX_GETDATA_RANDOM_DELAY); } return {}; } -std::chrono::microseconds CalculateTxGetDataTime(const CInv& inv, std::chrono::microseconds current_time, bool use_inbound_delay) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +std::chrono::microseconds CalculateTxGetDataTime(const CInv& inv, std::chrono::microseconds current_time, bool use_inbound_delay, bool use_txid_delay) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { std::chrono::microseconds process_time; const auto last_request_time = GetTxRequestTime(inv.hash); @@ -833,7 +851,10 @@ std::chrono::microseconds CalculateTxGetDataTime(const CInv& inv, std::chrono::m // We delay processing announcements from inbound peers // SYSCOIN - if (inv.type == MSG_TX && !fMasternodeMode && use_inbound_delay) process_time += INBOUND_PEER_TX_DELAY; + if ((inv.type == MSG_TX || inv.type == MSG_WTX) && !fMasternodeMode && use_inbound_delay) process_time += INBOUND_PEER_TX_DELAY; + + // We delay processing announcements from peers that use txid-relay (instead of wtxid) + if (use_txid_delay) process_time += TXID_RELAY_DELAY; return process_time; } @@ -852,7 +873,7 @@ void RequestTx(CNodeState* state, const CInv& inv, std::chrono::microseconds cur // Calculate the time to try requesting this transaction. Use // fPreferredDownload as a proxy for outbound peers. - const auto process_time = CalculateTxGetDataTime(inv, current_time, !state->fPreferredDownload); + const auto process_time = CalculateTxGetDataTime(inv, current_time, !state->fPreferredDownload, !state->m_wtxid_relay && g_wtxid_relay_peers > 0); peer_download_state.m_tx_process_time.emplace(process_time, inv); } @@ -914,14 +935,15 @@ void PeerLogicValidation::InitializeNode(CNode *pnode) { void PeerLogicValidation::ReattemptInitialBroadcast(CScheduler& scheduler) const { - std::set unbroadcast_txids = m_mempool.GetUnbroadcastTxs(); + std::map unbroadcast_txids = m_mempool.GetUnbroadcastTxs(); - for (const uint256& txid : unbroadcast_txids) { + for (const auto& elem : unbroadcast_txids) { // Sanity check: all unbroadcast txns should exist in the mempool - if (m_mempool.exists(txid)) { - RelayTransaction(txid, *connman); + if (m_mempool.exists(elem.first)) { + LOCK(cs_main); + RelayTransaction(elem.first, elem.second, *connman); } else { - m_mempool.RemoveUnbroadcastTx(txid, true); + m_mempool.RemoveUnbroadcastTx(elem.first, true); } } @@ -953,6 +975,8 @@ void PeerLogicValidation::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTim assert(nPeersWithValidatedDownloads >= 0); g_outbound_peers_with_protect_from_disconnect -= state->m_chain_sync.m_protect; assert(g_outbound_peers_with_protect_from_disconnect >= 0); + g_wtxid_relay_peers -= state->m_wtxid_relay; + assert(g_wtxid_relay_peers >= 0); mapNodeState.erase(nodeid); @@ -962,6 +986,7 @@ void PeerLogicValidation::FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTim assert(nPreferredDownload == 0); assert(nPeersWithValidatedDownloads == 0); assert(g_outbound_peers_with_protect_from_disconnect == 0); + assert(g_wtxid_relay_peers == 0); } LogPrint(BCLog::NET, "Cleared nodestate for peer=%d\n", nodeid); } @@ -1028,6 +1053,8 @@ bool AddOrphanTx(const CTransactionRef& tx, NodeId peer) EXCLUSIVE_LOCKS_REQUIRE auto ret = mapOrphanTransactions.emplace(hash, COrphanTx{tx, peer, GetTime() + ORPHAN_TX_EXPIRE_TIME, g_orphan_list.size()}); assert(ret.second); g_orphan_list.push_back(ret.first); + // Allow for lookups in the orphan pool by wtxid, as well as txid + g_orphans_by_wtxid.emplace(tx->GetWitnessHash(), ret.first); for (const CTxIn& txin : tx->vin) { mapOrphanTransactionsByPrev[txin.prevout].insert(ret.first); } @@ -1064,6 +1091,7 @@ int static EraseOrphanTx(uint256 hash) EXCLUSIVE_LOCKS_REQUIRED(g_cs_orphans) it_last->second.list_pos = old_pos; } g_orphan_list.pop_back(); + g_orphans_by_wtxid.erase(it->second.tx->GetWitnessHash()); mapOrphanTransactions.erase(it); return 1; @@ -1233,6 +1261,7 @@ static bool MaybePunishNodeForTx(NodeId nodeid, const TxValidationState& state, case TxValidationResult::TX_MISSING_INPUTS: case TxValidationResult::TX_PREMATURE_SPEND: case TxValidationResult::TX_WITNESS_MUTATED: + case TxValidationResult::TX_WITNESS_STRIPPED: case TxValidationResult::TX_CONFLICT: case TxValidationResult::TX_MEMPOOL_POLICY: break; @@ -1273,14 +1302,15 @@ PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn, BanMan* banman, CS recentRejects.reset(new CRollingBloomFilter(120000, 0.000001)); // Blocks don't typically have more than 4000 transactions, so this should - // be at least six blocks (~1 hr) worth of transactions that we can store. + // be at least six blocks (~1 hr) worth of transactions that we can store, + // inserting both a txid and wtxid for every observed transaction. // If the number of transactions appearing in a block goes up, or if we are // seeing getdata requests more than an hour after initial announcement, we // can increase this number. // The false positive rate of 1/1M should come out to less than 1 // transaction per day that would be inadvertently ignored (which is the // same probability that we have in the reject filter). - g_recent_confirmed_transactions.reset(new CRollingBloomFilter(24000, 0.000001)); + g_recent_confirmed_transactions.reset(new CRollingBloomFilter(48000, 0.000001)); const Consensus::Params& consensusParams = Params().GetConsensus(); // Stale tip checking and peer eviction are on two different timers, but we @@ -1336,6 +1366,9 @@ void PeerLogicValidation::BlockConnected(const std::shared_ptr& pb LOCK(g_cs_recent_confirmed_transactions); for (const auto& ptx : pblock->vtx) { g_recent_confirmed_transactions->insert(ptx->GetHash()); + if (ptx->GetHash() != ptx->GetWitnessHash()) { + g_recent_confirmed_transactions->insert(ptx->GetWitnessHash()); + } } } } @@ -1491,6 +1524,7 @@ bool static AlreadyHave(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_LO { case MSG_TX: case MSG_WITNESS_TX: + case MSG_WTX: { assert(recentRejects); if (::ChainActive().Tip()->GetBlockHash() != hashRecentRejectsChainTip) @@ -1505,7 +1539,11 @@ bool static AlreadyHave(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_LO { LOCK(g_cs_orphans); - if (mapOrphanTransactions.count(inv.hash)) return true; + if (inv.type != MSG_WTX && mapOrphanTransactions.count(inv.hash)) { + return true; + } else if (inv.type == MSG_WTX && g_orphans_by_wtxid.count(inv.hash)) { + return true; + } } { @@ -1513,8 +1551,8 @@ bool static AlreadyHave(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_LO if (g_recent_confirmed_transactions->contains(inv.hash)) return true; } - return recentRejects->contains(inv.hash) || - mempool.exists(inv.hash); + const bool by_wtxid = (inv.type == MSG_WTX); + return recentRejects->contains(inv.hash) || mempool.exists(inv.hash, by_wtxid); } case MSG_BLOCK: case MSG_WITNESS_BLOCK: @@ -1543,13 +1581,21 @@ bool static AlreadyHave(const CInv& inv, const CTxMemPool& mempool) EXCLUSIVE_LO return true; } -void RelayTransaction(const uint256& txid, const CConnman& connman) +void RelayTransaction(const uint256& txid, const uint256& wtxid, const CConnman& connman) { - connman.ForEachNode([&txid](CNode* pnode) + connman.ForEachNode([&txid, &wtxid](CNode* pnode) { // SYSCOIN only relay to outbound masternodes for efficiency if (!pnode->fMasternode) - pnode->PushTxInventory(txid); + { + AssertLockHeld(cs_main); + CNodeState &state = *State(pnode->GetId()); + if (state.m_wtxid_relay) { + pnode->PushTxInventory(wtxid); + } else { + pnode->PushTxInventory(txid); + } + } }); } @@ -1747,9 +1793,9 @@ void static ProcessGetBlockData(CNode& pfrom, const CChainParams& chainparams, c } //! Determine whether or not a peer can request a transaction, and return it (or nullptr if not found or not allowed). -CTransactionRef static FindTxForGetData(const CNode& peer, const uint256& txid, const std::chrono::seconds mempool_req, const std::chrono::seconds now) LOCKS_EXCLUDED(cs_main) +CTransactionRef static FindTxForGetData(const CNode& peer, const uint256& txid_or_wtxid, bool use_wtxid, const std::chrono::seconds mempool_req, const std::chrono::seconds now) LOCKS_EXCLUDED(cs_main) { - auto txinfo = mempool.info(txid); + auto txinfo = mempool.info(txid_or_wtxid, use_wtxid); if (txinfo.tx) { // If a TX could have been INVed in reply to a MEMPOOL request, // or is older than UNCONDITIONAL_RELAY_DELAY, permit the request @@ -1761,13 +1807,12 @@ CTransactionRef static FindTxForGetData(const CNode& peer, const uint256& txid, { LOCK(cs_main); - // Otherwise, the transaction must have been announced recently. - if (State(peer.GetId())->m_recently_announced_invs.contains(txid)) { + if (State(peer.GetId())->m_recently_announced_invs.contains(txid_or_wtxid)) { // If it was, it can be relayed from either the mempool... if (txinfo.tx) return std::move(txinfo.tx); // ... or the relay pool. - auto mi = mapRelay.find(txid); + auto mi = mapRelay.find(txid_or_wtxid); if (mi != mapRelay.end()) return mi->second; } } @@ -1792,7 +1837,7 @@ void static ProcessGetData(CNode& pfrom, const CChainParams& chainparams, CConnm // possible, since they're common and it's efficient to batch process // them. // SYSCOIN - while (it != pfrom.vRecvGetData.end() && (it->type == MSG_TX || it->type == MSG_WITNESS_TX || it->IsMnType())) { + while (it != pfrom.vRecvGetData.end() && (it->type == MSG_TX || it->type == MSG_WITNESS_TX || it->type == MSG_WTX || it->IsMnType())) { if (interruptMsgProc) return; // The send buffer provides backpressure. If there's no space in // the buffer, pause processing until the next call. @@ -1800,12 +1845,12 @@ void static ProcessGetData(CNode& pfrom, const CChainParams& chainparams, CConnm const CInv &inv = *it++; // SYSCOIN - if(inv.type == MSG_TX || inv.type == MSG_WITNESS_TX) { + if(inv.type == MSG_TX || inv.type == MSG_WITNESS_TX || inv.type == MSG_WTX) { if (pfrom.m_tx_relay == nullptr) { // Ignore GETDATA requests for transactions from blocks-only peers. continue; } - CTransactionRef tx = FindTxForGetData(pfrom, inv.hash, mempool_req, now); + CTransactionRef tx = FindTxForGetData(pfrom, inv.hash, inv.type == MSG_WTX, mempool_req, now); if (tx) { int nSendFlags = (inv.type == MSG_TX ? SERIALIZE_TRANSACTION_NO_WITNESS : 0); connman.PushMessage(&pfrom, msgMaker.Make(nSendFlags, NetMsgType::TX, *tx)); @@ -2207,7 +2252,7 @@ void static ProcessOrphanTx(CConnman& connman, CTxMemPool& mempool, std::setGetWitnessHash(), connman); for (unsigned int i = 0; i < orphanTx.vout.size(); i++) { auto it_by_prev = mapOrphanTransactionsByPrev.find(COutPoint(orphanHash, i)); if (it_by_prev != mapOrphanTransactionsByPrev.end()) { @@ -2232,12 +2277,22 @@ void static ProcessOrphanTx(CConnman& connman, CTxMemPool& mempool, std::setinsert(orphanHash); + recentRejects->insert(orphanTx.GetWitnessHash()); } EraseOrphanTx(orphanHash); done = true; @@ -2574,6 +2629,10 @@ void ProcessMessage( if (pfrom.fInbound) PushNodeVersion(pfrom, connman, GetAdjustedTime()); + if (nVersion >= WTXID_RELAY_VERSION) { + connman.PushMessage(&pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::WTXIDRELAY)); + } + connman.PushMessage(&pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERACK)); pfrom.nServices = nServices; @@ -2726,6 +2785,25 @@ void ProcessMessage( return; } + // Feature negotiation of wtxidrelay should happen between VERSION and + // VERACK, to avoid relay problems from switching after a connection is up + if (msg_type == NetMsgType::WTXIDRELAY) { + if (pfrom.fSuccessfullyConnected) { + // Disconnect peers that send wtxidrelay message after VERACK; this + // must be negotiated between VERSION and VERACK. + pfrom.fDisconnect = true; + return; + } + if (pfrom.nVersion >= WTXID_RELAY_VERSION) { + LOCK(cs_main); + if (!State(pfrom.GetId())->m_wtxid_relay) { + State(pfrom.GetId())->m_wtxid_relay = true; + g_wtxid_relay_peers++; + } + } + return; + } + if (!pfrom.fSuccessfullyConnected) { // Must have a verack message before anything else LOCK(cs_main); @@ -2865,6 +2943,13 @@ void ProcessMessage( if (interruptMsgProc) return; + // ignore INVs that don't match wtxidrelay setting + if (State(pfrom.GetId())->m_wtxid_relay) { + if (inv.type == MSG_TX) continue; + } else { + if (inv.type == MSG_WTX) continue; + } + bool fAlreadyHave = AlreadyHave(inv, mempool); LogPrint(BCLog::NET, "got inv: %s %s peer=%d\n", inv.ToString(), fAlreadyHave ? "have" : "new", pfrom.GetId()); @@ -2883,7 +2968,7 @@ void ProcessMessage( best_block = &inv.hash; } } else { - pfrom.AddInventoryKnown(inv); + pfrom.AddKnownTx(inv.hash); // SYSCOIN static std::set allowWhileInIBDObjs = { MSG_SPORK @@ -3151,23 +3236,48 @@ void ProcessMessage( vRecv >> ptx; const CTransaction& tx = *ptx; - CInv inv(MSG_TX, tx.GetHash()); - pfrom.AddInventoryKnown(inv); + const uint256& txid = ptx->GetHash(); + const uint256& wtxid = ptx->GetWitnessHash(); LOCK2(cs_main, g_cs_orphans); + CNodeState* nodestate = State(pfrom.GetId()); + + const uint256& hash = nodestate->m_wtxid_relay ? wtxid : txid; + pfrom.AddKnownTx(hash); + if (nodestate->m_wtxid_relay && txid != wtxid) { + // Insert txid into filterInventoryKnown, even for + // wtxidrelay peers. This prevents re-adding of + // unconfirmed parents to the recently_announced + // filter, when a child tx is requested. See + // ProcessGetData(). + pfrom.AddKnownTx(txid); + } + TxValidationState state; // SYSCOIN use inv not hash EraseTxRequest(pfrom.GetId(), inv); std::list lRemovedTxn; - if (!AlreadyHave(inv, mempool) && + // We do the AlreadyHave() check using wtxid, rather than txid - in the + // absence of witness malleation, this is strictly better, because the + // recent rejects filter may contain the wtxid but will never contain + // the txid of a segwit transaction that has been rejected. + // In the presence of witness malleation, it's possible that by only + // doing the check with wtxid, we could overlook a transaction which + // was confirmed with a different witness, or exists in our mempool + // with a different witness, but this has limited downside: + // mempool validation does its own lookup of whether we have the txid + // already; and an adversary can already relay us old transactions + // (older than our recency filter) if trying to DoS us, without any need + // for witness malleation. + if (!AlreadyHave(CInv(MSG_WTX, wtxid), mempool) && AcceptToMemoryPool(mempool, state, ptx, &lRemovedTxn, false /* bypass_limits */, 0 /* nAbsurdFee */)) { mempool.check(&::ChainstateActive().CoinsTip()); - RelayTransaction(tx.GetHash(), connman); + RelayTransaction(tx.GetHash(), tx.GetWitnessHash(), connman); for (unsigned int i = 0; i < tx.vout.size(); i++) { - auto it_by_prev = mapOrphanTransactionsByPrev.find(COutPoint(inv.hash, i)); + auto it_by_prev = mapOrphanTransactionsByPrev.find(COutPoint(txid, i)); if (it_by_prev != mapOrphanTransactionsByPrev.end()) { for (const auto& elem : it_by_prev->second) { pfrom.orphan_work_set.insert(elem->first); @@ -3199,8 +3309,13 @@ void ProcessMessage( const auto current_time = GetTime(); for (const CTxIn& txin : tx.vin) { + // Here, we only have the txid (and not wtxid) of the + // inputs, so we only request parents from + // non-wtxid-relay peers. + // Eventually we should replace this with an improved + // protocol for getting all unconfirmed parents. CInv _inv(MSG_TX | nFetchFlags, txin.prevout.hash); - pfrom.AddInventoryKnown(_inv); + pfrom.AddKnownTx(txin.prevout.hash); // SYSCOIN if (!AlreadyHave(_inv, mempool)) RequestTx(State(pfrom.GetId()), _inv, current_time); } @@ -3216,15 +3331,30 @@ void ProcessMessage( LogPrint(BCLog::MEMPOOL, "not keeping orphan with rejected parents %s\n",tx.GetHash().ToString()); // We will continue to reject this tx since it has rejected // parents so avoid re-requesting it from other peers. + // Here we add both the txid and the wtxid, as we know that + // regardless of what witness is provided, we will not accept + // this, so we don't need to allow for redownload of this txid + // from any of our non-wtxidrelay peers. recentRejects->insert(tx.GetHash()); + recentRejects->insert(tx.GetWitnessHash()); } } else { - if (!tx.HasWitness() && state.GetResult() != TxValidationResult::TX_WITNESS_MUTATED) { - // Do not use rejection cache for witness transactions or - // witness-stripped transactions, as they can have been malleated. - // See https://github.com/syscoin/syscoin/issues/8279 for details. + if (state.GetResult() != TxValidationResult::TX_WITNESS_STRIPPED) { + // We can add the wtxid of this transaction to our reject filter. + // Do not add txids of witness transactions or witness-stripped + // transactions to the filter, as they can have been malleated; + // adding such txids to the reject filter would potentially + // interfere with relay of valid transactions from peers that + // do not support wtxid-based relay. See + // https://github.com/bitcoin/bitcoin/issues/8279 for details. + // We can remove this restriction (and always add wtxids to + // the filter even for witness stripped transactions) once + // wtxid-based relay is broadly deployed. + // See also comments in https://github.com/bitcoin/bitcoin/pull/18044#discussion_r443419034 + // for concerns around weakening security of unupgraded nodes + // if we start doing this too early. assert(recentRejects); - recentRejects->insert(tx.GetHash()); + recentRejects->insert(tx.GetWitnessHash()); if (RecursiveDynamicUsage(*ptx) < 100000) { AddToCompactExtraTransactions(ptx); } @@ -3241,7 +3371,7 @@ void ProcessMessage( LogPrintf("Not relaying non-mempool transaction %s from forcerelay peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); } else { LogPrintf("Force relaying tx %s from peer=%d\n", tx.GetHash().ToString(), pfrom.GetId()); - RelayTransaction(tx.GetHash(), connman); + RelayTransaction(tx.GetHash(), tx.GetWitnessHash(), connman); } } } @@ -3906,7 +4036,7 @@ void ProcessMessage( if (vInv.size() <= MAX_PEER_TX_IN_FLIGHT + MAX_BLOCKS_IN_TRANSIT_PER_PEER) { for (CInv &inv : vInv) { // SYSCOIN - if (inv.type == MSG_TX || inv.type == MSG_WITNESS_TX || inv.IsMnType()) { + if (inv.type == MSG_TX || inv.type == MSG_WITNESS_TX || inv.type == MSG_WTX || inv.IsMnType()) { // If we receive a NOTFOUND message for a txid we requested, erase // it from our data structures for this peer. auto in_flight_it = state->m_tx_download.m_tx_in_flight.find(inv); @@ -4212,17 +4342,19 @@ namespace { class CompareInvMempoolOrder { CTxMemPool *mp; + bool m_wtxid_relay; public: - explicit CompareInvMempoolOrder(CTxMemPool *_mempool) + explicit CompareInvMempoolOrder(CTxMemPool *_mempool, bool use_wtxid) { mp = _mempool; + m_wtxid_relay = use_wtxid; } bool operator()(std::set::iterator a, std::set::iterator b) { /* As std::make_heap produces a max-heap, we want the entries with the * fewest ancestors/highest fee to sort later. */ - return mp->CompareDepthAndScore(*b, *a); + return mp->CompareDepthAndScore(*b, *a, m_wtxid_relay); } }; } @@ -4530,8 +4662,8 @@ bool PeerLogicValidation::SendMessages(CNode* pto) LOCK(pto->m_tx_relay->cs_filter); for (const auto& txinfo : vtxinfo) { - const uint256& hash = txinfo.tx->GetHash(); - CInv inv(MSG_TX, hash); + const uint256& hash = state.m_wtxid_relay ? txinfo.tx->GetWitnessHash() : txinfo.tx->GetHash(); + CInv inv(state.m_wtxid_relay ? MSG_WTX : MSG_TX, hash); pto->m_tx_relay->setInventoryTxToSend.erase(hash); // Don't send transactions that peers will not put into their mempool if (txinfo.fee < filterrate.GetFee(txinfo.vsize)) { @@ -4566,7 +4698,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) } // Topologically and fee-rate sort the inventory we send for privacy and priority reasons. // A heap is used so that not all items need sorting if only a few are being sent. - CompareInvMempoolOrder compareInvMempoolOrder(&m_mempool); + CompareInvMempoolOrder compareInvMempoolOrder(&m_mempool, state.m_wtxid_relay); std::make_heap(vInvTx.begin(), vInvTx.end(), compareInvMempoolOrder); // No reason to drain out at many times the network's capacity, // especially since we have many peers and some will draw much shorter delays. @@ -4585,10 +4717,12 @@ bool PeerLogicValidation::SendMessages(CNode* pto) continue; } // Not in the mempool anymore? don't bother sending it. - auto txinfo = m_mempool.info(hash); + auto txinfo = m_mempool.info(hash, state.m_wtxid_relay); if (!txinfo.tx) { continue; } + auto txid = txinfo.tx->GetHash(); + auto wtxid = txinfo.tx->GetWitnessHash(); // Peer told you to not send transactions at that feerate? Don't bother sending it. if (txinfo.fee < filterrate.GetFee(txinfo.vsize)) { continue; @@ -4596,7 +4730,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) if (pto->m_tx_relay->pfilter && !pto->m_tx_relay->pfilter->IsRelevantAndUpdate(*txinfo.tx)) continue; // Send State(pto->GetId())->m_recently_announced_invs.insert(hash); - vInv.push_back(CInv(MSG_TX, hash)); + vInv.push_back(CInv(state.m_wtxid_relay ? MSG_WTX : MSG_TX, hash)); nRelayedTransactions++; { // Expire old relay messages @@ -4606,9 +4740,14 @@ bool PeerLogicValidation::SendMessages(CNode* pto) vRelayExpiration.pop_front(); } - auto ret = mapRelay.insert(std::make_pair(hash, std::move(txinfo.tx))); + auto ret = mapRelay.emplace(txid, std::move(txinfo.tx)); if (ret.second) { - vRelayExpiration.push_back(std::make_pair(nNow + std::chrono::microseconds{RELAY_TX_CACHE_TIME}.count(), ret.first)); + vRelayExpiration.emplace_back(nNow + std::chrono::microseconds{RELAY_TX_CACHE_TIME}.count(), ret.first); + } + // Add wtxid-based lookup into mapRelay as well, so that peers can request by wtxid + auto ret2 = mapRelay.emplace(wtxid, ret.first->second); + if (ret2.second) { + vRelayExpiration.emplace_back(nNow + std::chrono::microseconds{RELAY_TX_CACHE_TIME}.count(), ret2.first); } } if (vInv.size() == MAX_INV_SZ) { @@ -4616,6 +4755,14 @@ bool PeerLogicValidation::SendMessages(CNode* pto) vInv.clear(); } pto->m_tx_relay->filterInventoryKnown.insert(hash); + if (hash != txid) { + // Insert txid into filterInventoryKnown, even for + // wtxidrelay peers. This prevents re-adding of + // unconfirmed parents to the recently_announced + // filter, when a child tx is requested. See + // ProcessGetData(). + pto->m_tx_relay->filterInventoryKnown.insert(txid); + } } } } @@ -4751,7 +4898,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // Erase this entry from tx_process_time (it may be added back for // processing at a later time, see below) tx_process_time.erase(tx_process_time.begin()); - //CInv inv(MSG_TX | GetFetchFlags(*pto), txid); + //CInv inv(state.m_wtxid_relay ? MSG_WTX : (MSG_TX | GetFetchFlags(*pto)), txid); if (!AlreadyHave(inv, m_mempool)) { // If this transaction was last requested more than 1 minute ago, // then request. @@ -4772,7 +4919,15 @@ bool PeerLogicValidation::SendMessages(CNode* pto) // (with a slight delay for inbound peers, to prefer // requests to outbound peers). // SYSCOIN - const auto next_process_time = CalculateTxGetDataTime(inv, current_time, !state.fPreferredDownload); + // Don't apply the txid-delay to re-requests of a + // transaction; the heuristic of delaying requests to + // txid-relay peers is to save bandwidth on initial + // announcement of a transaction, and doesn't make sense + // for a followup request if our first peer times out (and + // would open us up to an attacker using inbound + // wtxid-relay to prevent us from requesting transactions + // from outbound txid-relay peers). + const auto next_process_time = CalculateTxGetDataTime(inv, current_time, !state.fPreferredDownload, false); tx_process_time.emplace(next_process_time, inv); } } else { @@ -4837,6 +4992,7 @@ class CNetProcessingCleanup // orphan transactions mapOrphanTransactions.clear(); mapOrphanTransactionsByPrev.clear(); + g_orphans_by_wtxid.clear(); } }; static CNetProcessingCleanup instance_of_cnetprocessingcleanup; diff --git a/src/net_processing.h b/src/net_processing.h index 941f769cfd531..248e011837422 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -107,6 +107,6 @@ void Misbehaving(NodeId nodeid, int howmuch, const std::string& message="") EXCL void EraseTxRequest(NodeId nodeId, const CInv& inv) EXCLUSIVE_LOCKS_REQUIRED(cs_main); size_t GetRequestedTxCount(NodeId nodeId) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Relay transaction to every node */ -void RelayTransaction(const uint256&, const CConnman& connman); +void RelayTransaction(const uint256& txid, const uint256& wtxid, const CConnman& connman) EXCLUSIVE_LOCKS_REQUIRED(cs_main); #endif // SYSCOIN_NET_PROCESSING_H diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp index 3841d8687db45..5633abe817fd7 100644 --- a/src/node/transaction.cpp +++ b/src/node/transaction.cpp @@ -80,9 +80,10 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t if (relay) { // the mempool tracks locally submitted transactions to make a // best-effort of initial broadcast - node.mempool->AddUnbroadcastTx(hashTx); + node.mempool->AddUnbroadcastTx(hashTx, tx->GetWitnessHash()); - RelayTransaction(hashTx, *node.connman); + LOCK(cs_main); + RelayTransaction(hashTx, tx->GetWitnessHash(), *node.connman); } return TransactionError::OK; diff --git a/src/protocol.cpp b/src/protocol.cpp index 4cb832e4d0b27..68584385c788f 100644 --- a/src/protocol.cpp +++ b/src/protocol.cpp @@ -46,6 +46,7 @@ const char *GETCFHEADERS="getcfheaders"; const char *CFHEADERS="cfheaders"; const char *GETCFCHECKPT="getcfcheckpt"; const char *CFCHECKPT="cfcheckpt"; +const char *WTXIDRELAY="wtxidrelay"; // Syscoin message types const char *SPORK="spork"; const char *GETSPORKS="getsporks"; @@ -69,7 +70,6 @@ const char *QBSIGSHARES="qbsigs"; const char *QSIGREC="qsigrec"; const char *QSIGSHARE="qsigshare"; const char *MNAUTH="mnauth"; - } // namespace NetMsgType /** All known message types. Keep this in the same order as the list of @@ -131,8 +131,7 @@ const static std::string allNetMessageTypes[] = { NetMsgType::CFHEADERS, NetMsgType::GETCFCHECKPT, NetMsgType::CFCHECKPT, - NetMsgType::GETCFHEADERS, - NetMsgType::CFHEADERS, + NetMsgType::WTXIDRELAY, }; const static std::vector allNetMessageTypesVec(allNetMessageTypes, allNetMessageTypes+ARRAYLEN(allNetMessageTypes)); @@ -228,6 +227,8 @@ std::string CInv::GetCommand() const switch (masked) { case MSG_TX: return cmd.append(NetMsgType::TX); + // WTX is not a message type, just an inv type + case MSG_WTX: return cmd.append("wtx"); case MSG_BLOCK: return cmd.append(NetMsgType::BLOCK); case MSG_FILTERED_BLOCK: return cmd.append(NetMsgType::MERKLEBLOCK); case MSG_CMPCT_BLOCK: return cmd.append(NetMsgType::CMPCTBLOCK); diff --git a/src/protocol.h b/src/protocol.h index b97697a378569..5c823931b9597 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -284,6 +284,12 @@ extern const char* GETCFCHECKPT; * evenly spaced filter headers for blocks on the requested chain. */ extern const char* CFCHECKPT; +/** + * Indicates that a node prefers to relay transactions via wtxid, rather than + * txid. + * @since protocol version 70016 as described by BIP 339. + */ +extern const char *WTXIDRELAY; }; // namespace NetMsgType /* Get a vector of all valid message types (see above) */ @@ -420,11 +426,12 @@ const uint32_t MSG_TYPE_MASK = 0xffffffff >> 2; * These numbers are defined by the protocol. When adding a new value, be sure * to mention it in the respective BIP. */ -enum GetDataMsg { +enum GetDataMsg : uint32_t { UNDEFINED = 0, MSG_TX = 1, MSG_BLOCK = 2, - // The following can only occur in getdata. Invs always use TX or BLOCK. + MSG_WTX = 5, //!< Defined in BIP 339 + // The following can only occur in getdata. Invs always use TX/WTX or BLOCK. MSG_FILTERED_BLOCK = 3, //!< Defined in BIP37 MSG_CMPCT_BLOCK = 4, //!< Defined in BIP152 // SYSCOIN message types diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 0998f18db6f8c..3a98ca9e30b07 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1089,12 +1089,12 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const assert(innerUsage == cachedInnerUsage); } -bool CTxMemPool::CompareDepthAndScore(const uint256& hasha, const uint256& hashb) +bool CTxMemPool::CompareDepthAndScore(const uint256& hasha, const uint256& hashb, bool wtxid) { LOCK(cs); - indexed_transaction_set::const_iterator i = mapTx.find(hasha); + indexed_transaction_set::const_iterator i = wtxid ? get_iter_from_wtxid(hasha) : mapTx.find(hasha); if (i == mapTx.end()) return false; - indexed_transaction_set::const_iterator j = mapTx.find(hashb); + indexed_transaction_set::const_iterator j = wtxid ? get_iter_from_wtxid(hashb) : mapTx.find(hashb); if (j == mapTx.end()) return true; uint64_t counta = i->GetCountWithAncestors(); uint64_t countb = j->GetCountWithAncestors(); @@ -1174,10 +1174,10 @@ CTransactionRef CTxMemPool::get(const uint256& hash) const return i->GetSharedTx(); } -TxMempoolInfo CTxMemPool::info(const uint256& hash) const +TxMempoolInfo CTxMemPool::info(const uint256& hash, bool wtxid) const { LOCK(cs); - indexed_transaction_set::const_iterator i = mapTx.find(hash); + indexed_transaction_set::const_iterator i = (wtxid ? get_iter_from_wtxid(hash) : mapTx.find(hash)); if (i == mapTx.end()) return TxMempoolInfo(); return GetInfo(i); @@ -1369,8 +1369,8 @@ bool CCoinsViewMemPool::GetCoin(const COutPoint &outpoint, Coin &coin) const { size_t CTxMemPool::DynamicMemoryUsage() const { LOCK(cs); - // Estimate the overhead of mapTx to be 12 pointers + an allocation, as no exact formula for boost::multi_index_contained is implemented. - return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 12 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(mapLinks) + memusage::DynamicUsage(vTxHashes) + cachedInnerUsage; + // Estimate the overhead of mapTx to be 15 pointers + an allocation, as no exact formula for boost::multi_index_contained is implemented. + return memusage::MallocUsage(sizeof(CTxMemPoolEntry) + 15 * sizeof(void*)) * mapTx.size() + memusage::DynamicUsage(mapNextTx) + memusage::DynamicUsage(mapDeltas) + memusage::DynamicUsage(mapLinks) + memusage::DynamicUsage(vTxHashes) + cachedInnerUsage; } void CTxMemPool::RemoveUnbroadcastTx(const uint256& txid, const bool unchecked) { diff --git a/src/txmempool.h b/src/txmempool.h index a8734d9c3d093..015edba1f8d3d 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -206,6 +206,22 @@ struct mempoolentry_txid } }; +// extracts a transaction witness-hash from CTxMemPoolEntry or CTransactionRef +struct mempoolentry_wtxid +{ + typedef uint256 result_type; + result_type operator() (const CTxMemPoolEntry &entry) const + { + return entry.GetTx().GetWitnessHash(); + } + + result_type operator() (const CTransactionRef& tx) const + { + return tx->GetWitnessHash(); + } +}; + + /** \class CompareTxMemPoolEntryByDescendantScore * * Sort an entry by max(score/size of entry's tx, score/size with all descendants). @@ -326,6 +342,7 @@ class CompareTxMemPoolEntryByAncestorFee struct descendant_score {}; struct entry_time {}; struct ancestor_score {}; +struct index_by_wtxid {}; class CBlockPolicyEstimator; @@ -394,8 +411,9 @@ class SaltedTxidHasher * * CTxMemPool::mapTx, and CTxMemPoolEntry bookkeeping: * - * mapTx is a boost::multi_index that sorts the mempool on 4 criteria: - * - transaction hash + * mapTx is a boost::multi_index that sorts the mempool on 5 criteria: + * - transaction hash (txid) + * - witness-transaction hash (wtxid) * - descendant feerate [we use max(feerate of tx, feerate of tx with all descendants)] * - time in mempool * - ancestor feerate [we use min(feerate of tx, feerate of tx with all unconfirmed ancestors)] @@ -480,6 +498,12 @@ class CTxMemPool boost::multi_index::indexed_by< // sorted by txid boost::multi_index::hashed_unique, + // sorted by wtxid + boost::multi_index::hashed_unique< + boost::multi_index::tag, + mempoolentry_wtxid, + SaltedTxidHasher + >, // sorted by fee rate boost::multi_index::ordered_non_unique< boost::multi_index::tag, @@ -569,8 +593,11 @@ class CTxMemPool std::vector GetSortedDepthAndScore() const EXCLUSIVE_LOCKS_REQUIRED(cs); - /** track locally submitted transactions to periodically retry initial broadcast */ - std::set m_unbroadcast_txids GUARDED_BY(cs); + /** + * track locally submitted transactions to periodically retry initial broadcast + * map of txid -> wtxid + */ + std::map m_unbroadcast_txids GUARDED_BY(cs); // SYSCOIN std::multimap mapProTxRefs; // proTxHash -> transaction (all TXs that refer to an existing proTx) std::map mapProTxAddresses; @@ -620,7 +647,7 @@ class CTxMemPool void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(cs); void clear(); void _clear() EXCLUSIVE_LOCKS_REQUIRED(cs); //lock free - bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb); + bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb, bool wtxid=false); void queryHashes(std::vector& vtxid) const; bool isSpent(const COutPoint& outpoint) const; unsigned int GetTransactionsUpdated() const; @@ -723,25 +750,33 @@ class CTxMemPool return totalTxSize; } - bool exists(const uint256& hash) const + bool exists(const uint256& hash, bool wtxid=false) const { LOCK(cs); + if (wtxid) { + return (mapTx.get().count(hash) != 0); + } return (mapTx.count(hash) != 0); } CTransactionRef get(const uint256& hash) const; - TxMempoolInfo info(const uint256& hash) const; + txiter get_iter_from_wtxid(const uint256& wtxid) const EXCLUSIVE_LOCKS_REQUIRED(cs) + { + AssertLockHeld(cs); + return mapTx.project<0>(mapTx.get().find(wtxid)); + } + TxMempoolInfo info(const uint256& hash, bool wtxid=false) const; std::vector infoAll() const; // SYSCOIN bool existsProviderTxConflict(const CTransaction &tx) const; size_t DynamicMemoryUsage() const; /** Adds a transaction to the unbroadcast set */ - void AddUnbroadcastTx(const uint256& txid) { + void AddUnbroadcastTx(const uint256& txid, const uint256& wtxid) { LOCK(cs); // Sanity Check: the transaction should also be in the mempool if (exists(txid)) { - m_unbroadcast_txids.insert(txid); + m_unbroadcast_txids[txid] = wtxid; } } @@ -749,7 +784,7 @@ class CTxMemPool void RemoveUnbroadcastTx(const uint256& txid, const bool unchecked = false); /** Returns transactions in unbroadcast set */ - std::set GetUnbroadcastTxs() const { + std::map GetUnbroadcastTxs() const { LOCK(cs); return m_unbroadcast_txids; } diff --git a/src/validation.cpp b/src/validation.cpp index 1c99ceff5238e..6e4cee389ce3d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1040,7 +1040,7 @@ bool MemPoolAccept::PolicyScriptChecks(ATMPArgs& args, Workspace& ws, Precompute if (!tx.HasWitness() && CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~(SCRIPT_VERIFY_WITNESS | SCRIPT_VERIFY_CLEANSTACK), true, false, txdata) && !CheckInputScripts(tx, state_dummy, m_view, scriptVerifyFlags & ~SCRIPT_VERIFY_CLEANSTACK, true, false, txdata)) { // Only the witness is missing, so the transaction itself may be fine. - state.Invalid(TxValidationResult::TX_WITNESS_MUTATED, + state.Invalid(TxValidationResult::TX_WITNESS_STRIPPED, state.GetRejectReason(), state.GetDebugMessage()); } return false; // state filled in by CheckInputScripts @@ -5424,19 +5424,22 @@ bool LoadMempool(CTxMemPool& pool) } // TODO: remove this try except in v0.22 + std::map unbroadcast_txids; try { - std::set unbroadcast_txids; file >> unbroadcast_txids; unbroadcast = unbroadcast_txids.size(); - - for (const auto& txid : unbroadcast_txids) { - pool.AddUnbroadcastTx(txid); - } } catch (const std::exception&) { // mempool.dat files created prior to v0.21 will not have an // unbroadcast set. No need to log a failure if parsing fails here. } - + for (const auto& elem : unbroadcast_txids) { + // Don't add unbroadcast transactions that didn't get back into the + // mempool. + const CTransactionRef& added_tx = pool.get(elem.first); + if (added_tx != nullptr) { + pool.AddUnbroadcastTx(elem.first, added_tx->GetWitnessHash()); + } + } } catch (const std::exception& e) { LogPrintf("Failed to deserialize mempool data on disk: %s. Continuing anyway.\n", e.what()); return false; @@ -5452,7 +5455,7 @@ bool DumpMempool(const CTxMemPool& pool) std::map mapDeltas; std::vector vinfo; - std::set unbroadcast_txids; + std::map unbroadcast_txids; static Mutex dump_mutex; LOCK(dump_mutex); diff --git a/src/version.h b/src/version.h index 8c49e72479977..a531c04968974 100644 --- a/src/version.h +++ b/src/version.h @@ -9,7 +9,7 @@ * network protocol versioning */ -static const int PROTOCOL_VERSION = 70015; +static const int PROTOCOL_VERSION = 70016; //! Version when we switched to a size-based "headers" limit. static const int SIZE_HEADERS_LIMIT_VERSION = 70015; @@ -43,4 +43,7 @@ static const int SHORT_IDS_BLOCKS_VERSION = 70014; //! not banning for invalid compact blocks starts with this version static const int INVALID_CB_NO_BAN_VERSION = 70015; +//! "wtxidrelay" command for wtxid-based relay starts with this version +static const int WTXID_RELAY_VERSION = 70016; + #endif // SYSCOIN_VERSION_H diff --git a/test/functional/mempool_packages.py b/test/functional/mempool_packages.py index 4196ee5633f23..0b8b47c9123e5 100755 --- a/test/functional/mempool_packages.py +++ b/test/functional/mempool_packages.py @@ -69,14 +69,19 @@ def run_test(self): fee = Decimal("0.0001") # MAX_ANCESTORS transactions off a confirmed tx should be fine chain = [] + witness_chain = [] for i in range(MAX_ANCESTORS): (txid, sent_value) = self.chain_transaction(self.nodes[0], txid, 0, value, fee, 1) value = sent_value chain.append(txid) + # We need the wtxids to check P2P announcements + fulltx = self.nodes[0].getrawtransaction(txid) + witnesstx = self.nodes[0].decoderawtransaction(fulltx, True) + witness_chain.append(witnesstx['hash']) # Wait until mempool transactions have passed initial broadcast (sent inv and received getdata) # Otherwise, getrawmempool may be inconsistent with getmempoolentry if unbroadcast changes in between - self.nodes[0].p2p.wait_for_broadcast(chain) + self.nodes[0].p2p.wait_for_broadcast(witness_chain) # Check mempool has MAX_ANCESTORS transactions in it, and descendant and ancestor # count and fees should look correct diff --git a/test/functional/p2p_blocksonly.py b/test/functional/p2p_blocksonly.py index e5d0af1f28949..5f8255d730378 100755 --- a/test/functional/p2p_blocksonly.py +++ b/test/functional/p2p_blocksonly.py @@ -52,7 +52,7 @@ def run_test(self): self.log.info('Check that txs from rpc are not rejected and relayed to other peers') assert_equal(self.nodes[0].getpeerinfo()[0]['relaytxes'], True) txid = self.nodes[0].testmempoolaccept([sigtx])[0]['txid'] - with self.nodes[0].assert_debug_log(['received getdata for: tx {} peer=1'.format(txid)]): + with self.nodes[0].assert_debug_log(['received getdata for: wtx {} peer=1'.format(txid)]): self.nodes[0].sendrawtransaction(sigtx) self.nodes[0].p2p.wait_for_tx(txid) assert_equal(self.nodes[0].getmempoolinfo()['size'], 1) diff --git a/test/functional/p2p_feefilter.py b/test/functional/p2p_feefilter.py index 8cebd2b69c381..66cdc9ec28b97 100755 --- a/test/functional/p2p_feefilter.py +++ b/test/functional/p2p_feefilter.py @@ -7,7 +7,7 @@ from decimal import Decimal import time -from test_framework.messages import MSG_TX, msg_feefilter +from test_framework.messages import MSG_TX, MSG_WTX, msg_feefilter from test_framework.mininode import mininode_lock, P2PInterface from test_framework.test_framework import SyscoinTestFramework from test_framework.util import assert_equal @@ -45,7 +45,7 @@ def __init__(self): def on_inv(self, message): for i in message.inv: - if (i.type == MSG_TX): + if (i.type == MSG_TX) or (i.type == MSG_WTX): self.txinvs.append(hashToHex(i.hash)) def clear_invs(self): diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index cc5bf07d597c1..635ad862bc77f 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -25,6 +25,7 @@ MSG_BLOCK, MSG_TX, MSG_WITNESS_FLAG, + MSG_WTX, NODE_NETWORK, NODE_WITNESS, msg_no_witness_block, @@ -34,6 +35,7 @@ msg_tx, msg_block, msg_no_witness_tx, + msg_verack, ser_uint256, ser_vector, sha256, @@ -81,6 +83,7 @@ softfork_active, hex_str_to_bytes, assert_raises_rpc_error, + wait_until, ) # The versionbit bit used to signal activation of SegWit @@ -142,25 +145,47 @@ def test_witness_block(node, p2p, block, accepted, with_witness=True, reason=Non class TestP2PConn(P2PInterface): - def __init__(self): + def __init__(self, wtxidrelay=False): super().__init__() self.getdataset = set() + self.last_wtxidrelay = [] + self.lastgetdata = [] + self.wtxidrelay = wtxidrelay # Avoid sending out msg_getdata in the mininode thread as a reply to invs. # They are not needed and would only lead to races because we send msg_getdata out in the test thread def on_inv(self, message): pass + def on_version(self, message): + if self.wtxidrelay: + super().on_version(message) + else: + self.send_message(msg_verack()) + self.nServices = message.nServices + def on_getdata(self, message): + self.lastgetdata = message.inv for inv in message.inv: self.getdataset.add(inv.hash) - def announce_tx_and_wait_for_getdata(self, tx, timeout=60, success=True): + def on_wtxidrelay(self, message): + self.last_wtxidrelay.append(message) + + def announce_tx_and_wait_for_getdata(self, tx, timeout=60, success=True, use_wtxid=False): with mininode_lock: self.last_message.pop("getdata", None) - self.send_message(msg_inv(inv=[CInv(MSG_TX, tx.sha256)])) + if use_wtxid: + wtxid = tx.calc_sha256(True) + self.send_message(msg_inv(inv=[CInv(MSG_WTX, wtxid)])) + else: + self.send_message(msg_inv(inv=[CInv(MSG_TX, tx.sha256)])) + if success: - self.wait_for_getdata([tx.sha256], timeout) + if use_wtxid: + self.wait_for_getdata([wtxid], timeout) + else: + self.wait_for_getdata([tx.sha256], timeout) else: time.sleep(timeout) assert not self.last_message.get("getdata") @@ -277,6 +302,7 @@ def run_test(self): self.test_upgrade_after_activation() self.test_witness_sigops() self.test_superfluous_witness() + self.test_wtxid_relay() # Individual tests @@ -1273,7 +1299,6 @@ def test_tx_relay_after_segwit_activation(self): test_transaction_acceptance(self.nodes[0], self.test_node, tx, with_witness=True, accepted=False) # Verify that removing the witness succeeds. - self.test_node.announce_tx_and_wait_for_getdata(tx) test_transaction_acceptance(self.nodes[0], self.test_node, tx, with_witness=False, accepted=True) # Now try to add extra witness data to a valid witness tx. @@ -1300,8 +1325,6 @@ def test_tx_relay_after_segwit_activation(self): # Node will not be blinded to the transaction self.std_node.announce_tx_and_wait_for_getdata(tx3) test_transaction_acceptance(self.nodes[1], self.std_node, tx3, True, False, 'tx-size') - self.std_node.announce_tx_and_wait_for_getdata(tx3) - test_transaction_acceptance(self.nodes[1], self.std_node, tx3, True, False, 'tx-size') # Remove witness stuffing, instead add extra witness push on stack tx3.vout[0] = CTxOut(tx2.vout[0].nValue - 1000, CScript([OP_TRUE, OP_DROP] * 15 + [OP_TRUE])) @@ -2019,6 +2042,11 @@ def test_witness_sigops(self): # TODO: test p2sh sigop counting + # Cleanup and prep for next test + self.utxo.pop(0) + self.utxo.append(UTXO(tx2.sha256, 0, tx2.vout[0].nValue)) + + @subtest # type: ignore def test_superfluous_witness(self): # Serialization of tx that puts witness flag to 3 always def serialize_with_bogus_witness(tx): @@ -2062,6 +2090,67 @@ def serialize(self): with self.nodes[0].assert_debug_log(['Unknown transaction optional data']): self.nodes[0].p2p.send_and_ping(msg_bogus_tx(tx)) + @subtest # type: ignore + def test_wtxid_relay(self): + # Use brand new nodes to avoid contamination from earlier tests + self.wtx_node = self.nodes[0].add_p2p_connection(TestP2PConn(wtxidrelay=True), services=NODE_NETWORK | NODE_WITNESS) + self.tx_node = self.nodes[0].add_p2p_connection(TestP2PConn(wtxidrelay=False), services=NODE_NETWORK | NODE_WITNESS) + + # Check wtxidrelay feature negotiation message through connecting a new peer + def received_wtxidrelay(): + return (len(self.wtx_node.last_wtxidrelay) > 0) + wait_until(received_wtxidrelay, timeout=60, lock=mininode_lock) + + # Create a Segwit output from the latest UTXO + # and announce it to the network + witness_program = CScript([OP_TRUE]) + witness_hash = sha256(witness_program) + script_pubkey = CScript([OP_0, witness_hash]) + + tx = CTransaction() + tx.vin.append(CTxIn(COutPoint(self.utxo[0].sha256, self.utxo[0].n), b"")) + tx.vout.append(CTxOut(self.utxo[0].nValue - 1000, script_pubkey)) + tx.rehash() + + # Create a Segwit transaction + tx2 = CTransaction() + tx2.vin.append(CTxIn(COutPoint(tx.sha256, 0), b"")) + tx2.vout.append(CTxOut(tx.vout[0].nValue - 1000, script_pubkey)) + tx2.wit.vtxinwit.append(CTxInWitness()) + tx2.wit.vtxinwit[0].scriptWitness.stack = [witness_program] + tx2.rehash() + + # Announce Segwit transaction with wtxid + # and wait for getdata + self.wtx_node.announce_tx_and_wait_for_getdata(tx2, use_wtxid=True) + with mininode_lock: + lgd = self.wtx_node.lastgetdata[:] + assert_equal(lgd, [CInv(MSG_WTX, tx2.calc_sha256(True))]) + + # Announce Segwit transaction from non wtxidrelay peer + # and wait for getdata + self.tx_node.announce_tx_and_wait_for_getdata(tx2, use_wtxid=False) + with mininode_lock: + lgd = self.tx_node.lastgetdata[:] + assert_equal(lgd, [CInv(MSG_TX|MSG_WITNESS_FLAG, tx2.sha256)]) + + # Send tx2 through; it's an orphan so won't be accepted + with mininode_lock: + self.tx_node.last_message.pop("getdata", None) + test_transaction_acceptance(self.nodes[0], self.tx_node, tx2, with_witness=True, accepted=False) + + # Expect a request for parent (tx) due to use of non-WTX peer + self.tx_node.wait_for_getdata([tx.sha256], 60) + with mininode_lock: + lgd = self.tx_node.lastgetdata[:] + assert_equal(lgd, [CInv(MSG_TX|MSG_WITNESS_FLAG, tx.sha256)]) + + # Send tx through + test_transaction_acceptance(self.nodes[0], self.tx_node, tx, with_witness=False, accepted=True) + + # Check tx2 is there now + assert_equal(tx2.hash in self.nodes[0].getrawmempool(), True) + if __name__ == '__main__': SegWitTest().main() diff --git a/test/functional/p2p_tx_download.py b/test/functional/p2p_tx_download.py index c7d3d2a48c101..d7b48cf7387d7 100755 --- a/test/functional/p2p_tx_download.py +++ b/test/functional/p2p_tx_download.py @@ -12,6 +12,7 @@ FromHex, MSG_TX, MSG_TYPE_MASK, + MSG_WTX, msg_inv, msg_notfound, ) @@ -36,7 +37,7 @@ def __init__(self): def on_getdata(self, message): for i in message.inv: - if i.type & MSG_TYPE_MASK == MSG_TX: + if i.type & MSG_TYPE_MASK == MSG_TX or i.type & MSG_TYPE_MASK == MSG_WTX: self.tx_getdata_count += 1 @@ -44,12 +45,13 @@ def on_getdata(self, message): GETDATA_TX_INTERVAL = 60 # seconds MAX_GETDATA_RANDOM_DELAY = 2 # seconds INBOUND_PEER_TX_DELAY = 2 # seconds +TXID_RELAY_DELAY = 2 # seconds MAX_GETDATA_IN_FLIGHT = 100 TX_EXPIRY_INTERVAL = GETDATA_TX_INTERVAL * 10 # Python test constants NUM_INBOUND = 10 -MAX_GETDATA_INBOUND_WAIT = GETDATA_TX_INTERVAL + MAX_GETDATA_RANDOM_DELAY + INBOUND_PEER_TX_DELAY +MAX_GETDATA_INBOUND_WAIT = GETDATA_TX_INTERVAL + MAX_GETDATA_RANDOM_DELAY + INBOUND_PEER_TX_DELAY + TXID_RELAY_DELAY class TxDownloadTest(SyscoinTestFramework): @@ -63,7 +65,7 @@ def test_tx_requests(self): txid = 0xdeadbeef self.log.info("Announce the txid from each incoming peer to node 0") - msg = msg_inv([CInv(t=MSG_TX, h=txid)]) + msg = msg_inv([CInv(t=MSG_WTX, h=txid)]) for p in self.nodes[0].p2ps: p.send_and_ping(msg) @@ -135,13 +137,13 @@ def test_in_flight_max(self): with mininode_lock: p.tx_getdata_count = 0 - p.send_message(msg_inv([CInv(t=MSG_TX, h=i) for i in txids])) + p.send_message(msg_inv([CInv(t=MSG_WTX, h=i) for i in txids])) wait_until(lambda: p.tx_getdata_count >= MAX_GETDATA_IN_FLIGHT, lock=mininode_lock) with mininode_lock: assert_equal(p.tx_getdata_count, MAX_GETDATA_IN_FLIGHT) self.log.info("Now check that if we send a NOTFOUND for a transaction, we'll get one more request") - p.send_message(msg_notfound(vec=[CInv(t=MSG_TX, h=txids[0])])) + p.send_message(msg_notfound(vec=[CInv(t=MSG_WTX, h=txids[0])])) wait_until(lambda: p.tx_getdata_count >= MAX_GETDATA_IN_FLIGHT + 1, timeout=10, lock=mininode_lock) with mininode_lock: assert_equal(p.tx_getdata_count, MAX_GETDATA_IN_FLIGHT + 1) diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index abc45b9eb951f..43f32075c1f75 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -31,7 +31,7 @@ from test_framework.util import hex_str_to_bytes, assert_equal from collections import namedtuple MIN_VERSION_SUPPORTED = 60001 -MY_VERSION = 70014 # past bip-31 for ping/pong +MY_VERSION = 70016 # past wtxid relay MY_SUBVERSION = b"/python-mininode-tester:0.0.3/" MY_RELAY = 1 # from version 70001 onwards, fRelay should be appended to version messages (BIP37) @@ -60,6 +60,7 @@ MSG_BLOCK = 2 MSG_FILTERED_BLOCK = 3 MSG_CMPCT_BLOCK = 4 +MSG_WTX = 5 MSG_WITNESS_FLAG = 1 << 30 MSG_TYPE_MASK = 0xffffffff >> 2 @@ -314,6 +315,7 @@ class CInv: MSG_QUORUM_JUSTIFICATION: "qjustify", MSG_QUORUM_PREMATURE_COMMITMENT: "qpcommit", MSG_QUORUM_RECOVERED_SIG: "qsigrec" + 5: "WTX", } def __init__(self, t=0, h=0): @@ -334,6 +336,9 @@ def __repr__(self): return "CInv(type=%s hash=%064x)" \ % (self.typemap[self.type], self.hash) + def __eq__(self, other): + return isinstance(other, CInv) and self.hash == other.hash and self.type == other.type + class CBlockLocator: __slots__ = ("nVersion", "vHave") @@ -1364,6 +1369,22 @@ def serialize(self): def __repr__(self): return "msg_tx(tx=%s)" % (repr(self.tx)) +class msg_wtxidrelay: + __slots__ = () + msgtype = b"wtxidrelay" + + def __init__(self): + pass + + def deserialize(self, f): + pass + + def serialize(self): + return b"" + + def __repr__(self): + return "msg_wtxidrelay()" + class msg_no_witness_tx(msg_tx): __slots__ = () diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py index c0134eccb7493..d9f28c9b6cf5a 100755 --- a/test/functional/test_framework/mininode.py +++ b/test/functional/test_framework/mininode.py @@ -59,6 +59,8 @@ MSG_TYPE_MASK, msg_verack, msg_version, + MSG_WTX, + msg_wtxidrelay, NODE_NETWORK, NODE_WITNESS, sha256, @@ -99,6 +101,7 @@ b"tx": msg_tx, b"verack": msg_verack, b"version": msg_version, + b"wtxidrelay": msg_wtxidrelay, # Syscoin Specific b"getmnlistd": msg_getmnlistd, b"getsporks": None, @@ -370,6 +373,7 @@ def on_pong(self, message): pass def on_sendcmpct(self, message): pass def on_sendheaders(self, message): pass def on_tx(self, message): pass + def on_wtxidrelay(self, message): pass # SYSCOIN def on_mnlistdiff(self, message): pass @@ -389,6 +393,8 @@ def on_verack(self, message): def on_version(self, message): assert message.nVersion >= MIN_VERSION_SUPPORTED, "Version {} received. Test framework only supports versions greater than {}".format(message.nVersion, MIN_VERSION_SUPPORTED) + if message.nVersion >= 70016: + self.send_message(msg_wtxidrelay()) self.send_message(msg_verack()) self.nServices = message.nServices @@ -670,7 +676,7 @@ def on_inv(self, message): super().on_inv(message) # Send getdata in response. # Store how many times invs have been received for each tx. for i in message.inv: - if i.type == MSG_TX: + if (i.type == MSG_TX) or (i.type == MSG_WTX): # save txid self.tx_invs_received[i.hash] += 1