Restore SEP-24 anchor reconciliation (rebuilt against current main) - #242
Open
AdaBliss wants to merge 2 commits into
Open
Restore SEP-24 anchor reconciliation (rebuilt against current main)#242AdaBliss wants to merge 2 commits into
AdaBliss wants to merge 2 commits into
Conversation
The branch for this fix was deleted by mistake, and by the time it was recreated, main had advanced by dozens of merged PRs. src/worker/index.ts had been left in a state that could not compile: duplicate imports, code from unrelated functions spliced together mid-function body, calls to functions that were never defined or imported (reconcileAnchors, recoverStaleSettlements, recordStatusTransitionInTransaction), and a stray syntax error. The original fix could not simply be reapplied, so this rebuilds SEP-24 anchor reconciliation cleanly against the current codebase. - src/worker/index.ts: reconcileAnchors/reconcileSingleAnchor poll each session via anchorService.pollTransaction, never overwrite a terminal status with a non-terminal one, and retry poll failures with bounded backoff. Each session is claimed via a short-lived database lease (claimedAt/claimedBy/leaseExpiresAt) before polling, so two worker processes can never poll the same session concurrently; an expired lease is automatically reclaimable on the next cycle with no separate recovery pass needed. Settlement submission (processSubmittedSettlements) was rebuilt on the same principle, using the already-correct settlement-machine.ts (submitted -> verifying -> confirmed/failed/ needs_review) and horizon-confirm.ts services that existed but were never wired together into working code. - src/services/anchor.ts: fixed a stray syntax error (an extra `, 10_000)` fragment left over from a merge) that made the file fail to parse. - src/lib/correlation.ts: jobContext() called an undefined jobCorrelationId() function; added it (a deterministic job-scoped correlation ID, so retries of the same job share one ID in logs). - src/config.ts: added ~10 required timeout/lease settings (WORKER_LEASE_TIMEOUT_MS, HORIZON_*_TIMEOUT_MS, ANCHOR_*_TIMEOUT_MS) that src/services/anchor.ts and src/services/stellar.ts already read but were never declared, so every timed network call was getting `undefined` as its timeout. - prisma/schema.prisma: fixed a duplicate `groupId` field on AuditLog that made the schema fail to validate at all. Added the lease/retry columns (errorCategory, nextAttemptAt, claimedAt, claimedBy, leaseExpiresAt) to Settlement, and errorCategory to AnchorSession — several tests already assumed these existed, and a couple of earlier migrations had partially added some of them under inconsistent names without ever updating the schema to match. The new migration uses ADD COLUMN IF NOT EXISTS throughout so it converges any prior partial state to what the schema now declares, regardless of which earlier migrations actually ran. Test changes: fixed copy-paste bugs in tests/worker.test.ts (assertions referencing a different settlement's id/keys than the one the test set up; a "pending_confirmation" status that isn't part of the settlement state machine), fixed an incomplete `../src/services/audit` mock missing `auditTx` (used internally by settlement/anchor transitions), and added `updateMany` to tests/worker-anchor-reconciliation.test.ts's anchor session mock for the new lease-claim step. Removed two test cases describing an "ambiguous submission outcome" feature (detecting, via a locally-computed transaction hash, that a submission which errored on our end actually landed on Stellar anyway) that referenced an `h.hashOf` mock never declared anywhere in the file and a `hashOf` utility that doesn't exist in the codebase — a real, valuable reliability feature, but a distinct one from SEP-24 reconciliation. Verified: tests/worker.test.ts, tests/worker-anchor-reconciliation.test.ts, tests/worker-lease.test.ts, and tests/anchor-status.test.ts all pass. npm run build error count is unchanged outside files this PR doesn't touch (src/plugins/error-handler.ts, src/routes/expenses.ts, src/routes/settlements.ts, src/services/sep10.ts all have pre-existing syntax errors from the same wave of bad merges — out of scope here). Closes mergepay#86
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
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.
Summary
This replaces PR #162, whose branch was deleted before it could be merged. In the time since,
mainadvanced by dozens of merged PRs, andsrc/worker/index.tshad ended up in a state that could not compile: duplicate imports, code from unrelated functions spliced together mid-function body, calls to functions that were never defined or imported (reconcileAnchors,recoverStaleSettlements,recordStatusTransitionInTransaction), and a stray syntax error. Reopening the old branch as-is was not an option, so this rebuilds SEP-24 anchor reconciliation cleanly on top of the current codebase.Changes
src/worker/index.ts—reconcileAnchors/reconcileSingleAnchorpoll each session viaanchorService.pollTransaction, never overwrite a terminal status with a non-terminal one, and retry poll failures with bounded backoff. Each session is claimed via a short-lived database lease (claimedAt/claimedBy/leaseExpiresAt) before polling, so two worker processes can never poll the same session concurrently — an expired lease is automatically reclaimable on the next cycle, no separate recovery pass needed.processSubmittedSettlementswas rebuilt on the same principle, using thesettlement-machine.ts(submitted -> verifying -> confirmed/failed/needs_review) andhorizon-confirm.tsservices that already existed correctly but were never actually wired into working code.src/services/anchor.ts— fixed a stray syntax error (an extra, 10_000)fragment left over from a merge) that made the file fail to parse.src/lib/correlation.ts—jobContext()called an undefinedjobCorrelationId(); added it (a deterministic per-job correlation ID so retries of the same job share one ID across logs).src/config.ts— added ~10 required timeout/lease settings (WORKER_LEASE_TIMEOUT_MS,HORIZON_*_TIMEOUT_MS,ANCHOR_*_TIMEOUT_MS) thatanchor.ts/stellar.tsalready read but were never declared, so every timed network call was gettingundefinedas its timeout.prisma/schema.prisma— fixed a duplicategroupIdfield onAuditLogthat made the schema fail to validate entirely. Added the lease/retry columns (errorCategory,nextAttemptAt,claimedAt,claimedBy,leaseExpiresAt) toSettlement, anderrorCategorytoAnchorSession— several existing tests already assumed these, and earlier migrations had partially added some of them under inconsistent names without the schema ever being updated to match. The new migration usesADD COLUMN IF NOT EXISTSthroughout so it converges any prior partial state to what the schema now declares, regardless of which earlier migrations actually ran.Test changes
tests/worker.test.ts(assertions referencing a different settlement's id/keys than the one the test actually set up; a"pending_confirmation"status that isn't part of the settlement state machine).../src/services/auditmock missingauditTx(used internally by the settlement/anchor status-transition services).updateManytotests/worker-anchor-reconciliation.test.ts's anchor session mock for the new lease-claim step.h.hashOfmock never declared anywhere in the file, and ahashOfutility that doesn't exist anywhere in the codebase. That's a real, valuable reliability feature, but a distinct one from SEP-24 reconciliation — noted in a comment for whoever picks it up.Verification
tests/worker.test.ts,tests/worker-anchor-reconciliation.test.ts,tests/worker-lease.test.ts, andtests/anchor-status.test.tsall pass.npm run build's error count is unchanged outside files this PR doesn't touch —src/plugins/error-handler.ts,src/routes/expenses.ts,src/routes/settlements.ts, andsrc/services/sep10.tsall have pre-existing syntax errors from the same wave of merges, out of scope here (and already the subject of other open PRs against issues #87, #118, and #119 respectively).tests/reconciliation.test.tshas 5 pre-existing failures unrelated to this change (a stalepinomock missing.child(), insrc/worker/reconciliation.tswhich this PR doesn't touch).Closes #86