Fix typo incrementing instead of decrementing #60
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
|
Can you submit this upstream as well? |
Sure, here it is |
This bug requires a specific set of circumstances to trigger and become visible: 1. decrement a label that is non-zero but not first 2. repeat the decrement at least best_count()/2 times 3. increment that label 4. check the best count
teor2345
dismissed stale reviews from marc-aurele-besner and themself
via
0cf73c4
|
I pushed a commit with tests that check for this bug. Detailed reproduction steps are in the commit message, and the test comments. |
marc-aurele-besner
approved these changes
Dec 5, 2024
|
Can't approve but LGTM |
nazar-pc
approved these changes
Dec 6, 2024
|
Upstream has reviewed and approved it as well, so I think we can merge. Edit: upstream does squash merging, so I merged it that way to avoid merge conflicts. |
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.
This PR solves the label (network name) issue in Subspace Telemetry where Taurus testnet is sometimes labeled as Autonomys Mainnet.
The issue is that for determining the label of the network it's used a voting-like system in which when nodes start sending telemetry logs also send its network information (genesis hash, network name, etc.). For each genesis hash the network name that has been sent the most times is the one to be shown.
The issue was that when a node lost connection with telemetry instead of decreasing the number of votes from the network name they voted the service was increasing the number of votes, leading to a corruption of the voting. With the current fix, as long as the number of nodes sending the correct network name remains higher from those sending a wrong name, the correct label should be shown.
That being said this highlights an underlying issue in which some nodes are sending telemetry logs with the wrong network name but the correct genesis hash. I'll be looking for that error but this should solve the issue of wrong network label.