Check connected peer weight #213
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
cam-schultz
requested review from
michaelkaplan13,
minghinmatthewlam,
gwen917,
geoff-vball and
bernard-avalabs
as code owners
geoff-vball
previously approved these changes
Mar 5, 2024
geoff-vball
previously approved these changes
Mar 5, 2024
michaelkaplan13
previously approved these changes
Mar 5, 2024
geoff-vball
reviewed
Mar 5, 2024
| "Failed to connect to a threshold of stake", | ||
| zap.Uint64("connectedWeight", connectedWeight), | ||
| zap.Uint64("totalValidatorWeight", totalValidatorWeight), | ||
| zap.Any("warpQuorum", r.warpQuorum), |
There was a problem hiding this comment.
err referenced on lines 268 and 270 is no longer referencing a relevant error.
geoff-vball
approved these changes
Mar 5, 2024
michaelkaplan13
approved these changes
Mar 5, 2024
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.
Why this should be merged
Currently, we abandon message relaying if we fail to connect to any of the subnet validators. However, the aggregate signature only needs a quorum of stake weight to be valid, so this constraint is overly restrictive.
This change relaxes that constraint by attempting to connect to all peers, then comparing the total connected stake against the quorum value of the destination blockchain.
How this works
AppRequestNetwork.ConnectPeersreturns the set of connected peers, and no error. It is up to the caller to determine if the returned value constitutes an error. The message relayer uses the connected stake to determine whether or not to proceed with relaying.How this was tested
CI
How is this documented
N/A