Skip to content

Comments

fix: prevent split payment failures from blocking remaining targets#40

Open
hexdaemon wants to merge 1 commit intolnbits:mainfrom
hexdaemon:fix/splitpayments-audit-bugs
Open

fix: prevent split payment failures from blocking remaining targets#40
hexdaemon wants to merge 1 commit intolnbits:mainfrom
hexdaemon:fix/splitpayments-audit-bugs

Conversation

@hexdaemon
Copy link

Summary

  • Wrap per-target processing in try/except so one failed target logs an error and continues instead of aborting all remaining splits (critical: this was silently stopping payments)
  • Make LNURL detection case-insensitive — lowercase lnurl1... (the canonical bech32 form) was rejected at the API or misrouted as a wallet ID at payment time
  • Add minimum amount guard — sub-1-sat split amounts produced 0-amount invoices that crashed create_invoice, which then triggered the above cascading failure
  • Add post-fee-reserve minimum for LNURL targets — after subtracting fee_reserve, the amount could go negative/zero
  • Re-raise HTTPException before generic handler in api_targets_set — validation errors (invalid wallet, self-split, bad percent, over-100%) were being swallowed by a blanket except Exception and returned as a generic 500 "Cannot set targets"
  • Fix migration m003row["tag"] read from a table that has no tag column, causing KeyError when migrating existing data
  • Fix done_callback — replaced lambda fut: logger.success(fut.result()) which logged success(None) on failure with a proper helper
  • Fix frontend isTargetComplete — removed reference to deleted tag field that made dirty-checking always pass

Test plan

  • Configure split targets with a valid internal wallet and verify payments split correctly
  • Configure an LNURL target (lowercase lnurl1...) and verify it is accepted and paid
  • Send a very small payment (e.g. 1 sat with a 50% split) and verify the sub-1-sat target is skipped with a log warning rather than crashing
  • Configure one target as an invalid/deleted wallet and a second as valid — verify the second target still receives payment
  • Attempt to save targets with invalid wallet, self-split, or >100% total and verify the API returns a descriptive 400 error (not a 500)
  • Verify the frontend dirty indicator works correctly when adding/removing targets

🤖 Generated with Claude Code

Multiple bugs could cause split payments to silently stop going out:

- An exception processing any single target would abort all remaining
  targets in the loop. Each target is now wrapped in try/except so
  failures are logged and the loop continues.
- Lowercase LNURL bech32 strings (the canonical form) were not
  recognized, causing them to be rejected at the API or misrouted
  as wallet IDs at payment time. Checks are now case-insensitive.
- Sub-1-sat split amounts produced 0-amount invoices that failed
  invoice creation. A minimum amount guard now skips these with a
  warning.
- HTTPException validation errors (invalid wallet, self-split, bad
  percent) were caught by a blanket `except Exception` and re-raised
  as a generic 500, masking the actual problem from users.
- Migration m003 read a non-existent `tag` column from the m002
  table, causing a KeyError when migrating with existing data.
- The done_callback logged `None` as a success message on payment
  failure. Replaced with a helper that only logs actual results.
- Frontend `isTargetComplete` referenced a removed `tag` field,
  making the dirty-check always pass regardless of percent value.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant