p2p: more CR fixes: ERL and DHT err logging #6040
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/p2p #6040 +/- ##
===============================================
+ Coverage 55.95% 56.09% +0.14%
===============================================
Files 482 488 +6
Lines 68287 69544 +1257
===============================================
+ Hits 38213 39014 +801
- Misses 27473 27858 +385
- Partials 2601 2672 +71 ☔ View full report in Codecov by Sentry. |
data/txHandler.go
Outdated
| // this TX message was found in the duplicate cache, or ERL rate-limited it | ||
| return network.ValidatedMessage{Action: network.Ignore, ValidatedMessage: nil} | ||
| // note, this is the same check as in incomingMsgDupErlCheck prologue | ||
| // but for p2p network handlers no ERL needed because libp2p has own congestion control |
There was a problem hiding this comment.
maybe we could split incomingMsgDupErlCheck into dupCheck and erlCheck? the two halves don't have any relation to each other AFAICT
There was a problem hiding this comment.
IMO it does not make much sense - a new function would be a single call inside two if-conds by cost of adding extra lines in caller to check return codes.
the two halves don't have any relation to each other AFAICT
they do (both signal if a messages should be rejected with the same code) and they don't since use different message traits to decide on.
There was a problem hiding this comment.
Semi-orthogonal, do we understand the rate limiting behavior of libp2p vs our own implementation? It's reasonable to want to use that, but we should at least understand how they each perform relative to one another no?
|
Fix for the logging failure is in #6035 |
Summary
dht.Close()errors if any.Test Plan
Existing tests