Skip to content
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

les, tests: fix les clientpool #22756

Merged
merged 3 commits into from
Apr 28, 2021
Merged

Conversation

rjl493456442
Copy link
Member

No description provided.

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Contributor

@zsfelfoldi zsfelfoldi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment, it's not 100% correct but works for our current use cases so it's safe to merge now to get rid of the fuzzer error.

@@ -239,12 +239,11 @@ func (cp *ClientPool) SetCapacity(node *enode.Node, reqCap uint64, bias time.Dur
maxTarget = curve.maxCapacity(func(capacity uint64) int64 {
return balance.estimatePriority(capacity, 0, 0, bias, false)
})
if maxTarget <= capacity {
if maxTarget < reqCap {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this is not really correct, with this change SetCapacity cannot do a partial capacity increase. maxTarget is the highest available capacity according to the curve based estimation. If it's somewhere between current capacity and reqCap then we should not exit, we should still attempt requestCapacity.
Still, since we don't use partial capacity increase yet (SetCapacity is only accessible from the API right now), you can merge this PR as it is if the fuzzer error is annoying and I'll figure out a really robust method later.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I don't know. Perhaps a simple solution for this scenario is that the server can respond the current available capacity to the client and client needs to re-request the capacity with a lower target.

But let's see how will this partial capacity work.

@holiman holiman merged commit 6d7c956 into ethereum:master Apr 28, 2021
@holiman holiman added this to the 1.10.3 milestone Apr 28, 2021
atif-konasl pushed a commit to frozeman/pandora-execution-engine that referenced this pull request Oct 15, 2021
* les, tests: fix les clientpool

* tests: disable debug mode

* les: polish code
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants