Add setban/listbanned RPC commands #6158

jonasschnelli · 2015-05-19T08:30:02Z

Groundwork for #5866.
If this makes it into master i'd like to add a GUI context menu for the peers table.
A simple disconnect (without banning) would be possible with setban <ip> add 1 (where 1 is the bantime).

At the moment the banned set does not survive a restart (should be added once).
Also currently banning is per IP and not per Node which results in disconnecting all nodes of a given IP (if the nodes uses the same ip but different ports).

Also includes some whitespace fixes for httpbasics.py test.

jonasschnelli · 2015-05-19T09:01:54Z

I'm open to alternatives (naming) for setban and listbanned. I thought adding banning options over the addnode command is a misuse.
I'm also not sure about banning whole IPs instead of only IP:port (node).

Diapolo · 2015-05-19T12:32:34Z

src/net.cpp

Why was/is this returning a bool, if it seems to be only true?

Right. I didn't want to change this in this PR and i kept it for legacy reasons and to lower the risk of breaking something.

Understood, but IMHO a function that doesn't need it could just be void. Perhaps you can just add a commit for that at the end? At least it could be changed for GetBanned as you just added it :).

Agreed. Added a commit on top.

jonasschnelli · 2015-05-19T15:09:10Z

Added a clearbanned command (also included in tests).

laanwj · 2015-05-21T10:52:49Z

Concept ACK, did not test or review code yet, this will be after 0.11 release

gmaxwell · 2015-05-22T06:46:32Z

Great; I had an old bitrotted version of this and had implemented almost exactly the same interface. One thing this can't accomplish though is banning netgroups. (thats actually what held up my code: I found issues with out netgroup parser that broke my tests)

LeMiner · 2015-05-22T10:00:28Z

In light of what gmaxwell said, perhaps allowing for -- setban 12. * .12.12 or setban 50.50.50. * would make sense as well. To allow for banning of entire octets.

laanwj · 2015-05-22T10:08:16Z

@LeMiner Good idea, but I'd say the interface should use /n or /x.y.z.w CIDR syntax (as parsed by CSubNet) instead of bringing back 0.9-era wildcards.

jonasschnelli · 2015-05-22T11:32:57Z

Agreed with @laanwj.
I don't think there is a use case for 1.x.2.3

LeMiner · 2015-05-22T11:39:54Z

Yep, agreed as well.

laanwj · 2015-05-22T12:26:40Z

1.x.2.3 in CIDR would be 1.0.2.3/255.0.255.255. But no, I don't see a use-case either.

jonasschnelli · 2015-05-26T07:55:02Z

Extended this PR to allow subnet banning/unbanning.
This needs testing because it changes the internal ban set from CNetAddr to CSubNet.

jgarzik · 2015-06-11T19:45:40Z

ut ACK

laanwj · 2015-06-12T14:53:09Z

Needs rebase.
I agree with @luke-jr that it should be somehow possible to specify an absolute ban time. Relative times are useful for end-users, but not so much for programmatic use.

jonasschnelli · 2015-06-12T16:34:24Z

Rebased and added the possibility of setting an absolute bantime with setban add <ip> <unixtimestamp> true.

laanwj · 2015-06-12T16:49:24Z

src/netbase.cpp

I think this should be:

return (a.network < b.network || (a.network == b.network && memcmp(a.netmask, b.netmask, 16) < 0));

Good catch! Thanks for the review.
Fixed.

jonasschnelli · 2015-06-16T16:22:11Z

Rebased and added also tests for the new disconnectnode command

9d79afe add RPC tests for setban & disconnectnode (Jonas Schnelli) 1f02b80 setban: add RPCErrorCode (Jonas Schnelli) d624167 fix CSubNet comparison operator (Jonas Schnelli) 4e36e9b setban: rewrite to UniValue, allow absolute bantime (Jonas Schnelli) 3de24d7 rename json field "bannedtill" to "banned_until" (Jonas Schnelli) 433fb1a [RPC] extend setban to allow subnets (Jonas Schnelli) e8b9347 [net] remove unused return type bool from CNode::Ban() (Jonas Schnelli) 1086ffb [QA] add setban/listbanned/clearbanned tests (Jonas Schnelli) d930b26 [RPC] add setban/listbanned/clearbanned RPC commands (Jonas Schnelli) 2252fb9 [net] extend core functionallity for ban/unban/listban (Jonas Schnelli)

jonasschnelli · 2015-06-18T14:51:10Z

Thanks for the merge. I now try to extend this to the UI peers window.

jonasschnelli force-pushed the 2015/05/rpc_ban branch 2 times, most recently from 9792d11 to ab10e9d Compare May 19, 2015 08:42

Diapolo reviewed May 19, 2015
View reviewed changes

jonasschnelli force-pushed the 2015/05/rpc_ban branch 3 times, most recently from f2a123b to 8339b26 Compare May 19, 2015 15:08

jonasschnelli force-pushed the 2015/05/rpc_ban branch 4 times, most recently from 83cd720 to 9f1394b Compare May 19, 2015 20:14

laanwj added Feature P2P labels May 20, 2015

jonasschnelli force-pushed the 2015/05/rpc_ban branch from 14a37e8 to 1a7ec8a Compare May 26, 2015 07:53

jonasschnelli force-pushed the 2015/05/rpc_ban branch 2 times, most recently from 12d6265 to 2eff5b8 Compare May 27, 2015 07:14

jonasschnelli mentioned this pull request Jun 9, 2015

Disconnect node on addnode remove #6259

Closed

jonasschnelli force-pushed the 2015/05/rpc_ban branch 2 times, most recently from d416fe3 to 83b0cc6 Compare June 12, 2015 16:32

laanwj reviewed Jun 12, 2015
View reviewed changes

jonasschnelli force-pushed the 2015/05/rpc_ban branch from 83b0cc6 to bb69fd8 Compare June 12, 2015 17:52

laanwj mentioned this pull request Jun 16, 2015

New RPC command disconnectnode #6271

Closed

jonasschnelli force-pushed the 2015/05/rpc_ban branch 2 times, most recently from 46d242a to 10da816 Compare June 16, 2015 16:13

jonasschnelli added 10 commits June 17, 2015 21:40

[net] extend core functionallity for ban/unban/listban

2252fb9

[RPC] add setban/listbanned/clearbanned RPC commands

d930b26

[QA] add setban/listbanned/clearbanned tests

1086ffb

[net] remove unused return type bool from CNode::Ban()

e8b9347

[RPC] extend setban to allow subnets

433fb1a

rename json field "bannedtill" to "banned_until"

3de24d7

setban: rewrite to UniValue, allow absolute bantime

4e36e9b

fix CSubNet comparison operator

d624167

setban: add RPCErrorCode

1f02b80

add RPC tests for setban & disconnectnode

9d79afe

jonasschnelli force-pushed the 2015/05/rpc_ban branch from f04a3ac to 9d79afe Compare June 17, 2015 19:52

laanwj merged commit 9d79afe into bitcoin:master Jun 18, 2015

str4d mentioned this pull request Feb 14, 2017

Bitcoin 0.12 RPC PRs 1 zcash/zcash#2100

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Add setban/listbanned RPC commands #6158

Add setban/listbanned RPC commands #6158

Uh oh!

Conversation

jonasschnelli commented May 19, 2015

Uh oh!

jonasschnelli commented May 19, 2015

Uh oh!

Diapolo May 19, 2015

Choose a reason for hiding this comment

Uh oh!

jonasschnelli May 19, 2015

Choose a reason for hiding this comment

Uh oh!

Diapolo May 19, 2015

Choose a reason for hiding this comment

Uh oh!

jonasschnelli May 19, 2015

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented May 19, 2015

Uh oh!

laanwj commented May 21, 2015

Uh oh!

gmaxwell commented May 22, 2015

Uh oh!

LeMiner commented May 22, 2015

Uh oh!

laanwj commented May 22, 2015

Uh oh!

jonasschnelli commented May 22, 2015

Uh oh!

LeMiner commented May 22, 2015

Uh oh!

laanwj commented May 22, 2015

Uh oh!

jonasschnelli commented May 26, 2015

Uh oh!

jgarzik commented Jun 11, 2015

Uh oh!

laanwj commented Jun 12, 2015

Uh oh!

jonasschnelli commented Jun 12, 2015

Uh oh!

laanwj Jun 12, 2015

Choose a reason for hiding this comment

Uh oh!

jonasschnelli Jun 12, 2015

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Jun 16, 2015

Uh oh!

jonasschnelli commented Jun 18, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants