Skip to content

Fix monitor miners envelope normalization#6786

Open
JONASXZB wants to merge 1 commit into
Scottcjn:mainfrom
JONASXZB:codex/fix-monitor-miners-envelope
Open

Fix monitor miners envelope normalization#6786
JONASXZB wants to merge 1 commit into
Scottcjn:mainfrom
JONASXZB:codex/fix-monitor-miners-envelope

Conversation

@JONASXZB
Copy link
Copy Markdown
Contributor

@JONASXZB JONASXZB commented Jun 2, 2026

Fixes a RustChain monitor bug where the correct /api/miners envelope normalizer was immediately overwritten by a second narrower normalize_miners_payload definition.

Before:

  • get_miners() could fetch the current API shape { "items": [...], "pagination": ... }
  • the later duplicate normalizer only accepted { "miners": [...] }
  • the monitor returned the raw envelope instead of miner rows, and tests/test_rustchain_monitor.py::test_request_helpers_call_expected_endpoints failed

What changed:

  • remove the duplicate narrower normalizer
  • keep the earlier normalizer that accepts legacy lists plus miners, data, and items envelopes
  • make print_miners check for a normalized list explicitly before printing rows

Validation:

python3 - <<'CHECK'
import importlib.util
from pathlib import Path
spec = importlib.util.spec_from_file_location('rustchain_monitor', Path('tools/rustchain-monitor/rustchain_monitor.py'))
mod = importlib.util.module_from_spec(spec); spec.loader.exec_module(mod)
assert mod.normalize_miners_payload({'items':[{'miner':'alice'}], 'pagination': {'total': 1}}) == [{'miner':'alice'}]
assert mod.normalize_miners_payload({'data':[{'miner':'bob'}]}) == [{'miner':'bob'}]
assert mod.normalize_miners_payload({'pagination': {'total': 0}}) == {'pagination': {'total': 0}}
print('direct monitor normalization checks passed')
CHECK

Local note: python3 -m pytest tests/test_rustchain_monitor.py -q is blocked in this local environment before reaching the test file because tests/conftest.py imports the integrated Flask node and Flask is not installed locally. Hosted CI installs Flask in .github/workflows/ci.yml.

@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) size/XS PR: 1-10 lines labels Jun 2, 2026
@JONASXZB
Copy link
Copy Markdown
Contributor Author

JONASXZB commented Jun 2, 2026

CI note for reviewers:

  • PR head 7a88e487 fixed the monitor-specific failures. Hosted CI shows all tests/test_rustchain_monitor.py tests passing, including:
    • test_request_helpers_call_expected_endpoints PASSED
    • test_normalize_miners_payload_preserves_unexpected_shapes PASSED
    • test_print_miners_accepts_paginated_envelope PASSED
  • All non-test checks are green (size-label, label, BCOS, review gate, etc.).
  • The remaining hosted test failures are outside this PR's touched file (tools/rustchain-monitor/rustchain_monitor.py) and are existing broad-suite failures in Beacon Atlas / bridge / governance / tx-handler / proxy / transfer tests.

This PR is intentionally scoped to the monitor miners-envelope normalization bug only.

Copy link
Copy Markdown
Contributor

@zqleslie zqleslie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Bounty Claim — RustChain PR #6786

Bounty: #73 Code Review Bounty Program
Reviewer: zqleslie
RTC wallet: XKO212dF8324b9b61F294D26A6Dc68e3f81e4BE451D

Reviewed PR

  • PR: #6786
  • Review state: APPROVED

Review performed

Reviewed the monitor miners envelope normalization fix:

  1. Duplicate function removal — The earlier normalize_miners_payload correctly handles {items: [...], pagination: ...}, {miners: [...]}, {data: [...]}, and plain lists. The later duplicate only accepted {miners: [...]}, overwriting the correct one. Removing the duplicate restores proper envelope handling.

  2. print_miners guard — Changed from if miners is None to if not isinstance(miners, list). This is safer because the normalizer can return the original dict (e.g., {"pagination": {"total": 0}}) when there are no miners, and the old check would try to iterate a dict.

  3. Diff size — +1/-7 in 1 file. Minimal, correct, no side effects.

  4. Validation — The inline Python check in the PR body covers the three envelope shapes. No need for local pytest since the function is pure Python with no Flask deps.

No blocking issues found. Safe to merge.


This is a claim only. No payout asserted here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) size/XS PR: 1-10 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants