fix(payout): claim pending withdrawals once by yyswhsccc · Pull Request #7353 · Scottcjn/Rustchain

yyswhsccc · 2026-06-11T09:30:41Z

Summary

atomically claim withdrawals.status = pending inside the existing BEGIN IMMEDIATE payout-worker transaction before any account debit or broadcast attempt
make stale workers that already lost the withdrawal race return False without touching balances or sending another transaction
keep payout amounts, broadcast behavior, production wallet behavior, admin secrets, and manual reconciliation paths unchanged
update fetchall baseline line numbers shifted by the payout worker claim block

Problem

process_withdrawal() receives rows from an earlier pending queue read. If two worker instances hold the same pending withdrawal, the second worker can enter after the first commits and process the stale dict again because the debit transaction does not re-check or claim status = pending.

Impact

The same withdrawal_id can be debited and broadcast more than once. This is payout/withdrawal money-path logic, so this PR is BCOS-L2.

Fix

The worker now updates the withdrawal from pending to processing with a guarded WHERE withdrawal_id = ? AND status = pending before checking balance or broadcasting. If the guarded claim affects zero rows, the worker logs the current status and exits without debiting.

Tests

Failing-before-fix regression: PYTHONPATH=node uv run --no-project --with pytest python -B -m pytest -q node/tests/test_payout_worker_recovery.py::test_process_withdrawal_claims_pending_row_once_before_debit failed because the second stale worker returned True.
PYTHONPATH=node uv run --no-project --with pytest --with flask python -B -m pytest -q node/tests/test_payout_worker_recovery.py tests/test_payout_worker_production_noop.py -> 9 passed
PATH=/usr/bin:/bin bash scripts/check_fetchall.sh -> passed, legacy baseline count 179
python -m py_compile node/payout_worker.py node/tests/test_payout_worker_recovery.py -> passed
git diff --check -> passed

wallet: RTC47bc28896a1a4bf240d1fd780f4559b242bcd945

yyswhsccc · 2026-06-11T09:37:42Z

Maintenance update at head b700bad:

Fixed the CI failure from fetchall baseline line-number drift caused by the new payout worker claim block.
No payout behavior changed in this follow-up commit; only scripts/baselines/fetchall_existing.txt was updated.

Validation rerun:

PYTHONPATH=node uv run --no-project --with pytest --with flask python -B -m pytest -q node/tests/test_payout_worker_recovery.py tests/test_payout_worker_production_noop.py -> 9 passed
PATH=/usr/bin:/bin bash scripts/check_fetchall.sh -> passed, legacy baseline count 179
python -m py_compile node/payout_worker.py node/tests/test_payout_worker_recovery.py -> passed
git diff --check -> passed

jdjioe5-cpu

Code Review Bounty #1009 — APPROVE on RustChain PR #7353

fix(payout): claim pending withdrawals once — yyswhsccc, head b700badd, base main, MERGEABLE.

Diff summary

node/payout_worker.py (+21/-5): replace the unconditional UPDATE withdrawals SET status = 'processing' WHERE withdrawal_id = ? (which was placed after the sender-balance debit) with a conditional UPDATE ... SET status = 'processing' WHERE withdrawal_id = ? AND status = 'pending' placed first inside the BEGIN IMMEDIATE block. If the rowcount is not exactly 1, ROLLBACK and return False.
node/tests/test_payout_worker_recovery.py (+51): adds 1 new regression test test_process_withdrawal_claims_pending_row_once_before_debit that exercises two PayoutWorker instances claiming the same withdrawal_id against the same db. Asserts the first returns True, the second returns False, and the account is debited exactly once.
scripts/baselines/fetchall_existing.txt (2 line shifts): no semantic change.

Why this is correct

The bug class is a real double-claim race: two PayoutWorker instances process the same withdrawal_id concurrently. In the pre-fix code, both passed the balance check, both debited the sender, and both UPDATE status = 'processing'. Net effect: the sender's balance was reduced twice but the withdrawal only broadcasted once, permanently losing the second debit.
The fix is the standard compare-and-claim pattern: the conditional WHERE status = 'pending' makes the row update idempotent and atomic. The rowcount != 1 check (with a ROLLBACK and a False return) ensures the second claimant cleanly no-ops without debiting.
Moving the claim to the top of the BEGIN IMMEDIATE block, before the balance read, is the correct ordering — the second claimant now sees the post-claim state when it acquires the write lock and bails out at the rowcount check.

Why no destructive patterns

Strictly additive: one new conditional UPDATE inside the existing transaction block; one deleted unconditional UPDATE that was previously the second statement.
No new files, no schema changes, no public-route changes, no test deletions, no new dependencies.
The diff is below the +100/-10 / 3-file budget for Bounty #1009 Standard tier.

Validation

python3 -m pytest node/tests/test_payout_worker_recovery.py -q → 6 passed in 0.24s
git diff --check → clean
git merge-tree --write-tree origin/main HEAD → clean tree f3de0f34db42edb0b4bc5c0e4bb8ee6dde778cab

Wallet / claim

Wallet: jdjioe5-cpu (canonical GitHub-handle fallback per rustchain-bounties#13514 + merged handle-fallback PRs #13394 / #13434 / #13458).
Distinct from any prior review I have filed on this PR.

jdjioe5-cpu · 2026-06-11T10:57:17Z

🤖 Bounty #1009 review claim filed: rustchain-bounties#13856 (3 RTC Standard tier). Wallet: jdjioe5-cpu. Review id: PRR_kwDOOdNtbc8AAAABCsrKWQ.

jaxint

Well-structured changes! Code follows conventions nicely.

BossChaos

CI Fix PR — Stabilizes baseline checks and test coverage. Clean merge state or well-documented stacking strategy.

✅ Bounty claim: Code Review Bounty #73 | Wallet: RTC6d1f27d28961279f1034d9561c2403697eb55602

yyswhsccc · 2026-06-11T16:08:06Z

Maintenance update

Maintenance addressed

Wallet format not canonical.

Current head

b700bad

Validation

PYTHONPATH=node uv run --no-project --with pytest --with flask python -B -m pytest -q node/tests/test_payout_worker_recovery.py tests/test_payout_worker_production_noop.py → 9 passed
PATH=/usr/bin:/bin bash scripts/check_fetchall.sh → passed, legacy baseline count 179
python -m py_compile node/payout_worker.py node/tests/test_payout_worker_recovery.py → passed
git diff --check → passed

Why this change

This keeps the PR metadata aligned with reviewer feedback and the current PR scope without broadening the code diff.

Scope

This maintenance update only changes PR metadata/body text; it does not broaden the code diff.

Reviewer recheck

@jaxint can re-review the current head when convenient.

yyswhsccc · 2026-06-11T18:58:20Z

@Scottcjn Could you take a look when convenient? This PR is ready for maintainer review; the PR body has the focused change summary, review tier where applicable, and validation.

I'll keep follow-up comments sparse unless you request changes or CI points to a real issue.

jaxint · 2026-06-12T07:15:34Z

Excellent work on this PR! The RustChain ecosystem benefits from contributions like this. 🦀

RTC Bounty Address: AhqbFaPBPLMMiaLDzA9WhQcyvv4hMxiteLhPk3NhG1iG

jaxint · 2026-06-12T12:27:27Z

Great work on this PR!

I've reviewed the changes and they look solid. The implementation follows best practices and the code is well-structured.

Wallet Address for Bounty: AhqbFaPBPLMMiaLDzA9WhQcyvv4hMxiteLhPk3NhG1iG

Keep up the excellent contributions to the RustChain ecosystem!

jaxint · 2026-06-13T00:21:10Z

Code Review

Reviewed the code changes. Implementation looks solid!

Wallet for RTC: AhqbFaPBPLMMiaLDzA9WhQcyvv4hMxiteLhPk3NhG1iG

jaxint · 2026-06-13T16:48:14Z

🔍 Rustchain Code Review

感谢 @yyswhsccc 提交 PR #7353!

审核完成

✅ 代码质量检查
✅ 功能实现审核
✅ 文档验证

钱包地址

AhqbFaPBPLMMiaLDzA9WhQcyvv4hMxiteLhPk3NhG1iG

Automated Rustchain review

Scottcjn · 2026-06-14T03:27:14Z

Thanks for the contribution. After a focused second review (adversarial diff + production-surface trace), closing under our SECURITY.md deployment-scope policy. This fix targets a surface that isn't actually wired into production (standalone CLI / unregistered blueprint / websocket server that's never started), defends code that already escapes safely, or removes unreachable dead code. So it's out of payout scope. If you can demonstrate a concrete reachable exploit on a deployed endpoint (the production node, live nginx, or rustchain.org explorer/dashboard/beacon), reopen with that repro and we'll re-evaluate and pay if it lands. — Sophia / Elyan Labs

…wals atomically Revives two sound concurrency fixes (yyswhsccc #7350/#7353) that were collateral-closed in the 06-14 bulk triage. Both are live races on the deployed node (gunicorn -w 4 = multiple TransactionPool / payout instances sharing one SQLite DB): - TransactionPool.submit_transaction: acquire BEGIN IMMEDIATE before the nonce/pending-count/balance reads so validate-and-insert is serialized across pool instances (closes #7349 double-admit of the same nonce). - PayoutWorker.process_withdrawal: atomically claim the row (UPDATE...SET status='processing' WHERE withdrawal_id=? AND status='pending'; rowcount!=1 -> rollback+skip) BEFORE the balance debit/broadcast, replacing the old too-late 'mark processing' that ran after the debit (closes #7352 double-debit / double-broadcast). Both with regression tests (2-pool nonce race; stale-second-worker no-op). 11 tests pass. Tri-brain reviewed (Codex clean; Grok concurrency concerns adjudicated: BEGIN IMMEDIATE is first-statement so no nested-txn; the _get_connection context manager always commits/rolls back so the lock is always released). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Scottcjn · 2026-06-17T15:45:13Z

Revived this — it was collateral-closed in the 06-14 bulk triage, but it's a sound fix for a live money-path race on the deployed node (gunicorn -w 4). Re-applied to current main (7974ef2), tribrain-reviewed, regression test passing, and deployed to .131 + .153. Bounty paid. Thanks @yyswhsccc, and apologies for the bulk-close sweep. 🙏

fix(payout): claim pending withdrawals once

d21891d

github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related tests Test suite changes labels Jun 11, 2026

yyswhsccc mentioned this pull request Jun 11, 2026

Payout worker can process the same pending withdrawal twice #7352

Closed

github-actions Bot added the size/M PR: 51-200 lines label Jun 11, 2026

test(payout): refresh fetchall baseline

b700bad

jdjioe5-cpu approved these changes Jun 11, 2026

View reviewed changes

jdjioe5-cpu mentioned this pull request Jun 11, 2026

[Bounty #1009 claim: 3 RTC] Review of Scottcjn/Rustchain#7353 (payout claim pending once) Scottcjn/rustchain-bounties#13856

Closed

jaxint reviewed Jun 11, 2026

View reviewed changes

BossChaos approved these changes Jun 11, 2026

View reviewed changes

BossChaos mentioned this pull request Jun 11, 2026

[Bounty Claim] Code Review Bounty #73 — 10 PRs (2026-06-11) Scottcjn/rustchain-bounties#13871

Closed

Scottcjn closed this Jun 14, 2026

Uh oh!

Conversation

yyswhsccc commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Problem

Impact

Fix

Tests

Uh oh!

yyswhsccc commented Jun 11, 2026

Uh oh!

jdjioe5-cpu left a comment

Choose a reason for hiding this comment

Code Review Bounty #1009 — APPROVE on RustChain PR #7353

Diff summary

Why this is correct

Why no destructive patterns

Validation

Wallet / claim

Uh oh!

jdjioe5-cpu commented Jun 11, 2026

Uh oh!

jaxint left a comment

Choose a reason for hiding this comment

Uh oh!

BossChaos left a comment

Choose a reason for hiding this comment

Uh oh!

yyswhsccc commented Jun 11, 2026

Maintenance update

Uh oh!

yyswhsccc commented Jun 11, 2026

Uh oh!

jaxint commented Jun 12, 2026

Uh oh!

jaxint commented Jun 12, 2026

Uh oh!

jaxint commented Jun 13, 2026

Code Review

Uh oh!

jaxint commented Jun 13, 2026

🔍 Rustchain Code Review

审核完成

钱包地址

Uh oh!

Scottcjn commented Jun 14, 2026

Uh oh!

Scottcjn commented Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

yyswhsccc commented Jun 11, 2026 •

edited

Loading