clamping sync requests #1273
clamping sync requests #1273
Conversation
synzhu
requested review from
huitseeker,
vishalchangrani and
yhassanzadeh13
There was a problem hiding this comment.
Thanks a bunch for getting to this fast!
I would love a code comment on each of the two checks making it explicit we enforce existing client-side limits, and that this therefore shouldn't have an effect on a well-behaving client.
Without that, another later reader, may not find it obvious those lines are meant to check for mischief.
|
One more thing: |
|
Addressed comments |
| }, | ||
| ) | ||
| err = ss.e.requestHandler.onRangeRequest(originID, req) | ||
| require.NoError(ss.T(), err, "valid range request should pass") |
There was a problem hiding this comment.
Nit: do we want to call this "valid"?
| }, | ||
| ) | ||
| err = ss.e.requestHandler.onBatchRequest(originID, req) | ||
| require.NoError(ss.T(), err, "should pass request with valid blocks") |
There was a problem hiding this comment.
Nit: do we want to call this "valid"?
Codecov Report
@@ Coverage Diff @@
## master #1273 +/- ##
==========================================
+ Coverage 54.59% 54.61% +0.01%
==========================================
Files 498 498
Lines 31533 31538 +5
==========================================
+ Hits 17217 17223 +6
Misses 11968 11968
+ Partials 2348 2347 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
bors merge |
closes #https://github.com/dapperlabs/flow-go/issues/5836