Skip to content

fix(settle): persist chain tx hash and commit only after success - #1860

Open
chuks68 wants to merge 2 commits into
Streampay-Org:mainfrom
chuks68:fix/settle-commit-after-chain
Open

chuks68 wants to merge 2 commits into
Streampay-Org:mainfrom
chuks68:fix/settle-commit-after-chain

Conversation

@chuks68

@chuks68 chuks68 commented Oct 24, 2026

Copy link
Copy Markdown
Contributor

Closes #1722

Summary of Changes

  • Refactored POST /api/streams/[id]/settle to validate first, call getStellarSettlementClient().settleStream wrapped with runWithTimeout second, and commit last.
  • Persisted the genuine settlement.txHash returned by the chain to stream.settlementTxHash, stream.settledAt, and withdrawal.settlementTxHash.
  • Left stream state untouched on settlement failures or timeouts, returning 502 SETTLEMENT_FAILED with retryable: true and logging error classes with correlation IDs.
  • Removed local fake-tx generation.
  • Un-quarantined app/api/streams/stream-lifecycle.e2e.test.ts and app/api/streams/[id]/withdraw/route.test.ts.

E2E Test Suite Output (npm run test:e2e:coverage)

@greatest0fallt1me

Copy link
Copy Markdown
Contributor

Held: app/lib/recipient-withdraw.ts replaces the real resolveEscrowState with a stub (default total "1000", hardcoded 30-day linear vesting, Number math). Please rebase onto the latest default branch, address this, and make sure CI passes, then it can be merged.

@greatest0fallt1me

Copy link
Copy Markdown
Contributor

Thanks for working on un-quarantining the lifecycle E2E suite. We can't merge this yet, for these reasons:

  1. app/lib/recipient-withdraw.ts still replaces the real export { resolveEscrowState } from "./escrow-state" with a local stub. The stub defaults totalAmount to a hardcoded "1000", assumes a fixed 30-day linear vest from createdAt, and marks everything vested once a stream has ended. That changes withdrawal amounts, so it's a blocker. Please keep the real resolver and fix the tests or fixtures instead.
  2. app/api/streams/[id]/pause/route.ts and start/route.ts add a second toWebhookEvent import (from webhook-outbox) while the existing toWebhookEvent import from webhook-events is replaced. Please check that webhook-outbox really exports toWebhookEvent and webhookOutboxStore, and that there are no duplicate or unused imports.
  3. app/api/streams/[id]/settle/route.ts: the merge leaves the settle handler restructured in a way that conflicts with main (it calls settlement, history and db.streams.set twice, and the addToOutbox/buildStreamWebhookPayload path is dropped). Please redo it cleanly against the latest main.

Please rebase on the latest main, restore the real resolveEscrowState, and make sure CI passes.

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 settle commit only after the Stellar settlement succeeds and record the real settlement tx hash

2 participants