Skip to content

Commit 5400ef6

Browse files
committed
Replace trickle nodes with per-node/message Poisson delays
We used to have a trickle node, a node which was chosen in each iteration of the send loop that was privileged and allowed to send out queued up non-time critical messages. Since the removal of the fixed sleeps in the network code, this resulted in fast and attackable treatment of such broadcasts. This pull request changes the 3 remaining trickle use cases by random delays: * Local address broadcast (while also removing the the wiping of the seen filter) * Address relay * Inv relay (for transactions; blocks are always relayed immediately) The code is based on older commits by Patrick Strateman.
1 parent 9ee02cf commit 5400ef6

File tree

5 files changed

+47
-36
lines changed

5 files changed

+47
-36
lines changed

src/main.cpp

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5326,7 +5326,7 @@ bool ProcessMessages(CNode* pfrom)
53265326
}
53275327

53285328

5329-
bool SendMessages(CNode* pto, bool fSendTrickle)
5329+
bool SendMessages(CNode* pto)
53305330
{
53315331
const Consensus::Params& consensusParams = Params().GetConsensus();
53325332
{
@@ -5368,28 +5368,17 @@ bool SendMessages(CNode* pto, bool fSendTrickle)
53685368
return true;
53695369

53705370
// Address refresh broadcast
5371-
static int64_t nLastRebroadcast;
5372-
if (!IsInitialBlockDownload() && (GetTime() - nLastRebroadcast > 24 * 60 * 60))
5373-
{
5374-
LOCK(cs_vNodes);
5375-
BOOST_FOREACH(CNode* pnode, vNodes)
5376-
{
5377-
// Periodically clear addrKnown to allow refresh broadcasts
5378-
if (nLastRebroadcast)
5379-
pnode->addrKnown.reset();
5380-
5381-
// Rebroadcast our address
5382-
AdvertizeLocal(pnode);
5383-
}
5384-
if (!vNodes.empty())
5385-
nLastRebroadcast = GetTime();
5371+
int64_t nNow = GetTimeMicros();
5372+
if (!IsInitialBlockDownload() && pto->nNextLocalAddrSend < nNow) {
5373+
AdvertizeLocal(pto);
5374+
pto->nNextLocalAddrSend = PoissonNextSend(nNow, AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL);
53865375
}
53875376

53885377
//
53895378
// Message: addr
53905379
//
5391-
if (fSendTrickle)
5392-
{
5380+
if (pto->nNextAddrSend < nNow) {
5381+
pto->nNextAddrSend = PoissonNextSend(nNow, AVG_ADDRESS_BROADCAST_INTERVAL);
53935382
vector<CAddress> vAddr;
53945383
vAddr.reserve(pto->vAddrToSend.size());
53955384
BOOST_FOREACH(const CAddress& addr, pto->vAddrToSend)
@@ -5563,8 +5552,13 @@ bool SendMessages(CNode* pto, bool fSendTrickle)
55635552
vector<CInv> vInv;
55645553
vector<CInv> vInvWait;
55655554
{
5555+
bool fSendTrickle = pto->fWhitelisted;
5556+
if (pto->nNextInvSend < nNow) {
5557+
fSendTrickle = true;
5558+
pto->nNextInvSend = PoissonNextSend(nNow, AVG_INVENTORY_BROADCAST_INTERVAL);
5559+
}
55665560
LOCK(pto->cs_inventory);
5567-
vInv.reserve(pto->vInventoryToSend.size());
5561+
vInv.reserve(std::min<size_t>(1000, pto->vInventoryToSend.size()));
55685562
vInvWait.reserve(pto->vInventoryToSend.size());
55695563
BOOST_FOREACH(const CInv& inv, pto->vInventoryToSend)
55705564
{
@@ -5604,7 +5598,7 @@ bool SendMessages(CNode* pto, bool fSendTrickle)
56045598
pto->PushMessage(NetMsgType::INV, vInv);
56055599

56065600
// Detect whether we're stalling
5607-
int64_t nNow = GetTimeMicros();
5601+
nNow = GetTimeMicros();
56085602
if (!pto->fDisconnect && state.nStallingSince && state.nStallingSince < nNow - 1000000 * BLOCK_STALLING_TIMEOUT) {
56095603
// Stalling only triggers when the block download window cannot move. During normal steady state,
56105604
// the download window should be much larger than the to-be-downloaded set of blocks, so disconnection

src/main.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,14 @@ static const unsigned int DATABASE_WRITE_INTERVAL = 60 * 60;
8787
static const unsigned int DATABASE_FLUSH_INTERVAL = 24 * 60 * 60;
8888
/** Maximum length of reject messages. */
8989
static const unsigned int MAX_REJECT_MESSAGE_LENGTH = 111;
90+
/** Average delay between local address broadcasts in seconds. */
91+
static const unsigned int AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL = 24 * 24 * 60;
92+
/** Average delay between peer address broadcasts in seconds. */
93+
static const unsigned int AVG_ADDRESS_BROADCAST_INTERVAL = 30;
94+
/** Average delay between trickled inventory broadcasts in seconds.
95+
* Blocks, whitelisted receivers, and a random 25% of transactions bypass this. */
96+
static const unsigned int AVG_INVENTORY_BROADCAST_INTERVAL = 5;
97+
9098
static const unsigned int DEFAULT_LIMITFREERELAY = 15;
9199
static const bool DEFAULT_RELAYPRIORITY = true;
92100

@@ -197,9 +205,8 @@ bool ProcessMessages(CNode* pfrom);
197205
* Send queued protocol messages to be sent to a give node.
198206
*
199207
* @param[in] pto The node which we are sending messages to.
200-
* @param[in] fSendTrickle When true send the trickled data, otherwise trickle the data until true.
201208
*/
202-
bool SendMessages(CNode* pto, bool fSendTrickle);
209+
bool SendMessages(CNode* pto);
203210
/** Run an instance of the script checking thread */
204211
void ThreadScriptCheck();
205212
/** Try to detect Partition (network isolation) attacks against us */

src/net.cpp

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@
3636
#include <boost/filesystem.hpp>
3737
#include <boost/thread.hpp>
3838

39+
#include <math.h>
40+
3941
// Dump addresses to peers.dat every 15 minutes (900s)
4042
#define DUMP_ADDRESSES_INTERVAL 900
4143

@@ -1733,11 +1735,6 @@ void ThreadMessageHandler()
17331735
}
17341736
}
17351737

1736-
// Poll the connected nodes for messages
1737-
CNode* pnodeTrickle = NULL;
1738-
if (!vNodesCopy.empty())
1739-
pnodeTrickle = vNodesCopy[GetRand(vNodesCopy.size())];
1740-
17411738
bool fSleep = true;
17421739

17431740
BOOST_FOREACH(CNode* pnode, vNodesCopy)
@@ -1768,7 +1765,7 @@ void ThreadMessageHandler()
17681765
{
17691766
TRY_LOCK(pnode->cs_vSend, lockSend);
17701767
if (lockSend)
1771-
g_signals.SendMessages(pnode, pnode == pnodeTrickle || pnode->fWhitelisted);
1768+
g_signals.SendMessages(pnode);
17721769
}
17731770
boost::this_thread::interruption_point();
17741771
}
@@ -2384,6 +2381,9 @@ CNode::CNode(SOCKET hSocketIn, const CAddress& addrIn, const std::string& addrNa
23842381
nStartingHeight = -1;
23852382
filterInventoryKnown.reset();
23862383
fGetAddr = false;
2384+
nNextLocalAddrSend = 0;
2385+
nNextAddrSend = 0;
2386+
nNextInvSend = 0;
23872387
fRelayTxes = false;
23882388
pfilter = new CBloomFilter();
23892389
nPingNonceSent = 0;
@@ -2634,3 +2634,7 @@ void DumpBanlist()
26342634
LogPrint("net", "Flushed %d banned node ips/subnets to banlist.dat %dms\n",
26352635
banmap.size(), GetTimeMillis() - nStart);
26362636
}
2637+
2638+
int64_t PoissonNextSend(int64_t nNow, int average_interval_seconds) {
2639+
return nNow + (int64_t)(log1p(GetRand(1ULL << 48) * -0.0000000000000035527136788 /* -1/2^48 */) * average_interval_seconds * -1000000.0 + 0.5);
2640+
}

src/net.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ struct CNodeSignals
113113
{
114114
boost::signals2::signal<int ()> GetHeight;
115115
boost::signals2::signal<bool (CNode*), CombinerAll> ProcessMessages;
116-
boost::signals2::signal<bool (CNode*, bool), CombinerAll> SendMessages;
116+
boost::signals2::signal<bool (CNode*), CombinerAll> SendMessages;
117117
boost::signals2::signal<void (NodeId, const CNode*)> InitializeNode;
118118
boost::signals2::signal<void (NodeId)> FinalizeNode;
119119
};
@@ -391,13 +391,16 @@ class CNode
391391
CRollingBloomFilter addrKnown;
392392
bool fGetAddr;
393393
std::set<uint256> setKnown;
394+
int64_t nNextAddrSend;
395+
int64_t nNextLocalAddrSend;
394396

395397
// inventory based relay
396398
CRollingBloomFilter filterInventoryKnown;
397399
std::vector<CInv> vInventoryToSend;
398400
CCriticalSection cs_inventory;
399401
std::set<uint256> setAskFor;
400402
std::multimap<int64_t, CInv> mapAskFor;
403+
int64_t nNextInvSend;
401404
// Used for headers announcements - unfiltered blocks to relay
402405
// Also protected by cs_inventory
403406
std::vector<uint256> vBlockHashesToAnnounce;
@@ -791,4 +794,7 @@ class CBanDB
791794

792795
void DumpBanlist();
793796

797+
/** Return a timestamp in the future (in microseconds) for exponentially distributed events. */
798+
int64_t PoissonNextSend(int64_t nNow, int average_interval_seconds);
799+
794800
#endif // BITCOIN_NET_H

src/test/DoS_tests.cpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,19 +49,19 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
4949
CNode dummyNode1(INVALID_SOCKET, addr1, "", true);
5050
dummyNode1.nVersion = 1;
5151
Misbehaving(dummyNode1.GetId(), 100); // Should get banned
52-
SendMessages(&dummyNode1, false);
52+
SendMessages(&dummyNode1);
5353
BOOST_CHECK(CNode::IsBanned(addr1));
5454
BOOST_CHECK(!CNode::IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned
5555

5656
CAddress addr2(ip(0xa0b0c002));
5757
CNode dummyNode2(INVALID_SOCKET, addr2, "", true);
5858
dummyNode2.nVersion = 1;
5959
Misbehaving(dummyNode2.GetId(), 50);
60-
SendMessages(&dummyNode2, false);
60+
SendMessages(&dummyNode2);
6161
BOOST_CHECK(!CNode::IsBanned(addr2)); // 2 not banned yet...
6262
BOOST_CHECK(CNode::IsBanned(addr1)); // ... but 1 still should be
6363
Misbehaving(dummyNode2.GetId(), 50);
64-
SendMessages(&dummyNode2, false);
64+
SendMessages(&dummyNode2);
6565
BOOST_CHECK(CNode::IsBanned(addr2));
6666
}
6767

@@ -73,13 +73,13 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
7373
CNode dummyNode1(INVALID_SOCKET, addr1, "", true);
7474
dummyNode1.nVersion = 1;
7575
Misbehaving(dummyNode1.GetId(), 100);
76-
SendMessages(&dummyNode1, false);
76+
SendMessages(&dummyNode1);
7777
BOOST_CHECK(!CNode::IsBanned(addr1));
7878
Misbehaving(dummyNode1.GetId(), 10);
79-
SendMessages(&dummyNode1, false);
79+
SendMessages(&dummyNode1);
8080
BOOST_CHECK(!CNode::IsBanned(addr1));
8181
Misbehaving(dummyNode1.GetId(), 1);
82-
SendMessages(&dummyNode1, false);
82+
SendMessages(&dummyNode1);
8383
BOOST_CHECK(CNode::IsBanned(addr1));
8484
mapArgs.erase("-banscore");
8585
}
@@ -95,7 +95,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
9595
dummyNode.nVersion = 1;
9696

9797
Misbehaving(dummyNode.GetId(), 100);
98-
SendMessages(&dummyNode, false);
98+
SendMessages(&dummyNode);
9999
BOOST_CHECK(CNode::IsBanned(addr));
100100

101101
SetMockTime(nStartTime+60*60);

0 commit comments

Comments
 (0)