fix(node-health): total_miners sums replica counts and inflates the figure Nx - #8008
Open
Vyacheslav-Tomashevskiy wants to merge 1 commit into
Open
Conversation
…ure Nx The monitored attestation nodes are replicas of one shared ledger, so each /status reports the SAME network-wide miner count (ledger_verify and node_sync_validator both flag any divergence as a mismatch). get_network_health summed the per-node counts, so 3 online nodes each reporting 150 produced total_miners=450. Take the agreed value instead (max, robust to a mid-sync node reporting fewer). Adds a regression test.
github-actions
Bot
added
BCOS-L1
Owner
|
Right diagnosis (replicas of one ledger shouldn't be summed). Held pending two fixes: (1) CI is red because the pre-existing |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The bug
NodeHealthMonitor.get_network_healthreports the headline Total miners figure by summing each online node's count:But the monitored nodes (
DEFAULT_NODES, three:8088/:8099hosts) are replicas of one shared ledger. Each/statusreturns the network-wide active-miner count (miners/active_miners/miner_count), not a disjoint per-node slice. The sibling tools in this repo prove the intent — they treat any divergence between nodes as an anomaly:monitoring/ledger_verify.py→miner_count_match/MINER COUNT MISMATCHtools/node_sync_validator.py→miner_count_mismatchSo in the healthy case every online node agrees, and summing multiplies the true count by the number of online nodes.
Concrete input → wrong output: 3 online nodes each reporting
miners = 150(a network with 150 miners) → buggytotal_miners = 150 + 150 + 150 = 450; correct =150. The operator's table (print_table) and--jsonoutput always show a fabricated 2-3x miner population.The fix
Take the value the replicas agree on instead of summing.
maxis used so a node that is mid-sync and momentarily reporting fewer miners doesn't drag the figure down:Test
Added
tools/test_node_health_total_miners.py: 3 replicas → 150 (not 450); one lagging node (149) → 150; one offline → 150 withnodes_online=2; no miner data → 0 (no crash). All pass.Verified against
upstream/main@ c6501de.RTC:
RTCd1554f0f35576faf01d386a6be1c947f560dd0b7