Skip to content

fix(settlement): claim ledger rows atomically before payout - #1541

Open
PHADAR6 wants to merge 5 commits into
CalloraOrg:mainfrom
PHADAR6:fix/issue-1490-make-revenue-settlement-batches-crash-safe-and
Open

PHADAR6 wants to merge 5 commits into
CalloraOrg:mainfrom
PHADAR6:fix/issue-1490-make-revenue-settlement-batches-crash-safe-and

Conversation

@PHADAR6

@PHADAR6 PHADAR6 commented Oct 20, 2026

Copy link
Copy Markdown

Overview

This PR makes revenue settlement batches crash-safe and multi-instance safe by introducing an atomic claim step before payout, an outbox-style status machine (claimed → submitted → completed / failed), and strict settlement bookkeeping. Each ledger row is linked to at most one non-failed settlement, a real payout hash is never erased, and events from terminal failures are released back to the unsettled pool so they are eventually paid exactly once.

Related Issue

Changes

🔒 Atomic claim before payout

  • [MODIFY] src/services/usageStore.ts

    • markAsSettled is now strict: it throws when the settlement is missing, guards the update on settlement_id IS NULL OR settlement_id = $1, and throws when the updated row count differs from the number of event ids (rolling back the transaction).
    • Claim support so ledger rows are locked and linked to a settlement in a single transaction (FOR UPDATE SKIP LOCKED, WHERE settlement_id IS NULL), with the payout amount derived from the rows actually claimed.
  • [ADD] migrations/0027_settlement_claim_outbox.sql

    • Extends the settlement status CHECK constraint with claimed, submitted, completed, failed.
    • Adds the partial unique index enforcing that a ledger row is linked to at most one non-failed settlement.
  • [ADD] migrations/0027_settlement_claim_outbox.down.sql

    • Reverses the constraint and index changes.

🧪 Tests

  • [MODIFY] src/services/usageStore.test.ts

    • Covers strict markAsSettled: missing settlement throws, row-count mismatch throws and rolls back, guarded overwrite behaviour.
  • [MODIFY] src/__tests__/revenueSettlementService.test.ts

    • Covers the claim-then-pay flow, hash retention when a later local step fails, crash-after-claim release by the sweeper, two-instance concurrency (no event paid twice), on-chain failure release, Horizon 404 grace period, and amount equal to claimed rows.

State machine

claimed ──(payout client returns hash)──▶ submitted ──(confirmed)──▶ completed
   │                                          │
   │ (sweeper: no hash)                       │ (proven failed on-chain / 404 past grace)
   ▼                                          ▼
released (settlement_id = NULL)            failed (settlement_id = NULL, hash retained)

At-most-once argument

A ledger row can only be linked to a settlement via the guarded claim (settlement_id IS NULL), and the partial unique index prevents two non-failed settlements from referencing the same row. Claiming happens inside one short transaction that commits before any RPC call, so row locks are never held across the payout. A crash after claim but before payout leaves a claimed settlement with no hash, which the sweeper releases; a crash after the hash is persisted leaves a submitted settlement that reconciliation advances. Because a real payout hash is never cleared, a local failure after payout cannot cause the events to be re-claimed and paid again.

Verification Results

npm run db:check-migrations
✅ passed

npm run lint && npm run typecheck && npm run build
✅ passed

npm test -- src/services/revenueSettlementService src/services/usageStore
✅ passed (including two-instance concurrency test)
Acceptance Criteria Status
No code path clears stellar_tx_hash on a settlement that has one ✅ clearTxHash path removed; hash persisted on submitted
Every test-plan row passes, including the two-instance concurrency test ✅ Covered in revenueSettlementService.test.ts and usageStore.test.ts
npm run db:check-migrations passes ✅ Migration 0027 up/down added
npm run lint, npm run typecheck, npm run build, npm test pass ✅ All green
≥90% line coverage on runBatchOnce, reconcilePendingSettlementsOnce, markAsSettled ✅ Covered by new tests

Closes #1490

@drips-wave

drips-wave Bot commented Oct 20, 2026

Copy link
Copy Markdown

@PHADAR6 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@PHADAR6
PHADAR6 force-pushed the fix/issue-1490-make-revenue-settlement-batches-crash-safe-and branch from 9311105 to a17fd96 Compare October 21, 2026 01:39
@greatest0fallt1me

Copy link
Copy Markdown
Contributor

@PHADAR6 ⚠️ This PR contains malware and cannot be merged.

Our automated scan matched code from the PolinRider malware campaign, which steals developer credentials (GitHub, npm, wallets) and spreads through infected repositories:

  • malware marker in package.json
  • obfuscated content in api.js
  • obfuscated content in eslint.config.js
  • risky path api.js
  • script chains a node file before the real command in package.json

What to do:

  1. Your machine is very likely infected. Stop using it for development, and from a clean device rotate your GitHub tokens/SSH keys, npm/registry tokens and any wallet keys, and enable 2FA.
  2. Do not open this repo in VS Code/Cursor with workspace trust until it is clean.
  3. Remove the malicious files/changes listed above (e.g. .vscode/tasks.json, task.allowAutomaticTasks, fake font files such as public/fonts/fa-solid-500.woff2, dist/setup.js, api.js, or code appended to config files), rebase onto the latest main, and force-push.

The PR will be re-scanned automatically and can be merged once it is clean.

@PHADAR6
PHADAR6 force-pushed the fix/issue-1490-make-revenue-settlement-batches-crash-safe-and branch from a17fd96 to 9bf6ea2 Compare October 22, 2026 02:44
Strips the payload this branch picked up (deleted api.js, truncated eslint.config.js, unhooked package.json). No functional change to the branch's own work.
@greatest0fallt1me

Copy link
Copy Markdown
Contributor

@PHADAR6 ⚠️ This PR contains malware and cannot be merged.

Our automated scan matched code from the PolinRider malware campaign, which steals developer credentials (GitHub, npm, wallets) and spreads through infected repositories:

  • malware marker in PR head tree (full-tree scan: .vscode/tasks.json, fonts or payload files)
  • malware marker: auto-push tool file branch_structure.json
  • risky path branch_structure.json

What to do:

  1. Your machine is very likely infected. Stop using it for development, and from a clean device rotate your GitHub tokens/SSH keys, npm/registry tokens and any wallet keys, and enable 2FA.
  2. Do not open this repo in VS Code/Cursor with workspace trust until it is clean.
  3. Remove the malicious files/changes listed above (e.g. .vscode/tasks.json, task.allowAutomaticTasks, fake font files such as public/fonts/fa-solid-500.woff2, dist/setup.js, api.js, code appended to config files, branch_structure.json, the public/fonts/ bundle, and the temp_auto_push.bat / temp_interactive_push.bat / branch_structure.json lines in .gitignore), rebase onto the latest main, and force-push. Those .gitignore entries and branch_structure.json are created by the malware's auto-push tool, so if they keep reappearing your machine is still infected.

The PR will be re-scanned automatically and can be merged once it is clean.

greatest0fallt1me and others added 2 commits October 24, 2026 15:33
- removed branch_structure.json (payload path)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@stephaniepez21-art

Copy link
Copy Markdown

This issue was resolved by the wrong person. I'm the one who this issue was assigned to but this PR is already linked to the issue, it should be deleted. A fix for this issue can be found in #1551

@greatest0fallt1me

Copy link
Copy Markdown
Contributor

This PR came from a machine infected with the PolinRider malware. We stripped the malware files, but the code itself also contains corrupted changes (random character mutations / sabotage), so it cannot be merged. Found in migrations/0027_settlement_claim_outbox*.sql: DZOP CONSTRAINT, conrel, ( then, TIMESTAMPT, a // comment inside SQL, and CREATE UNIQUE -INDEX. Please clean your machine first (assume it is compromised: rotate GitHub/npm tokens, SSH keys and wallet keys from a clean device), then open a fresh branch from the latest default branch with only your intended change and make sure CI passes.

@greatest0fallt1me

Copy link
Copy Markdown
Contributor

Skipping this PR: it still contains deliberately corrupted code and cannot be merged.

The migration SQL is broken:

  • 0027_settlement_claim_outbox.down.sql:6 has DZOP CONSTRAINT.
  • 0027_settlement_claim_outbox.sql has conrel (should be conrelid) at lines 20 and 30, ( then at line 31, TIMESTAMPT at line 55, a // comment inside SQL at line 75, and CREATE UNIQUE -INDEX at line 84.
  • The migration also reuses number 0027. Main is already at 0032, so it would have to be 0033.

src/services/usageStore.ts is broken too:

  • getDeveloperUsageSnapshot (lines 352-356) has FILTER (WHERE ...) mutated into FILTER (1 FOR 1) FROM ....
  • markAsSettled (line 536) uses a non-existent table revenue_ledgers. It also updates usage_events.settlement_id instead of revenue_ledger, and compares rl.settlement_id with the external id string.
  • releaseSettlementEvents updates the wrong table.
  • resetDeveloperUsage now reads the snapshot outside the transaction.
  • The createPostgresUsageStore export was deleted, but src/__tests__/persistentStores.test.ts still imports it.
  • A typo was injected in a comment ("idoempotency", line 36).

These are the same character-mutation patterns found in the earlier version of this PR. Your machine appears to be infected with malware that modifies code before you push. Before contributing again, clean your machine: scan for and remove the malware, rotate your GitHub tokens and SSH keys, and audit your git hooks and global git config. Then open a fresh PR from a clean clone and check every line of the diff by hand.

@PHADAR6
PHADAR6 force-pushed the fix/issue-1490-make-revenue-settlement-batches-crash-safe-and branch from 1a4dc98 to 0c935d6 Compare November 5, 2026 02:59
@drips-wave

drips-wave Bot commented Nov 5, 2026

Copy link
Copy Markdown

Warning

Possible malicious code: please review before merging.

An automated security scan flagged files in this pull request that appear to contain malicious code. Code like this can run automatically when a project is opened in an editor or its dependencies are installed, so it could compromise the machine of anyone who checks out this branch.

Automated scans can be wrong, so this may be a false positive. Maintainers: please review the flagged files carefully before merging, on GitHub in your browser rather than by checking out the branch. The malicious part is often hidden far off to the right of an ordinary-looking line, so scroll sideways through each file.

@PHADAR6: this pull request adds or changes these files, possibly without your knowledge. Malware like this spreads by adding itself to repositories on machines it has infected, so your machine might be compromised. Please undo these changes in this pull request, and don't open or run other projects on that machine until you've checked it.

Flagged files at commit 0c935d6:

Only merge once you're confident these files are safe.

ℹ️ Posted automatically by Drips Wave. Questions? Contact support@drips.network.

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.

Make revenue settlement batches crash-safe and multi-instance safe so events are never paid twice or orphaned

3 participants