-
Notifications
You must be signed in to change notification settings - Fork 72
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
Add state to the validator-network and fix some network issues #2216
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
54835e8
to
6eaa7b3
Compare
hrxi
reviewed
Feb 21, 2024
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 found one potential issue, but the whole thing looks very good. :)
nibhar
reviewed
Feb 21, 2024
6eaa7b3
to
5006dd9
Compare
hrxi
reviewed
Feb 27, 2024
5006dd9
to
6a64ff1
Compare
Add some state to the validator `PeerId` cache to make less queries as possible to the DHT. This also makes the `validator-network` able to quickly return if the DHT query is still in progress allowing upper layers to continue their process without blocking them until the query finishes or errors out.
Fix the mishandling of `FinishedWithNoAdditionalRecord` since we could get such event even if the `query.finish` isn't called (which the network is calling once `Quorum` records have been received). This mishandling was causing some of the queries to hang indefinitely since no error nor result was being sent to the application layer. Now the network always pushes the result to the application layer once the `FinishedWithNoAdditionalRecord` is received.
Change the function that clears the validator `PeerId` to receive the request error such that it doesn't clear the cache if `InboundRequestError::NoReceiver` error was received. This error is an indicative that the requested peer has no receiver for such request which could happen when the peer is no running the specified aggregation and this is not an indicative of the `PeerId` cache entry being wrong. Thus in case of receiving this error, the network won't invalidate the cache entry for this validator.
Change the client code to use `3` as the default network quorum value. Also make the `devnet` scripts use this value for configuring the nodes.
6a64ff1
to
2522b42
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
3
as the default network quorum value3
as the default network quorum value.Also make the
devnet
scripts use this value for configuring thenodes.
PeerId
cache clearsPeerId
to receive the request error such that it doesn't clear the cache ifInboundRequestError::NoReceiver
error was received. This error is an indicative that the requested peer has no receiver for such request which could happen when the peer is no running the specified aggregation and this is not an indicative of thePeerId
cache entry being wrong. Thus in case of receiving this error, the network won't invalidate the cache entry for this validator.FinishedWithNoAdditionalRecord
FinishedWithNoAdditionalRecord
since we could get such event even if thequery.finish
isn't called (which the network is calling onceQuorum
records have been received).This mishandling was causing some of the queries to hang indefinitely since no error nor result was being sent to the application layer.
Now the network always pushes the result to the application layer once the
FinishedWithNoAdditionalRecord
is received.PeerId
cachePeerId
cache to make less queries as possible to the DHT.This also makes the
validator-network
able to quickly return if the DHT query is still in progress allowing upper layers to continue their process without blocking them until the query finishes or errors out.Pull request checklist
clippy
andrustfmt
warnings.