feat: Handle partial sweep failure on contract-authorized / Horizon-failed path (#169)#268
Merged
phertyameen merged 3 commits intoJul 1, 2026
Conversation
…gelet-org#169) When the SweepController contract execute_sweep() succeeds but the subsequent Horizon Operation.payment fails, the contract is left in Swept state with no funds moved. Previously this was logged as a CRITICAL error and required manual recovery. Now: - SweepsService.executeSweep catches the Horizon payment failure and returns a structured partial result { isPartial: true, error } instead of throwing. - ClaimRedemptionProvider accepts PARTIAL_SWEEP as a re-entrable state, transitions the account to it on a partial outcome, records outcome=partial on the audit log, and fires a sweep.partial webhook. - A subsequent redemption attempt on a PARTIAL_SWEEP account passes skipContractAuth=true so the contract is NOT re-invoked (it is already in Swept state and re-invocation would revert); only the Horizon payment is retried. Stellar sequence numbers protect against duplicate payment on retry. - New migration adds the partial_sweep value to the account_status_enum PostgreSQL enum. Closes bridgelet-org#169
|
@dzekojohn4 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! 🚀 |
Contributor
|
@dzekojohn4 YOur CI is failing and resolve conflict along with the CI please |
- Add 'partial' outcome to ClaimOutcome union so audit logs can record the new contract-authorized-but-payment-failed outcome. - Update sweep spec to remove the obsolete 'propagates TransactionProvider errors' assertion; the sweeper now returns a structured isPartial result instead of throwing. - Reset mutable fields on the shared mockAccount in claim-redemption.spec so prior test mutations do not leak, and fix the failing partial-redemption test (mock a partial sweep result rather than the full success result). - Bump expected migration count to 9 and register the new AddPartialSweepToAccountStatus migration in the integration runner.
Contributor
|
@dzekojohn4 Can you please resolve conflict? |
…elet-org#167 and bridgelet-org#169 blocks) Three-way merge with upstream/main (7ee4689) flagged a single content conflict in src/modules/claims/providers/claim-redemption.provider.spec.ts. Both sides added a new top-level describe('\''redeemClaim - ...'\'''') block at the same insertion point (right after the existing sweep-failure block): - ours (issue bridgelet-org#169) added '\''redeemClaim - partial sweep path'\'' with six tests covering the new PARTIAL_SWEEP entry + isPartial result + audit/webhook/return shape + revert-on-throw behavior. - theirs (issue bridgelet-org#167) added '\''redeemClaim - propagates TokenVerification errors'\'' with three tests asserting BadRequestException propagation from verifyClaimToken. Resolution: keep both describe blocks. Order is theirs (bridgelet-org#167) before ours (bridgelet-org#169) so error-propagation tests precede partial-sweep behavior, matching the rough chronological feature order. Also preserved from this branch: - beforeEach now resets mockAccount.{status, destinationAddress, claimedAt} to avoid leakage between tests where the provider mutates the locked record in-place. - The success-redemption toHaveBeenCalledWith matcher now includes skipContractAuth: false explicitly so the test breaks loudly if the provider ever stops passing that flag on a fresh attempt. Verification: - brace balance: 109 open / 109 close. - jest on this spec: all 23 tests pass (3 new from bridgelet-org#167 + 6 new from bridgelet-org#169 + 14 prior). - tsc --noEmit on this file: clean. Resolves the merge conflict on PR bridgelet-org#268.
phertyameen
approved these changes
Jul 1, 2026
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
When the SweepController authorises a sweep but the subsequent Horizon
payment fails, the contract is in
Sweptstate and no funds havemoved. The previous behaviour logged a
CRITICALerror and requiredmanual recovery. Capture the partial outcome on the account instead so
a retry of the same claim token only re-submits the Horizon payment
(skipping
execute_sweep, which would revert) and finishes the sweep.AccountStatus.PARTIAL_SWEEP+ migrationAddPartialSweepToAccountStatus.SweepResultgainsisPartial?: booleananderror?: string;txHash/timestampbecome optional.SweepExecutionRequestgainsskipContractAuth?: boolean.SweepsService.executeSweepno longer throws on Horizon payment failure — it returns{success: false, isPartial: true, error, contractAuthHash}. WhenskipContractAuth=true, steps 2-3 (auth +execute_sweep) are skipped and only the Horizon payment runs.ClaimRedemptionProvidercaptureswasPartialOnEntryfrom theSELECT FOR UPDATEtx, acceptsPARTIAL_SWEEPas a re-entrable state, threadsskipContractAuththrough, persists the new partial sweep path (accountsRepository.update+ auditoutcome=partial+sweep.partialwebhook), and the catch-block revert now preservesPARTIAL_SWEEPwhen the entry state was partial.skipContractAuthx3,isPartialx2 sweeps + 4 new partial-sweep cases in the orchestrator.Testing
npx tsc --noEmitclean on all touched dirs.npx prettier --checkclean on all touched files.claim-redemption.provider.spec.ts: 20/20 passingsweeps.service.spec.ts: 17/17 passingOperational notes
sweep.partialwebhook event with{accountId, amount, asset, destination, error, contractAuthHash}so they can decide whether to retry, refund, or escalate.{success: false, isPartial: true, error, contractAuthHash}so callers can distinguish recoverable partial failure from a genuine error.SELECT FOR UPDATEpessimistic_write; Stellar sequence numbers are monotonic so double-payment is impossible.PARTIAL_SWEEPDB status is sticky: a re-submit on the same token stays inPARTIAL_SWEEPuntil the Horizon payment succeeds (or an operator intervenes).Closes #169
Closes #172
Closes #173