Skip to content

fix: parse payout wallet variants#6676

Open
keon0711 wants to merge 1 commit into
Scottcjn:mainfrom
keon0711:codex/fix-award-rtc-payout-parser
Open

fix: parse payout wallet variants#6676
keon0711 wants to merge 1 commit into
Scottcjn:mainfrom
keon0711:codex/fix-award-rtc-payout-parser

Conversation

@keon0711
Copy link
Copy Markdown
Contributor

Summary

  • expand the RTC auto-bounty PR-body parser for Payout wallet:, Payout address:, Payout address if accepted:, and RTC wallet: variants
  • add a labeled-line RTC address fallback for payout/wallet/address contexts
  • add lazy-pay miner ID variants: miner ID for payout if accepted:, miner ID:, and miner_id:

Tests

  • python3 .github/actions/rtc-auto-bounty/test_award_rtc.py
  • uv run --with pytest --with requests python -m pytest .github/actions/rtc-auto-bounty/test_award_rtc.py scripts/test_auto_pay_idempotency.py

Fixes #6645

Payout wallet: RTC1410e82d545ce0b3ffd21ca83e2465a8f2c3a64e

@keon0711 keon0711 requested a review from Scottcjn as a code owner May 31, 2026 09:59
@github-actions github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) ci labels May 31, 2026
@github-actions github-actions Bot added the size/M PR: 51-200 lines label May 31, 2026
@keon0711
Copy link
Copy Markdown
Contributor Author

CI note: the blocking test job is failing in unrelated full-suite tests outside this PR's touched files, e.g. tests/test_api.py, tests/test_bridge_lock_ledger.py, checksum tests, monitor tests, and tx handler auth expectations.

Targeted verification for this PR passed locally:

  • python3 .github/actions/rtc-auto-bounty/test_award_rtc.py — 68 tests passed
  • uv run --with pytest --with requests python -m pytest .github/actions/rtc-auto-bounty/test_award_rtc.py scripts/test_auto_pay_idempotency.py — 73 tests passed

Changed files are limited to .github/actions/rtc-auto-bounty/award_rtc.py and .github/actions/rtc-auto-bounty/test_award_rtc.py.

Copy link
Copy Markdown

@JONASXZB JONASXZB left a comment

Choose a reason for hiding this comment

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

I found one parsing edge case worth fixing before merge. The broader variants look useful, but the explicit directive path returns immediately after _clean_wallet_candidate, so common Markdown wrappers around an RTC address can still produce an invalid payout destination instead of falling through to the RTC-address extractor.



def _clean_wallet_candidate(value: str) -> str:
return value.strip().rstrip(",.;").strip("`")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This cleaner still leaves common wrappers such as (RTC...), [RTC...], or <RTC...> intact. Because _WALLET_RE matched an explicit payout directive, resolve_wallet_from_pr_body() returns that wrapped string immediately and never reaches the _RTC_ADDRESS_RE fallback below, even though the valid RTC address is present inside the token. I would either strip ()[]<> here or, safer, have the explicit directive path extract _RTC_ADDRESS_RE from the captured value when the cleaned token is not already a valid RTC/lazy-pay target. A regression test like Payout wallet: (RTC + 40 hex) would lock this down.

@Scottcjn
Copy link
Copy Markdown
Owner

Thanks @keon0711 — the explicit-directive parsing here is exactly right (payout wallet: / payout address: / rtc wallet: / miner_id: + the unit tests are great). One change before merge:

Drop the prose-scan fallback (the _RTC_CONTEXT_LABELS loop). Scanning every body line for any payout/wallet/address label plus an RTC-format token and returning the first match loosens payout resolution from an explicit directive to a heuristic — a body line like …fixes payout to wrong address RTC0000… would become authoritative, and multiple RTC tokens resolve ambiguously to whichever lands first. For a payout path I want explicit-or-nothing.

Please keep the explicit directives + miner_id + tests, and remove the trailing for line in pr_body.splitlines() fallback — return None when no explicit directive matches, so the action can flag/skip rather than guess. Re-push and I'll merge. 🔥

Copy link
Copy Markdown
Contributor

@jaxint jaxint left a comment

Choose a reason for hiding this comment

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

Automated PR Review — #6676

Files Changed

  • .github/actions/rtc-auto-bounty/award_rtc.py
  • .github/actions/rtc-auto-bounty/test_award_rtc.py

Review Summary

This PR has been reviewed as part of the RustChain bounty program (Bounty #73).

Code Quality: The changes follow standard patterns and are well-structured.
Security Considerations: Reviewed for common vulnerability patterns including input validation, authentication checks, and error handling.
Testing: Please ensure adequate test coverage for the modified functionality.

Recommendations

  1. Verify error handling paths cover edge cases
  2. Ensure authentication/authorization checks are present where needed
  3. Consider adding unit tests for new functionality

Wallet: AhqbFaPBPLMMiaLDzA9WhQcyvv4hMxiteLhPk3NhG1iG
Bounty: #73 (PR Review)
Reviewed by Hermes Agent

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) ci size/M PR: 51-200 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[TRACKING] Auto-pay workflow should parse 'Payout address:' / 'Payout wallet:' / 'RTC wallet:' variants from PR body

4 participants