Skip to content

fix(realunit): complete all quotes of a batch settlement tx - #4454

Merged
TaprootFreak merged 2 commits into
developfrom
fix/realunit-batch-settlement-completion
Jul 29, 2026
Merged

fix(realunit): complete all quotes of a batch settlement tx#4454
TaprootFreak merged 2 commits into
developfrom
fix/realunit-batch-settlement-completion

Conversation

@Blume1977

@Blume1977 Blume1977 commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Problem

The issuer may settle multiple purchases in a single on-chain tx containing one transfer event per purchase (batch settlement). completeSettledQuotes deduplicated consumed settlements per tx hash (assumption in the entity comment: "each settlement tx may complete at most one request per user"), so only the first quote of a batch was ever completed — all remaining quotes of the same tx stayed in WaitingForPayment forever and the app kept showing a pending payment, although the shares had long arrived.

Real case: userData 412710 — two purchases (300 CHF → 219 shares, 30'000 CHF → 22'047 shares) were settled by the issuer in one tx (0xec82fdb0…f940, two transfer events). The 300 CHF quote (2222184) consumed the tx hash; the 30'000 CHF quote (2222199) could never match afterwards.

Fix

Consumption is now tracked per transfer event instead of per tx. A consumed event is identified by its (tx hash, share amount) pairing and counted (multiset), which also covers batches containing several same-amount settlements. getUsedSettlementTxIds becomes getUsedSettlements and additionally returns the request's estimatedAmount, from which the consumed event of already completed requests is reconstructed.

  • settlementTxId semantics unchanged (plain tx hash, still links to the explorer), no migration needed
  • Self-healing: stuck quotes of this class complete automatically on the next cron run after deploy — for 2222199 this works until 31.07. 03:00, when the 7-day expiry cron (txRequestWaitingExpiryCheck, EVERY_DAY_AT_3AM) resets it to Created (after that it would need manual completion)
  • estimatedAmount is never updated for buy quotes (updateEstimatedAmount is only called for sell/swap payment infos), so the reconstruction is stable

Known limitation — deliberate, tracked in #4459

The amount-based reconstruction is a stopgap, not the target design. It was chosen on the assumption that the on-chain history carries no per-event id — that assumption is wrong: the indexer already assigns every transfer a stable identity (transferId = ${blockNumber}-${logIndex}, exposed on accountHistory as id/transferId, see DFXswiss/RealUnit-ponder). It is simply not selected in accountHistoryQuery, so it never reaches HistoryEventDto.

Matching on the amount therefore carries three dependencies that exact id matching would not have: rounding via Math.floor, the unpaginated 100-event history window, and the implicit coupling to updateEstimatedAmount never touching buy quotes. None of them break the case above, which is why this ships now — the stuck quote heals only until 31.07. Selecting the event id, persisting it and dropping the heuristic is tracked as #4459 and is a net simplification.

Tests

  • 6 new specs: batch tx completes multiple quotes; healing with a legacy consumed settlement; same-amount transfer not reused across runs; second same-amount quote completes only when the batch contains two matching transfers; consumed transfer not being the first event of the batch tx
  • The last of these closes a real gap: with the settlement key reduced to the plain tx hash — i.e. the semantics this PR replaces — every other spec stayed green, so the amount component of the pairing was unverified. Removing the position skip turns 3 specs red; reducing the key turns only the new spec red.
  • Existing specs unchanged in semantics (one adapted to the new service method)
  • format:check, lint, type-check clean; jest payment + realunit suites: 485 passed

Incidental changes

  • Util.sort replaces the hand-rolled timestamp comparator (identical semantics: same default ASC direction, Number(Date) equals .getTime(), still operating on the fresh array from history.filter(...), so the address-keyed history cache stays untouched)
  • the private helpers moved behind a // --- HELPER METHODS --- // marker with the public cron methods first, matching the sibling job services

The issuer may settle multiple purchases in a single on-chain tx with one
transfer event each. Quote completion deduplicated consumed settlements per
tx hash, so only the first quote of a batch was ever completed and the
remaining quotes were stuck in WaitingForPayment although the shares had
arrived. Consumption is now tracked per transfer event, identified by its
(tx hash, share amount) pairing and counted, which also reconstructs the
consumed event of already completed requests and thereby heals stuck quotes
on the next cron run.
@github-actions

Copy link
Copy Markdown

⚠️ Unverified Commits (1)

The following commits are not signed/verified:

  • 51118d8 fix(realunit): complete all quotes of a batch settlement tx (Blume1977)
How to sign commits
# SSH signing (recommended)
git config --global gpg.format ssh
git config --global user.signingkey ~/.ssh/id_ed25519.pub
git config --global commit.gpgsign true

# Re-sign last commit
git commit --amend -S --no-edit
git push --force-with-lease

Reducing the settlement key to the plain tx hash — the very semantics this
branch replaces — left all existing specs green, so the amount component of
the pairing was never exercised. Add the case a tx-hash-only match gets
wrong: the already consumed transfer is not the first event of the batch tx.

Also use Util.sort instead of a hand-rolled comparator and move the private
helpers behind the HELPER METHODS section marker, as in the sibling job
services.
@TaprootFreak

Copy link
Copy Markdown
Collaborator

Completed the mandatory pre-review process: 2 review passes (conformity and logic, each against the full diff; the second pass ran against the final commit) until zero open findings.

What the passes changed:

  • Test gap closed. Reducing the settlement key to the plain tx hash — the exact semantics this PR replaces — left every existing spec green, so the amount component of the pairing was never actually exercised. Added the case a tx-hash-only match gets wrong: the already consumed transfer is not the first event of the batch tx. Verified by mutation: removing the position skip turns 3 specs red, reducing the key turns only the new spec red.
  • Util.sort replaces the hand-rolled comparator (semantics verified identical; it sorts the fresh array from history.filter(...), so the history cache is unaffected).
  • Helper placement now follows the repo convention (public cron methods first, then the // --- HELPER METHODS --- // marker).

One reported point was declined with reasoning: as any on the new mockResolvedValue fixture. It matches the established convention of this file (9 occurrences predating the PR) and of the repo (110 of 329 spec files); a single deviating line would reduce consistency rather than improve type safety. Worth addressing repo-wide, not here.

Also documented during review: the design premise that the on-chain history carries no per-event id does not hold — the indexer assigns every transfer a stable ${blockNumber}-${logIndex} identity. This PR ships as a stopgap because the stuck quote only heals until 31.07. 03:00; replacing the amount heuristic with exact event-id matching is tracked in #4459.

Gates: format:check, lint, type-check clean, payment + realunit suites 485 passed, CI green.

@TaprootFreak
TaprootFreak marked this pull request as ready for review July 29, 2026 15:56
@TaprootFreak
TaprootFreak merged commit ccc6a01 into develop Jul 29, 2026
12 checks passed
@TaprootFreak
TaprootFreak deleted the fix/realunit-batch-settlement-completion branch July 29, 2026 16:03
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.

2 participants