Return of the Banman

dongcarl commented at 5:39 am on October 30, 2018: member

Old English à la Beowulf

0Banman wæs bréme    --blaéd wíde sprang--
1Connmanes eafera    Coreum in.
2aéglaéca            léodum forstandan
3Swá bealdode        bearn Connmanes
4guma gúðum cúð      gódum daédum·
5dréah æfter dóme·   nealles druncne slóg

Modern English Translation

0Banman was famed              --his renown spread wide--
1Conman's hier,                in Core-land.
2against the evil creature     defend the people
3Thus he was bold,             the son of Connman
4man famed in war,             for good deeds;
5he led his life for glory,    never, having drunk, slew

–

With @theuni’s blessing, here is Banman, rebased. Original PR: #11457

–

Followup PRs:

Give CNode a Disconnect method (source)
Add a comment to std::atomic_bool fDisconnect in net.h that setting this to true will cause the node to be disconnected the next time DisconnectNodes() runs (source)

fanquake added the label P2P on Oct 30, 2018

DrahtBot commented at 6:19 am on October 30, 2018: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#14987 (RPCHelpMan: Pass through Result and Examples by MarcoFalke)
#14929 (net: Allow connections from misbehavior banned peers by gmaxwell)
#14897 (randomize GETDATA(tx) request order and introduce bias toward outbound by naumenkogs)
#14856 (net: remove more CConnman globals (theuni) by dongcarl)
#14464 (refactor: make checkqueue manage the threads by itself (also removed some boost dependencies) by ken2812221)
#10785 (Serialization improvements by sipa)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

fanquake added this to the "In progress" column in a project

in src/qt/rpcconsole.cpp:1231 in af2c654832 outdated

1232-	// Find possible nodes, ban it and clear the selected node
1233-	const CNodeCombinedStats *stats = clientModel->getPeerTableModel()->getNodeStats(detailNodeRow);
1234-	if(stats) {
1235+    // Find possible nodes, ban it and clear the selected node
1236+    const CNodeCombinedStats *stats = clientModel->getPeerTableModel()->getNodeStats(detailNodeRow);
1237+    if(stats) {

MarcoFalke commented at 12:22 pm on October 30, 2018:

Accidental changes in the first commit?

in src/addrdb.cpp:108 in af2c654832 outdated

104@@ -105,9 +105,8 @@ bool DeserializeFileDB(const fs::path& path, Data& data)
105 
106 }
107 
108-CBanDB::CBanDB()
109+CBanDB::CBanDB(fs::path ban_file) : pathBanlist(std::move(ban_file))

practicalswift commented at 2:35 pm on October 30, 2018:

Make sure parameter name matches between declaration and definition :-)

in src/banman.h:39 in af2c654832 outdated

34+class BanMan
35+{
36+public:
37+    ~BanMan();
38+    BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time);
39+    void Ban(const CNetAddr& netAddr, const BanReason& reason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false);

practicalswift commented at 2:35 pm on October 30, 2018:

Make sure parameter names match between:

0    void Ban(const CNetAddr& netAddr, const BanReason& reason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false);
1    void BanMan::Ban(const CNetAddr& addr, const BanReason &banReason, int64_t bantimeoffset, bool sinceUnixEpoch) {

in src/banman.h:40 in af2c654832 outdated

35+{
36+public:
37+    ~BanMan();
38+    BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time);
39+    void Ban(const CNetAddr& netAddr, const BanReason& reason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false);
40+    void Ban(const CSubNet& subNet, const BanReason& reason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false);

practicalswift commented at 2:36 pm on October 30, 2018:

Make sure these match:

0    void Ban(const CSubNet& subNet, const BanReason& reason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false);
1    void BanMan::Ban(const CSubNet& subNet, const BanReason &banReason, int64_t bantimeoffset, bool sinceUnixEpoch) {

in src/banman.h:44 in af2c654832 outdated

39+    void Ban(const CNetAddr& netAddr, const BanReason& reason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false);
40+    void Ban(const CSubNet& subNet, const BanReason& reason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false);
41+    void ClearBanned();
42+    bool IsBanned(CNetAddr ip);
43+    bool IsBanned(CSubNet subnet);
44+    bool Unban(const CNetAddr &ip);

practicalswift commented at 2:36 pm on October 30, 2018:

Make sure these match:

0    bool Unban(const CNetAddr &ip);
1    bool BanMan::Unban(const CNetAddr &addr) {

in src/banman.h:45 in af2c654832 outdated

40+    void Ban(const CSubNet& subNet, const BanReason& reason, int64_t bantimeoffset = 0, bool sinceUnixEpoch = false);
41+    void ClearBanned();
42+    bool IsBanned(CNetAddr ip);
43+    bool IsBanned(CSubNet subnet);
44+    bool Unban(const CNetAddr &ip);
45+    bool Unban(const CSubNet &ip);

practicalswift commented at 2:36 pm on October 30, 2018:

Same here:

0    bool Unban(const CSubNet &ip);
1    bool BanMan::Unban(const CSubNet &subNet) {

in src/banman.h:46 in af2c654832 outdated

41+    void ClearBanned();
42+    bool IsBanned(CNetAddr ip);
43+    bool IsBanned(CSubNet subnet);
44+    bool Unban(const CNetAddr &ip);
45+    bool Unban(const CSubNet &ip);
46+    void GetBanned(banmap_t &banmap);

practicalswift commented at 2:37 pm on October 30, 2018:

Same here:

0    void GetBanned(banmap_t &banmap);
1    void BanMan::GetBanned(banmap_t &banMap)

in src/banman.h:50 in af2c654832 outdated

45+    bool Unban(const CSubNet &ip);
46+    void GetBanned(banmap_t &banmap);
47+    void DumpBanlist();
48+
49+private:
50+    void SetBanned(const banmap_t &banmap);

practicalswift commented at 2:37 pm on October 30, 2018:

And here :-)

0    void SetBanned(const banmap_t &banmap);
1    void BanMan::SetBanned(const banmap_t &banMap)

in src/banman.cpp:139 in af2c654832 outdated

134+}
135+
136+bool BanMan::Unban(const CSubNet &subNet) {
137+    {
138+        LOCK(m_cs_banned);
139+        if (!m_banned.erase(subNet))

practicalswift commented at 2:40 pm on October 30, 2018:

Nit: if (m_banned.erase(subNet) == 0) to increase readability and avoid implicit conversion?

in src/banman.cpp:68 in af2c654832 outdated

63+        LOCK(m_cs_banned);
64+        m_banned.clear();
65+        m_is_dirty = true;
66+    }
67+    DumpBanlist(); //store banlist to disk
68+    if(m_client_interface)

practicalswift commented at 2:47 pm on October 30, 2018:

Since this is new code - please run all new C++ files through clang-format (using the project local settings) to get the expected formatting.

in src/banman.cpp:111 in af2c654832 outdated

106+    CBanEntry banEntry(GetTime());
107+    banEntry.banReason = banReason;
108+    if (bantimeoffset <= 0)
109+    {
110+        bantimeoffset = m_default_ban_time;
111+        sinceUnixEpoch = false;

practicalswift commented at 8:55 pm on October 30, 2018:

Could this be re-formulated to avoid reassigning parameters?

dongcarl commented at 9:03 pm on October 31, 2018:

Reformulated in https://github.com/bitcoin/bitcoin/pull/14605/commits/36e6b241d0ed6f6d6ab503d61dfd95053202ce9b, let me know if you think that’s good

practicalswift commented at 9:07 pm on November 15, 2018:

@dongcarl Nice! Thanks! :-)

dongcarl force-pushed on Oct 31, 2018

laanwj commented at 5:14 pm on October 31, 2018: member

thank you for picking this up! We welcome the Ban Man to our kingdom

laanwj added this to the milestone 0.18.0 on Oct 31, 2018

dongcarl force-pushed on Nov 1, 2018

laanwj commented at 1:10 pm on November 6, 2018: member

No merge conflicts, but it does need rebase [#14555]; current build fails wiith

0/.../bitcoin/src/banman.cpp:10:10: fatal error: util.h: No such file or directory
1 #include <util.h>
2          ^~~~~~~~
3compilation terminated.

dongcarl force-pushed on Nov 6, 2018

dongcarl commented at 4:59 pm on November 6, 2018: member

Done.

in src/banman.cpp:77 in 157684a341 outdated

72+bool BanMan::IsBanned(CNetAddr net_addr)
73+{
74+    LOCK(m_cs_banned);
75+    for (const auto& it : m_banned) {
76+        CSubNet sub_net = it.first;
77+        CBanEntry banEntry = it.second;

kostyantyn commented at 5:25 pm on November 6, 2018:

since it’s a new class and it mostly everywhere follows snake_case notation, WDYS to stick to it and in a few other places?

theuni commented at 9:56 pm on December 4, 2018:

@kostyantyn That’d be fine in a follow-up PR, but the primary interest here is renaming the member variables so that they’re immediately distinguishable.

DrahtBot added the label Needs rebase on Nov 15, 2018

dongcarl force-pushed on Nov 15, 2018

DrahtBot removed the label Needs rebase on Nov 15, 2018

dongcarl commented at 2:04 am on November 17, 2018: member

Rebased, and addressed casing concerns. Additional review welcome.

DrahtBot added the label Needs rebase on Nov 28, 2018

dongcarl force-pushed on Nov 28, 2018

DrahtBot removed the label Needs rebase on Nov 28, 2018

in src/init.cpp:1287 in 701702427e outdated

1282@@ -1278,11 +1283,12 @@ bool AppInitMain(InitInterfaces& interfaces)
1283     // is not yet setup and may end up being set up twice if we
1284     // need to reindex later.
1285 
1286+    assert(!g_banman);
1287+    g_banman = std::unique_ptr<BanMan>(new BanMan(&uiInterface));

theuni commented at 9:27 pm on December 4, 2018:

This can now use MakeUnique.

ryanofsky commented at 9:35 pm on December 4, 2018: member

I’d like to review this soon, but is there a description of what high level goal of this PR is? Is it cleanup geared towards making code maintenance easier? Or is it intended to enable a new feature? Is it cleaning up technical debt from a previously added feature? The previous PR #14605 just says it “breaks the ban/unban functions out of CConnman and into a new class because, while logically bans are tied to connections, they’re really just entries in a database,” which I think justifies the change but doesn’t really motivate it.

in src/interfaces/node.cpp:126 in 701702427e outdated

119@@ -120,24 +120,24 @@ class NodeImpl : public Node
120     }
121     bool getBanned(banmap_t& banmap) override
122     {
123-        if (g_connman) {
124-            g_connman->GetBanned(banmap);
125+        if (g_banman) {

theuni commented at 9:39 pm on December 4, 2018:

Note for follow-up PR:

connman and banman pointers should be passed into NodeImpl’s constructor. That would eliminate the use of globals here.

Sjors commented at 6:06 pm on January 16, 2019:

cc @ryanofsky

in src/banman.cpp:111 in 5090c8c9fd outdated

105@@ -106,11 +106,14 @@ void BanMan::Ban(const CSubNet& subNet, const BanReason& banReason, int64_t bant
106 {
107     CBanEntry banEntry(GetTime());
108     banEntry.banReason = banReason;
109+
110+    int64_t normalized_bantimeoffset = bantimeoffset;
111+    int64_t normalized_sinceUnixEpoch = sinceUnixEpoch;

theuni commented at 10:25 pm on December 4, 2018:

normalized_sinceUnixEpoch should be a bool.

in src/banman.cpp:28 in 9bad9fa116 outdated

24@@ -25,7 +25,7 @@ BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t
25         SweepBanned();            // sweep out unused entries
26 
27         LogPrint(BCLog::NET, "Loaded %d banned node ips/subnets from banlist.dat  %dms\n",
28-            banmap.size(), GetTimeMillis() - nStart);

theuni commented at 10:27 pm on December 4, 2018:

Hungarian or snake_case, pick one :)

dongcarl commented at 1:56 pm on December 5, 2018:

Fixed in 9bad9fa1168ec53e92bb8b2df0b5e9d578344a2f, unless this should be squashed in

in src/banman.cpp:108 in 9bad9fa116 outdated

123-    int64_t normalized_sinceUnixEpoch = sinceUnixEpoch;
124-    if (bantimeoffset <= 0) {
125-        normalized_bantimeoffset = m_default_ban_time;
126-        normalized_sinceUnixEpoch = false;
127+    CBanEntry ban_entry(GetTime());
128+    ban_entry.banReason = ban_reason;

theuni commented at 10:38 pm on December 4, 2018:

Please don’t use special-cases in scripted-diffs to negate part of a change, as that means that manual review is required.

Instead, please add a commit before the scripted-diff which renames any necessary conflicts.

dongcarl commented at 1:54 pm on December 5, 2018:

Do you mean rename CBanEntry.banReason to CBanEntry.m_ban_reason before the scripted diff?

theuni commented at 10:27 pm on December 6, 2018:

How about just first adding a commit to hack around it so that we don’t have to use the var by name?

 0diff --git a/src/addrdb.h b/src/addrdb.h
 1index 90eca44..f50442f 100644
 2--- a/src/addrdb.h
 3+++ b/src/addrdb.h
 4@@ -43,6 +43,11 @@ public:
 5         nCreateTime = nCreateTimeIn;
 6     }
 7
 8+    explicit CBanEntry(int64_t nCreateTimeIn, BanReason banReasonIn) : CBanEntry(nCreateTimeIn)
 9+    {
10+        banReason = banReasonIn;
11+    }
12+
13     ADD_SERIALIZE_METHODS;
14
15     template <typename Stream, typename Operation>
16diff --git a/src/net.cpp b/src/net.cpp
17index fde85b0..db97ae8 100644
18--- a/src/net.cpp
19+++ b/src/net.cpp
20@@ -540,8 +540,7 @@ void CConnman::Ban(const CNetAddr& addr, const BanReason &banReason, int64_t ban
21 }
22
23 void CConnman::Ban(const CSubNet& subNet, const BanReason &banReason, int64_t bantimeoffset, bool sinceUnixEpoch) {
24-    CBanEntry banEntry(GetTime());
25-    banEntry.banReason = banReason;
26+    CBanEntry banEntry(GetTime(), banReason);
27     if (bantimeoffset <= 0)
28     {
29         bantimeoffset = gArgs.GetArg("-bantime", DEFAULT_MISBEHAVING_BANTIME);

After that, I believe the scripted diff should work without hacks.

theuni commented at 10:46 pm on December 4, 2018: member

@dongcarl Thanks for picking this up! Added a few nits.

theuni commented at 10:49 pm on December 4, 2018: member

@ryanofsky This is one step (one of the big ones) closer to instantiated CConnman (self-contained and no-globals), which would allow for all kinds of cool things, primarily running separate net instances against each-other.

Another nice side-effect of the above is that once self-contained, bitcoind could be run (or even compiled!) without net support.

Edit: See my comment here as an example of next steps: #14605 (review)

DrahtBot added the label Needs rebase on Dec 13, 2018

dongcarl force-pushed on Jan 3, 2019

dongcarl commented at 3:19 pm on January 3, 2019: member

Rebased and addressed reviews!

DrahtBot removed the label Needs rebase on Jan 3, 2019

in src/addrdb.h:48 in bc526e9e98 outdated

42@@ -43,6 +43,11 @@ class CBanEntry
43         nCreateTime = nCreateTimeIn;
44     }
45 
46+    explicit CBanEntry(int64_t n_create_time_in, BanReason ban_reason_in) : CBanEntry(n_create_time_in)
47+    {
48+        banReason = ban_reason_in;

Empact commented at 10:48 am on January 9, 2019:

nit: could be initializer

dongcarl commented at 7:43 pm on January 15, 2019:

I don’t think this can be initializer as we need to SetNull(), the current one does the delegation thing.

in src/addrdb.h:100 in bc526e9e98 outdated

 96@@ -92,9 +97,9 @@ class CAddrDB
 97 class CBanDB
 98 {
 99 private:
100-    fs::path pathBanlist;
101+    fs::path m_ban_list_path;

Empact commented at 10:48 am on January 9, 2019:

nit: could be const

in src/banman.cpp:14 in bc526e9e98 outdated

 9+#include <ui_interface.h>
10+#include <util/system.h>
11+#include <util/time.h>
12+
13+
14+BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time) : m_client_interface(client_interface), m_ban_db(std::move(ban_file)), m_default_ban_time(default_ban_time)

Empact commented at 10:49 am on January 9, 2019:

nit: could wrap this line

in src/banman.cpp:17 in bc526e9e98 outdated

12+
13+
14+BanMan::BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time) : m_client_interface(client_interface), m_ban_db(std::move(ban_file)), m_default_ban_time(default_ban_time)
15+{
16+    if (m_client_interface)
17+        m_client_interface->InitMessage(_("Loading banlist..."));

Empact commented at 10:50 am on January 9, 2019:

nit: one line or braces, here and a few other places

in src/banman.h:65 in bc526e9e98 outdated

57+    CCriticalSection m_cs_banned;
58+    banmap_t m_banned GUARDED_BY(m_cs_banned);
59+    bool m_is_dirty GUARDED_BY(m_cs_banned);
60+    CClientUIInterface* m_client_interface = nullptr;
61+    CBanDB m_ban_db;
62+    const int64_t m_default_ban_time;

Empact commented at 11:04 am on January 9, 2019:

nit: #include <cstdint>

in src/banman.h:68 in bc526e9e98 outdated

60+    CClientUIInterface* m_client_interface = nullptr;
61+    CBanDB m_ban_db;
62+    const int64_t m_default_ban_time;
63+};
64+
65+extern std::unique_ptr<BanMan> g_banman;

Empact commented at 11:04 am on January 9, 2019:

nit: #include <memory>

in src/net.cpp:2482 in bc526e9e98 outdated

2477+bool CConnman::DisconnectNode(const CSubNet& subnet)
2478+{
2479+    bool disconnected = false;
2480+    LOCK(cs_vNodes);
2481+    for (CNode* pnode : vNodes) {
2482+        if (subnet.Match(static_cast<CNetAddr>(pnode->addr))) {

Empact commented at 11:14 am on January 9, 2019:

Seems dynamic_cast would be more appropriate here, given the cast to base class.

Edit: maybe not:

A downcast can also be performed with static_cast, which avoids the cost of the runtime check, but it’s only safe if the program can guarantee (through some other logic) that the object pointed to by expression is definitely Derived. https://en.cppreference.com/w/cpp/language/dynamic_cast

Maybe a static_assert is in order to enforce?

dongcarl commented at 8:28 pm on January 15, 2019:

We are upcasting from CAddress to its base class of CNetAddr here, so it wouldn’t need a static_assert I believe.

Since we are just upcasting, we don’t even need casting here, so I’ll just remove it unless there are obscure C++ reasons for doing so.

laanwj commented at 3:18 pm on January 16, 2019:

I’d advise against using dynamic_cast. It’s not used anywhere else in the code and it relies on C++ run-time type information which has at least historically a large overhead. I’m also fairly sure it’s unnecessary here.

in src/net_processing.cpp:2967 in bc526e9e98 outdated

2969-            {
2970-                connman->Ban(pnode->addr, BanReasonNodeMisbehaving);
2971+        else if (pnode->addr.IsLocal()) {
2972+            // Disconnect but don't ban _this_ local node
2973+            LogPrintf("Warning: disconnecting but not banning local peer %s!\n", pnode->addr.ToString());
2974+            connman->DisconnectNode(pnode->GetId());

Empact commented at 11:23 am on January 9, 2019:

nit: could have a CNode* version of DisconnectNode to avoid the lookup

dongcarl commented at 9:17 pm on January 15, 2019:

I’m going to revert to doing

0pnode->fDisconnect = true;

so as to avoid the lookup, then in a followup PR we can make fDisconnect private, and have a Disconnect method on CNode.

Empact commented at 11:28 am on January 9, 2019: member

Concept ACK

DrahtBot added the label Needs rebase on Jan 9, 2019

laanwj commented at 2:53 pm on January 15, 2019: member

We want to have this in for 0.18.0, there’s not that much time anymore. This has had a fair amount of review. Let’s rebase, address the nits, do a last review round, and merge it.

dongcarl commented at 6:35 pm on January 15, 2019: member

@laanwj Yes. I will make it happen this week.

dongcarl force-pushed on Jan 15, 2019

dongcarl commented at 10:53 pm on January 15, 2019: member

Rebased and addressed nits.

DrahtBot removed the label Needs rebase on Jan 15, 2019

laanwj commented at 12:52 pm on January 16, 2019: member

Thank you @dongkarl!

laanwj commented at 1:04 pm on January 16, 2019: member

utACK cb1ee04b6a7d56834a6447711ca5500fc8bd83cd

Original PR had utACKs by me, @ryanofsky, @jimpo. Please re-review so that we can merge this!

net: Break disconnecting out of Ban()

These are separate events which need to be carried out by separate subsystems.

This also cleans up some whitespace and tabs in qt to avoid getting flagged by
the linter.

Current behavior is preserved.

7cc2b9f678

tests: remove member connman/peerLogic in TestingSetup 136bd7926c

in src/net_processing.cpp:2971 in b4ca7e2933 outdated

2971+                // Disconnect but don't ban _this_ local node
2972+                LogPrintf("Warning: disconnecting but not banning local peer %s!\n", pnode->addr.ToString());
2973+            } else {
2974+                // Disconnect and ban all nodes sharing the address
2975                 connman->Ban(pnode->addr, BanReasonNodeMisbehaving);
2976             }

promag commented at 4:50 pm on January 16, 2019:

Commit b4ca7e2

Discussed with @dongcarl about not changing behavior in this commit, the conclusion is to call conman->DisconnectNode(pnode->addr) here and add pnode->fDisconnect = true; above.

dongcarl force-pushed on Jan 16, 2019

in src/net.cpp:2646 in 7cc2b9f678 outdated

2641+{
2642+    bool disconnected = false;
2643+    LOCK(cs_vNodes);
2644+    for (CNode* pnode : vNodes) {
2645+        if (subnet.Match(pnode->addr)) {
2646+            pnode->fDisconnect = true;

Sjors commented at 5:45 pm on January 16, 2019:

Maybe add a comment to std::atomic_bool fDisconnect in net.h that setting this to true will cause the node to be disconnected the next time DisconnectNodes() runs.

dongcarl commented at 6:54 pm on January 16, 2019:

I think this belongs in another PR, but happy to do that PR :-)

in src/net.cpp:45 in 9db3a772fc outdated

40@@ -41,8 +41,11 @@
41 
42 #include <math.h>
43 
44-// Dump addresses to peers.dat and banlist.dat every 15 minutes (900s)
45-#define DUMP_ADDRESSES_INTERVAL 900
46+// Dump addresses to peers.dat every 15 minutes (900s)
47+static constexpr int DUMP_PEERS_INTERVAL = 60 * 15;

Sjors commented at 6:01 pm on January 16, 2019:

Nit: 15 * 60 to be consistent with other _INTERVAL constants.

dongcarl commented at 6:55 pm on January 16, 2019:

Done!

Sjors approved

Sjors commented at 6:22 pm on January 16, 2019: member

tACK 95b495b9697502b2b478dda8cece0ae9ffb6818f @ryanofsky wrote:

is there a description of what high level goal of this PR is? Is it cleanup geared towards making code maintenance easier? […]

I think reducing the 3000 lines in net.cpp by 200 is almost worth it by itself :-)

From the original description in #11457:

This also makes testing easier as different implementations can be dropped in.

So this is a step towards being able to mock network code and e.g. test banning behavior for a large number of simulated nodes.

net: split up addresses/ban dumps in preparation for moving them 83c1ea2e5e

banman: create and split out banman

Some say he has always been.

4c0d961eb0

banman: pass the banfile path in

There's no need to hard-code the path here. Passing it in means that there are
no ordering concerns wrt establishing the datadir.

2e56702ece

banman: pass in default ban time as a parameter

Removes the dependency on arg parsing.

d0469b2e93

net: move BanMan to its own files af3503d903

scripted-diff: batch-rename BanMan members

-BEGIN VERIFY SCRIPT-
sed -i "s/clientInterface/m_client_interface/g" src/banman.h src/banman.cpp
sed -i "s/setBannedIsDirty/m_is_dirty/g" src/banman.h src/banman.cpp
sed -i "s/cs_setBanned/m_cs_banned/g" src/banman.h src/banman.cpp
sed -i "s/setBanned/m_banned/g" src/banman.h src/banman.cpp
-END VERIFY SCRIPT-

84fc3fbd03

banman: add thread annotations and mark members const where possible

Also remove misleading comment. ClearBanned is used by rpc as well.

daae598feb

banman: reformulate nBanUtil calculation

Avoid reassigning parameters.

1ffa4ce27d

banman: Add, use CBanEntry ctor that takes ban reason c2e04d37f3

scripted-diff: batch-recase BanMan variables

-BEGIN VERIFY SCRIPT-
sed -i "s/banMap/banmap/g" src/banman.h src/banman.cpp
sed -i "s/netAddr/net_addr/g" src/banman.h src/banman.cpp
sed -i "s/sinceUnixEpoch/since_unix_epoch/g" src/banman.h src/banman.cpp
sed -i "s/bantimeoffset/ban_time_offset/g" src/banman.h src/banman.cpp
sed -i "s/subNet/sub_net/g" src/banman.h src/banman.cpp
sed -i "s/banReason/ban_reason/g" src/banman.h src/banman.cpp
sed -i "s/notifyUI/notify_ui/g" src/banman.h src/banman.cpp
sed -i "s/banEntry/ban_entry/g" src/banman.h src/banman.cpp
sed -i "s/nStart/n_start/g" src/banman.h src/banman.cpp
-END VERIFY SCRIPT-

18185b57c3

dongcarl force-pushed on Jan 16, 2019

Sjors commented at 7:10 pm on January 16, 2019: member

re-tACK 18185b57

laanwj referenced this in commit 978682b9dc on Jan 20, 2019

laanwj commented at 5:58 pm on January 21, 2019: member

re-utACK 18185b57c32d0a43afeca4c125b9352c692923e9

laanwj merged this on Jan 21, 2019

laanwj closed this on Jan 21, 2019

laanwj referenced this in commit 5baa9092c4 on Jan 21, 2019

fanquake moved this from the "In progress" to the "Done" column in a project

deadalnix referenced this in commit 47f476c061 on Jun 9, 2020

PastaPastaPasta referenced this in commit 38ee2a7a94 on Jul 18, 2021

PastaPastaPasta referenced this in commit 7cf87fe383 on Jul 18, 2021

Munkybooty referenced this in commit 8edd40fcff on Aug 21, 2021

Munkybooty referenced this in commit 27c483e6fd on Aug 23, 2021

Munkybooty referenced this in commit aceea13f9f on Aug 24, 2021

Munkybooty referenced this in commit 9845e85bcd on Aug 24, 2021

Munkybooty referenced this in commit 53a6f62ea4 on Aug 24, 2021

UdjinM6 referenced this in commit 45c54cb89d on Aug 24, 2021

Munkybooty referenced this in commit 30f6319fd0 on Aug 24, 2021

MarcoFalke locked this on Dec 16, 2021

Return of the Banman #14605

Conflicts