diff --git a/src/main.cpp b/src/main.cpp index 6bc40c659..c8ee7cb3c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -8336,6 +8336,24 @@ bool SendMessages(CNode* pto, bool fSendTrickle) // Message: addr if (fSendTrickle) { + // Accumulate into vAddr and send it ONCE (or in MAX_ADDR_TO_SEND-sized batches). + // This loop previously pushed the ENTIRE pto->vAddrToSend on every accepted address, + // so a single 24-byte getaddr produced N messages of N addresses each instead of one + // message of N -- ~500x the intended bandwidth on a typical addrman, all serialized + // (with per-message double-SHA256 checksums) while cs_main is held. The locally built + // vAddr was accumulated and then discarded, and the vAddr.resize(MAX_ADDR_TO_SEND) was + // a no-op standing where upstream has vAddr.clear(). + const char* msg_type; + int make_flags; + if (pto->m_wants_addrv2) { + msg_type = NetMsgType::ADDRV2; + make_flags = ADDRV2_FORMAT; + } else { + msg_type = NetMsgType::ADDR; + make_flags = 0; + } + const CNetMsgMaker msgMaker(std::min(pto->nVersion, PROTOCOL_VERSION)); + vector vAddr; vAddr.reserve(pto->vAddrToSend.size()); BOOST_FOREACH(const CAddress& addr, pto->vAddrToSend) @@ -8343,29 +8361,17 @@ bool SendMessages(CNode* pto, bool fSendTrickle) if (pto->AddAddressIfNotAlreadyKnown(addr)) { vAddr.push_back(addr); - if (vAddr.size() >= MAX_ADDR_TO_SEND) { - // Should be impossible since we always check size before adding to - // vAddrToSend. Recover by trimming the vector. - vAddr.resize(MAX_ADDR_TO_SEND); + pto->PushAddrMessage(msgMaker.Make(make_flags, msg_type, vAddr)); + vAddr.clear(); } - const char* msg_type; - int make_flags; - if (pto->m_wants_addrv2) { - msg_type = NetMsgType::ADDRV2; - make_flags = ADDRV2_FORMAT; - } else { - msg_type = NetMsgType::ADDR; - make_flags = 0; - } - pto->PushAddrMessage(CNetMsgMaker(std::min(pto->nVersion, PROTOCOL_VERSION)).Make(make_flags, msg_type, pto->vAddrToSend)); - } } pto->vAddrToSend.clear(); - vAddr.clear(); + if (!vAddr.empty()) + pto->PushAddrMessage(msgMaker.Make(make_flags, msg_type, vAddr)); } CNodeState &state = *State(pto->GetId()); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 443498cba..285c09f5d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1240,6 +1240,16 @@ int CWallet::SaplingWitnessMinimumHeight(const uint256& nullifier, int nWitnessH return nMinimumHeight; } +int CWallet::SaplingWitnessMinimumHeight(const boost::optional& nullifier, int nWitnessHeight, int nMinimumHeight) +{ + // No nullifier => an incoming-viewing-key-only note (z_importviewingkey without the full + // viewing key). Spend depth is unknowable, so treat it as unspent and keep its witness. + if (!nullifier) { + return min(nWitnessHeight, nMinimumHeight); + } + return SaplingWitnessMinimumHeight(*nullifier, nWitnessHeight, nMinimumHeight); +} + int CWallet::VerifyAndSetInitialWitness(const CBlockIndex* pindex, bool witnessOnly) { LOCK2(cs_main, cs_wallet); @@ -1277,7 +1287,7 @@ int CWallet::VerifyAndSetInitialWitness(const CBlockIndex* pindex, bool witnessO //Skip Validation when witness root has been validated if (nd->witnessRootValidated) { - nMinimumHeight = SaplingWitnessMinimumHeight(*item.second.nullifier, nd->witnessHeight, nMinimumHeight); + nMinimumHeight = SaplingWitnessMinimumHeight(item.second.nullifier, nd->witnessHeight, nMinimumHeight); continue; } @@ -1289,12 +1299,12 @@ int CWallet::VerifyAndSetInitialWitness(const CBlockIndex* pindex, bool witnessO CBlockIndex* whIndex = chainActive[nd->witnessHeight]; if (whIndex == NULL) { //witnessHeight strictly above the active chain (transient catch-up): cannot validate yet - nMinimumHeight = SaplingWitnessMinimumHeight(*item.second.nullifier, nd->witnessHeight, nMinimumHeight); + nMinimumHeight = SaplingWitnessMinimumHeight(item.second.nullifier, nd->witnessHeight, nMinimumHeight); continue; } if (nd->witnesses.front().root() == whIndex->hashFinalSaplingRoot) { nd->witnessRootValidated = true; - nMinimumHeight = SaplingWitnessMinimumHeight(*item.second.nullifier, nd->witnessHeight, nMinimumHeight); + nMinimumHeight = SaplingWitnessMinimumHeight(item.second.nullifier, nd->witnessHeight, nMinimumHeight); continue; } //root mismatch on the active chain -> desynced; fall through to rebuild below @@ -1306,7 +1316,7 @@ int CWallet::VerifyAndSetInitialWitness(const CBlockIndex* pindex, bool witnessO blockRoot = pblockindex->hashFinalSaplingRoot; if (witnessRoot == blockRoot) { nd->witnessRootValidated = true; - nMinimumHeight = SaplingWitnessMinimumHeight(*item.second.nullifier, nd->witnessHeight, nMinimumHeight); + nMinimumHeight = SaplingWitnessMinimumHeight(item.second.nullifier, nd->witnessHeight, nMinimumHeight); continue; } } @@ -1358,7 +1368,7 @@ int CWallet::VerifyAndSetInitialWitness(const CBlockIndex* pindex, bool witnessO } nd->witnessHeight = pblockindex->GetHeight(); UpdateSaplingNullifierNoteMapWithTx(wtxItem.second); - nMinimumHeight = SaplingWitnessMinimumHeight(*item.second.nullifier, nd->witnessHeight, nMinimumHeight); + nMinimumHeight = SaplingWitnessMinimumHeight(item.second.nullifier, nd->witnessHeight, nMinimumHeight); } } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 107814971..27754d8f3 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -896,6 +896,12 @@ public: protected: int SaplingWitnessMinimumHeight(const uint256& nullifier, int nWitnessHeight, int nMinimumHeight); + //! Overload for a note whose nullifier may be unset. A note discovered through an imported + //! INCOMING viewing key has no nullifier (computing one needs the full viewing key), so + //! dereferencing the optional aborts the daemon. Treats such a note as unspent, which is the + //! conservative direction: it keeps the witness alive rather than pruning a note we cannot + //! prove spent. + int SaplingWitnessMinimumHeight(const boost::optional& nullifier, int nWitnessHeight, int nMinimumHeight); /** * pindex is the new tip being connected.