Replace setInventoryKnown with a rolling bloom filter.

Mruset setInventoryKnown was reduced to a remarkably small 1000 entries as a side effect of sendbuffer size reductions in 2012. This removes setInventoryKnown filtering from merkleBlock responses because false positives there are especially unattractive and also because I'm not sure if there aren't race conditions around the relay pool that would cause some transactions there to be suppressed. (Also, ProcessGetData was accessing setInventoryKnown without taking the required lock.)
PIVX-Project · Jun 23, 2020 · 2e3b05c · 2e3b05c
1 parent 409aa83
commit 2e3b05c
Show file tree

Hide file tree

Showing 3 changed files with 9 additions and 9 deletions.
diff --git a/src/main.cpp b/src/main.cpp
@@ -5159,8 +5159,7 @@ void static ProcessGetData(CNode* pfrom)
                             // however we MUST always provide at least what the remote peer needs
                             typedef std::pair<unsigned int, uint256> PairType;
                             for (PairType& pair : merkleBlock.vMatchedTxn)
-                                if (!pfrom->setInventoryKnown.count(CInv(MSG_TX, pair.second)))
-                                    pfrom->PushMessage(NetMsgType::TX, block.vtx[pair.first]);
+                                pfrom->PushMessage(NetMsgType::TX, block.vtx[pair.first]);
                         }
                         // else
                         // no response
@@ -6398,7 +6397,7 @@ bool SendMessages(CNode* pto)
             vInv.reserve(pto->vInventoryToSend.size());
             vInvWait.reserve(pto->vInventoryToSend.size());
             for (const CInv& inv : pto->vInventoryToSend) {
-                if (pto->setInventoryKnown.count(inv))
+                if (pto->setInventoryKnown.contains(inv.hash))
                     continue;
 
                 // trickle out tx inv to protect privacy
@@ -6417,8 +6416,8 @@ bool SendMessages(CNode* pto)
                     }
                 }
 
-                // returns true if wasn't already contained in the set
-                if (pto->setInventoryKnown.insert(inv).second) {
+                if (!pto->setInventoryKnown.contains(inv.hash)) {
+                    pto->setInventoryKnown.insert(inv.hash);
                     vInv.push_back(inv);
                     if (vInv.size() >= 1000) {
                         pto->PushMessage(NetMsgType::INV, vInv);

diff --git a/src/net.cpp b/src/net.cpp
@@ -2319,7 +2319,7 @@ unsigned int SendBufferSize() { return 1000 * GetArg("-maxsendbuffer", 1 * 1000)
 CNode::CNode(SOCKET hSocketIn, CAddress addrIn, std::string addrNameIn, bool fInboundIn) :
     ssSend(SER_NETWORK, INIT_PROTO_VERSION),
     addrKnown(5000, 0.001),
-    setInventoryKnown(SendBufferSize() / 1000)
+    setInventoryKnown(50000, 0.000001)
 {
     nServices = NODE_NONE;
     nServicesExpected = NODE_NONE;
@@ -2347,6 +2347,7 @@ CNode::CNode(SOCKET hSocketIn, CAddress addrIn, std::string addrNameIn, bool fIn
     nSendOffset = 0;
     hashContinue = UINT256_ZERO;
     nStartingHeight = -1;
+    setInventoryKnown.reset();
     fGetAddr = false;
     nNextLocalAddrSend = 0;
     nNextAddrSend = 0;

diff --git a/src/net.h b/src/net.h
@@ -387,7 +387,7 @@ class CNode
     int64_t nNextLocalAddrSend;
 
     // inventory based relay
-    mruset<CInv> setInventoryKnown;
+    CRollingBloomFilter setInventoryKnown;
     std::vector<CInv> vInventoryToSend;
     RecursiveMutex cs_inventory;
     std::multimap<int64_t, CInv> mapAskFor;
@@ -487,15 +487,15 @@ class CNode
     {
         {
             LOCK(cs_inventory);
-            setInventoryKnown.insert(inv);
+            setInventoryKnown.insert(inv.hash);
         }
     }
 
     void PushInventory(const CInv& inv)
     {
         {
             LOCK(cs_inventory);
-            if (!setInventoryKnown.count(inv))
+            if (!setInventoryKnown.contains(inv.hash))
                 vInventoryToSend.push_back(inv);
         }
     }