-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Soft and hard ban #45
base: develop-1.10.8
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the difference of this PR to #44?
My bad, I forgot to push few files. Now it should look fine. |
cleanupInterval = 1 * time.Second | ||
en1, _ := enode.Parse(enode.ValidSchemes, "enode://969654fc30f10e45903ee2d107cfc165feaff6f9acbd2a9472120a3581f7c717ded76113aa116e35b8389dfcc9129ecfc85a409f6c46fc864ce077cb85316ac8@3.252.226.57:5050") | ||
BanStatic(en1.ID()) | ||
time.Sleep(2 * time.Second) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seeing that in the other PR you significantly reduced the Sleep time - wondering if you could do this here too?
cleanupInterval = 1 * time.Second | ||
en1, _ := enode.Parse(enode.ValidSchemes, "enode://969654fc30f10e45903ee2d107cfc165feaff6f9acbd2a9472120a3581f7c717ded76113aa116e35b8389dfcc9129ecfc85a409f6c46fc864ce077cb85316ac8@3.252.226.57:5050") | ||
BanDynamic(en1.ID(), 1*time.Second) // remove the entry after 1 second | ||
time.Sleep(2 * time.Second) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
func TestStaticBan(t *testing.T) { | ||
Enable() | ||
cleanupInterval = 1 * time.Second | ||
en1, _ := enode.Parse(enode.ValidSchemes, "enode://969654fc30f10e45903ee2d107cfc165feaff6f9acbd2a9472120a3581f7c717ded76113aa116e35b8389dfcc9129ecfc85a409f6c46fc864ce077cb85316ac8@3.252.226.57:5050") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this node exist? If so, could we maybe just use a fake enode? The test just checks the ID...
} | ||
|
||
func BannedDynamic(id enode.ID) bool { | ||
func BanDynamic(id enode.ID, banInterval time.Duration) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see this new function being used other than in the test, is that intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is used in #Fantom-foundation/go-opera#441
This PR adds support for Static and Dynamic bans.
A static ban means that the peer will b banned until the node is restarted.
The dynamic ban means that the peer will be banned for 4 hours.
This PR depends on #44.
Once #44 is merged, this PR should be rebased on top of that.