[tontester] fix consensus explorer overflow for skipped slots, add leader stats - #2482
[tontester] fix consensus explorer overflow for skipped slots, add leader stats#2482yungwine wants to merge 7 commits into
Conversation
yungwine
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 136999e3e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for slot, collator in pairs: | ||
| if collator != first_collator: | ||
| leader_window = slot | ||
| break |
There was a problem hiding this comment.
When --logs contains a truncated session or --stats-dir has retained only files beginning mid-group, the first collator transition occurs at an absolute slot such as 104 even when the leader window is 4. Assigning that slot number as the window makes the verification fail, so analyze_group() silently omits an otherwise valid group and the group API reports it as not found.
Useful? React with 👍 / 👎.
| collators_seen: set[int] = set() | ||
| for _, collator in pairs: | ||
| collators_seen.add(collator) | ||
|
|
||
| num_validators = len(collators_seen) |
There was a problem hiding this comment.
For a current or partially retained group that has not yet exposed every collator, len(collators_seen) is only the number of validators observed so far, not the group's validator count. For example, the first 26 slots of a 100-validator group with a four-slot window are accepted as a seven-validator group, causing the API and aggregate output to omit the other validators and publish an incorrect total.
Useful? React with 👍 / 👎.
| if leader_window is None: | ||
| return None, None |
There was a problem hiding this comment.
A valid single-validator consensus group has the same collator for every slot, so this transition-based inference always leaves leader_window unset. Such groups are therefore discarded by analyze_group() even when they contain complete slot and finalization data, which breaks leader statistics for common single-node test configurations.
Useful? React with 👍 / 👎.
| # Walk parent chains from each finalized slot to reconstruct the | ||
| # finalized history. Visited slots are finalized (non-empty), slots | ||
| # skipped over between parent links are empty, everything else is | ||
| # unknown. | ||
| finalized_history = _walk_finalized_chains(directly_finalized, group_slots) |
There was a problem hiding this comment.
When a collator equivocates or multiple candidates for one slot are present in the collected validator logs, SlotData retains whichever candidate was parsed last while directly_finalized retains only the slot number. Walking that unrelated candidate's parent chain can therefore mark canonical ancestors as skipped and noncanonical ancestors as finalized, corrupting the per-leader results precisely for the fork scenarios a consensus analyzer needs to diagnose.
Useful? React with 👍 / 👎.
| print(f"Leader stats server running at http://{host}:{port}{web_root}") | ||
| app.run(host=host, port=port, debug=False) |
There was a problem hiding this comment.
In --logs web-server mode, Flask serves requests concurrently by default, but all endpoints share one mutable ParserSessionStats instance whose cache and parsing dictionaries have no lock. If two initial requests or two requests after a log update overlap, one parse can append into the other's state and clear it while the other is still running, leaving duplicated or partial events cached and returning incorrect statistics; disable threaded serving or synchronize parser access.
Useful? React with 👍 / 👎.
| if (showIdx) html += '<td>' + v.validator_idx + '</td>'; | ||
| html += '<td>' + adnl + '</td>'; | ||
| html += '<td>' + pkh + '</td>'; | ||
| html += '<td>' + (v.name || '') + '</td>'; |
There was a problem hiding this comment.
When --validator-names-json contains a validator name with HTML markup, the value is concatenated directly into a table that is later assigned through innerHTML. A legitimate name containing angle brackets corrupts the rendered table, and a payload such as an image with an event handler executes script in every viewer's browser when the server is shared; render this value as text or HTML-escape it first.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bc4031ad5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if text: | ||
| _print_text(analyzer, time_from, time_until) |
There was a problem hiding this comment.
When --stats-dir --text is used with a new or stale index database, entering the FileIndex context only starts the indexing thread (file_index.py:111-114), while its initial scan runs asynchronously (file_index.py:404-411). This call can therefore query the database before files are indexed, print No groups found. or partial results, and immediately exit; wait for initial indexing to complete before producing the one-shot text report.
Useful? React with 👍 / 👎.
| if sd is not None and (sd.block_id_ext == "empty" or sd.is_empty): | ||
| history[current] = SlotStatus.EMPTY | ||
| else: | ||
| history[current] = SlotStatus.FINALIZED |
There was a problem hiding this comment.
When a finalization quorum is reconstructed from vote records but the corresponding candidate_received record is absent from the retained logs, _parse_vote_event still creates a SlotData whose block_id_ext remains None. This branch classifies that slot as FINALIZED (documented here as a finalized non-empty block), even though the finalized candidate may have been empty, so partial logs silently inflate finalized and undercount empty; preserve an unknown block type until candidate data is available.
Useful? React with 👍 / 👎.
| if (!timeFrom && !timeUntil && !groupFilter) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
On the initial web page all three inputs are blank, the group field is explicitly described as optional, and the page tells the user to click Load, but this guard makes that button return without issuing a request or changing the page. Since /api/stats already supports an unfiltered request, users cannot load all available groups through the advertised default interaction and receive no explanation that a filter is required.
Useful? React with 👍 / 👎.
| # Gather slots with finalization certificates. | ||
| directly_finalized: set[int] = set() | ||
| for e in group_events: | ||
| if e.label == "finalize_reached": | ||
| directly_finalized.add(e.slot) |
There was a problem hiding this comment.
When the latest observed slots have reached a skip quorum but no later candidate has finalized yet, the parser emits definitive skip_reached events for those slots, but this analyzer collects only finalize_reached. Because no later finalized parent chain exists to mark the gap, the trailing skipped slots are reported as unknown, understating each affected leader's skipped count for current or truncated sessions; seed the history from skip_reached certificates as well.
Useful? React with 👍 / 👎.
| def api_group(valgroup_name: str) -> Response | tuple[Response, int]: | ||
| stats = analyzer.analyze_group(valgroup_name) | ||
| if stats is None: | ||
| return jsonify({"error": "group not found"}), 404 |
There was a problem hiding this comment.
In --stats-dir mode, a group filter that is not present in the index is passed to CachedGroupParser._resolve_name, which falls back to base64.b64decode; common typos such as foo raise binascii.Error. The exception escapes this handler and produces a 500 response instead of reaching the intended group not found 404 branch, so validate or catch unresolved group names before analysis.
Useful? React with 👍 / 👎.
`with sqlite3.connect(...) as conn:` manages the transaction, not the connection, so every query left its db and -wal fds open until the cyclic GC collected the connection. Connection/cursor cycles keep refcounting from doing it, and gen-2 collections are rare in a process holding a large heap of cached parse results, so the fds accumulated. The mainnet leader_stats service hit its 1024 fd limit this way: of 1023 open fds, 507 were stats_mainnet_new.db, 506 its -wal and 1 its -shm. Requests then failed with "unable to open database file" and the server could no longer accept connections. Repeating one query 600 times leaks 440 fds before this change and none after. Add a _connection() context manager that commits like `with conn:` did and closes in a finally, and route all five call sites through it. Also stop piping the sudo helper's stderr: nothing ever read it, so a helper writing more than the pipe buffer would block forever, holding its fds and the child process. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6623d2510f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # One pass over the union of their files: a file holds many groups, so | ||
| # parsing group by group re-reads and re-deserializes the same bytes. | ||
| data = self._parse_uncached([hashes[name] for name in missing], f"{len(missing)} groups") | ||
| split = split_by_group(data, missing) |
There was a problem hiding this comment.
When a caller supplies the valid base64 hash of a named group, _resolve_name() accepts it, but parsed GroupInfo, slots, and events use the canonical workchain,shard.seqno name. Splitting by the caller-provided base64 string therefore produces an empty ConsensusData, which is then cached under the real group hash; the request reports the group as missing, and subsequent canonical requests also receive the poisoned empty cache until invalidation. Resolve the canonical name before splitting or split by group hash.
Useful? React with 👍 / 👎.
2 participants
No description provided.