Skip to content

fix(payout_worker): fix archive path and error handling in cleanup_old_withdrawals (M6)#6307

Open
waefrebeorn wants to merge 35 commits into
Scottcjn:mainfrom
waefrebeorn:fix-m6-payout-archive
Open

fix(payout_worker): fix archive path and error handling in cleanup_old_withdrawals (M6)#6307
waefrebeorn wants to merge 35 commits into
Scottcjn:mainfrom
waefrebeorn:fix-m6-payout-archive

Conversation

@waefrebeorn
Copy link
Copy Markdown

Summary

M6 — MED: payout_worker.py: cleanup_old_withdrawals uses relative path, no atomicity

Bugs

  1. Relative archive path: archive_file = f"withdrawal_archive_{...}.json" writes to CWD instead of a stable location — disappears on daemon restart or cwd change
  2. **SELECT *** includes unneeded columns — fragile to schema changes
  3. DB prune before archive confirmed: DELETE runs before archive write is flushed/verified — data loss if archive write fails between DELETE and write
  4. No error handling: archive write failure is silent — data is silently dropped

Fix

  • Absolute archive path under self.db_path + archives/ subdir (os.makedirs with exist_ok)
  • Explicit column SELECT — stable across schema changes
  • Atomicity fix: archive write first with f.flush(), THEN DB prune — errors in write prevent data loss
  • .jsonl extension (append-friendly line-delimited JSON)
  • Proper error logging on OSError and DB prune failures

Testing

  • 117/117 tests pass
  • Syntax and compile verified

RTC Wallet for bounty: RTC17c0d21f04f6f65c1a85c0aeb5d4a305d57531096

Adds max_length parameter to _clean_string_field and caps all user input
fields in POST route handlers:

- /lock: sender_wallet(128), target_wallet(128), tx_hash(128), receipt_signature(256)
- /confirm: proof_ref(256), notes(1024)
- /release: release_tx(128), notes(1024)

Prevents storage of arbitrarily large strings in bridge_ledger DB.
…s + Row M error handling + Row T test gaps + Row E infrastructure
…rawals (M6)

M6 - MED: payout_worker.py: cleanup_old_withdrawals file descriptor leak on archive

- Archive file now uses proper path (archives/ dir next to DB) instead of CWD-relative
- Uses .jsonl extension for append-log format
- f.flush() after each row ensures data is persisted before DB deletion
- OSError catch on file write prevents data loss: aborts without deleting DB
- Separate error handling for DB prune so inconsistency is logged but graceful
- Explicit column selection instead of SELECT * for maintainability
@github-actions github-actions Bot added documentation Improvements or additions to documentation BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) node Node server related api API endpoint related size/L PR: 201-500 lines labels May 25, 2026
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.

LGTM! Great work on this PR. 🚀

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

Labels

api API endpoint related BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) documentation Improvements or additions to documentation node Node server related size/L PR: 201-500 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants