-
Notifications
You must be signed in to change notification settings - Fork 198
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
More antiflood fixes #1861
More antiflood fixes #1861
Conversation
iulianpascalau
commented
Jun 3, 2020
- added an out-of-specs flood preventer
- added antiflood debugger
- moved PeerID from p2p to core package
- replaced blackListHandler with peerBlackListHandler in antiflood package
- added the possibility to switch off the observer's antiflood system in system tests
- added antiflood debugger - moved PeerID from p2p to core package - replaced blackListHandler with peerBlackListHandler in antiflood package - added the possibility to switch off the observer's antiflood system in system tests
# Conflicts: # p2p/libp2p/networksharding/prioBitsSharder.go # p2p/libp2p/networksharding/prioBitsSharder_test.go
ResetForTopic(topic string) | ||
SetMaxMessagesForTopic(topic string, maxNum uint32) | ||
SetDebugger(debugger process.AntifloodDebugger) error |
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.
👍 a good example for injection via methods.
AttachDebugger
🎆
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.
Yes, the injection via method was done because a debugger is not a critical component.
@@ -85,6 +86,11 @@ updateNodeConfig() { | |||
cp nodesSetup_edit.json nodesSetup.json | |||
rm nodesSetup_edit.json | |||
|
|||
if [ $OBSERVERS_ANTIFLOOD_DISABLE -eq 1 ] | |||
then | |||
sed -i '/\[Antiflood\]/,/\[Logger\]/ s/true/false/' config_observer.toml |
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.
💥
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.
Yup, sed stuff :(
} | ||
|
||
return nil | ||
} | ||
|
||
func (af *p2pAntiflood) canProcessMessage(fp process.FloodPreventer, message p2p.MessageP2P, fromConnectedPeer p2p.PeerID) error { | ||
func (af *p2pAntiflood) recordDebugEvent(pid core.PeerID, topic string, numRejected uint32, sizeRejected uint64, isBlacklisted bool) { |
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.
👍
debug/antiflood/debugger.go
Outdated
mutCriticalArea sync.RWMutex | ||
cache storage.Cacher | ||
intervalAutoPrint time.Duration | ||
printEventHandler func(data string) |
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.
Naming a bit unfortunate (because we use the suffix handler
with other meaning usually) - could be printEventFunc
, eventPrinter
.
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.
printEventFunc shall be
|
||
assert.Equal(t, 1, d.cache.Len()) | ||
ev := d.GetData([]byte(string(pid) + topic)) | ||
require.NotNil(t, ev) |
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.
👍
# Conflicts: # dataRetriever/resolvers/topicResolverSender/export_test.go
- fixed mcl seldom failing test: Test_ScalarMulPkOK
928a23e
…rond-go into antiflood-fixes
# Conflicts: # factory/networkComponents_test.go # p2p/libp2p/netMessenger.go
p2p/mock/reconnecterStub.go
Outdated
return rs.ReconnectToNetworkCalled() | ||
} | ||
|
||
return make(chan struct{}, 1000) |
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.
why buffered?
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.
yes, why?
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.
made unbuffered
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.
ok
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.
👍