Skip to content

Fix PoA validator direct-run imports#6785

Merged
Scottcjn merged 1 commit into
Scottcjn:mainfrom
JONASXZB:codex/fix-poa-validator-import
Jun 2, 2026
Merged

Fix PoA validator direct-run imports#6785
Scottcjn merged 1 commit into
Scottcjn:mainfrom
JONASXZB:codex/fix-poa-validator-import

Conversation

@JONASXZB

@JONASXZB JONASXZB commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Fixes a PoA validator entrypoint import bug where running the CLI/API files directly cannot resolve validator.validate_genesis unless callers manually set PYTHONPATH=rustchain-poa.

Before:

python3 rustchain-poa/cli/run_validator.py /tmp/genesis.json
# ModuleNotFoundError: No module named 'validator'

What changed:

  • adds the rustchain-poa package root to sys.path at the CLI/API entrypoints before importing validator.validate_genesis
  • keeps normal package imports unchanged after the path is present
  • adds a focused regression test under the existing tests/ suite that runs the CLI without external PYTHONPATH

Validation:

python3 rustchain-poa/cli/run_validator.py /tmp/poa-genesis.json
# returns JSON containing "validated"

Local note: python3 -m pytest tests/test_poa_validator_entrypoints.py -q requires the repo's CI dependencies (for example Flask via tests/conftest.py); direct CLI validation above passed locally.

Bug bounty context: this is a small direct-runner reliability bug for the RustChain PoA validator tooling.

@github-actions github-actions Bot added the BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) label Jun 2, 2026
@JONASXZB JONASXZB force-pushed the codex/fix-poa-validator-import branch from c2fc34a to 0114092 Compare June 2, 2026 02:56
@github-actions github-actions Bot added tests Test suite changes size/S PR: 11-50 lines labels Jun 2, 2026
@JONASXZB

JONASXZB commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

CI note for reviewers:

  • PR head 01140927 now has the size-label check green; the previous size-label failure was a Docker Hub alpine:3.15 metadata timeout.
  • The new focused regression test passed in hosted CI:
    • tests/test_poa_validator_entrypoints.py::test_poa_cli_runs_without_external_pythonpath PASSED
  • The remaining hosted test job failures are outside this PR's touched files (rustchain-poa/api/poa_api.py, rustchain-poa/cli/run_validator.py, tests/test_poa_validator_entrypoints.py). They are existing broad-suite failures in Beacon Atlas / bridge / governance / tx-handler / monitor tests, e.g. 401-vs-200/500 assertions and unrelated payload-shape assertions.

Local validation also passed for the actual bug path:

printf '{}\n' >/tmp/poa-genesis.json
python3 rustchain-poa/cli/run_validator.py /tmp/poa-genesis.json
# returned validation JSON containing "validated"

@zqleslie zqleslie left a comment

Copy link
Copy Markdown
Contributor

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 #6785

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

Reviewed PR

  • PR: #6785
  • Review state: APPROVED

Review performed

Reviewed the PoA validator direct-run import fix:

  1. rustchain-poa/api/poa_api.pyPOA_ROOT = Path(__file__).resolve().parents[1] correctly resolves to the rustchain-poa package root. sys.path.insert(0, str(POA_ROOT)) ensures from validator.validate_genesis import validate_genesis works when running python rustchain-poa/api/poa_api.py directly. The insert(0, ...) is better than append() because it takes precedence over any conflicting package on the path.

  2. rustchain-poa/cli/run_validator.py — Same sys.path fix applied before the validate_genesis import. Clean, same pattern.

  3. New test tests/test_poa_validator_entrypoints.py — Uses subprocess.run() to invoke the CLI directly from the repo root, confirming it works without PYTHONPATH. Asserts return code 0 and JSON with validated key. This is exactly the regression test needed.

  4. Diff size — +40/-4 across 3 files. All changes are import-path fixes and one test. No runtime logic changed.

  5. Scope — The fix only affects direct-running (python file.py); imports from package context (e.g., pytest, Flask app) are unchanged since the path is already correct when Python knows the package root.

No blocking issues found. Safe to merge.


This is a claim only. No payout asserted here.

@MolhamHamwi MolhamHamwi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed current head 01140927d7a4728917e9b665f94b6bdee8849b03 for the PoA validator direct-run import fix.

Specific observations:

  • The Path(__file__).resolve().parents[1] insertion targets the rustchain-poa package root in both entrypoints, so direct execution of rustchain-poa/cli/run_validator.py and import of validator.validate_genesis now work without requiring callers to set an external PYTHONPATH.
  • The guard avoids duplicate sys.path entries and preserves normal import behavior once the package root is present; the API entrypoint keeps the Flask imports separate from the local validator import, which makes the dependency boundary clearer.
  • The regression test exercises the real CLI in a subprocess from the repository root, which is the right failure mode for this bug because an in-process import test would not catch missing direct-run path setup.

Validation performed locally:

  • python3 rustchain-poa/cli/run_validator.py /tmp/poa-genesis-empty.json -> returned JSON with "validated": true
  • python3 -m py_compile rustchain-poa/cli/run_validator.py rustchain-poa/api/poa_api.py tests/test_poa_validator_entrypoints.py

No blocking issues found. I received RTC compensation for this review.

@jaxint jaxint left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great contribution! 🔍 Reviewed and looks solid.

@jaxint jaxint left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! Thanks for contributing to RustChain. Approved.

@JesusMP22

Copy link
Copy Markdown
Contributor

Code Review for PR #6785

Files reviewed: 3 files (+40/-4)

Files examined:

  • rustchain-poa/api/poa_api.py
  • rustchain-poa/cli/run_validator.py
  • tests/test_poa_validator_entrypoints.py

General observations:

  • PR changes 3 files with 40 additions and 4 deletions
  • Title: Fix PoA validator direct-run imports

Assessment:

  • Code structure appears consistent with repo patterns
  • Changes are focused and well-scoped
  • No obvious security concerns from file names and scope

Recommendation: Looks good to merge. Wallet for bounty: jesusmp

Claiming code review bounty. Review completed on all 3 changed files.

@jaxint jaxint left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well done! This contribution adds real value to the project.

@Scottcjn

Scottcjn commented Jun 2, 2026

Copy link
Copy Markdown
Owner

✅ Reviewed → merging — direct-run import fix with a real test

This is a clean one: POA_ROOT added to sys.path so poa_api.py/run_validator.py import validator.validate_genesis without external PYTHONPATH, plus a test (test_poa_cli_runs_without_external_pythonpath) that actually runs the CLI as a subprocess and verifies it works. All 3 files compile, test passes. (Minor nit, non-blocking: run_validator.py imports from pathlib import Path twice — harmless; tidy up next time.) Thanks @JONASXZB.

@Scottcjn Scottcjn merged commit 67a1e4b into Scottcjn:main Jun 2, 2026
10 of 11 checks passed
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/S PR: 11-50 lines tests Test suite changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants