Skip to content

Direct transfers are recommended up to the last millisecond of a preapproval's window and then abort on-ledger #72

Closed

Description

@lmcorbalan

Description

POST /registry/transfer-instruction/v1/transfer-factory decides between a
one-step direct transfer and a two-step offer by checking whether the
sender holds a matching, in-window TokenTransferPreapproval:

const now = Date.now()
const pre = preapprovalRows.find(
  (p) =>
    matchesInstrument(p.payload, instrumentId) &&
    p.payload.receiver === receiver &&
    Date.parse(p.payload.validFrom) <= now &&
    now < Date.parse(p.payload.expiresAt),
)

The decision is made against the registry's wall clock and its read
snapshot
. The ledger then re-asserts the same window at execution time:
TokenTransferPreapproval_Send checks validFrom <= now < expiresAt against
ledger time, and resolveDirect routes a fetched, matching preapproval to
PreapprovedDirect unconditionally, so a failed assertion aborts the whole
choice. The direct path has no offer fallback by design.

Three ways the two clocks disagree:

  1. Boundary. A sender requests a factory a few seconds before
    expiresAt. The registry answers direct. By the time the command is
    submitted and sequenced, ledger time is past expiresAt and the whole
    transfer is rejected.
  2. Skew. The registry host's clock runs ahead of or behind the
    synchronizer's. Every request near either edge of the window is decided on
    the wrong side.
  3. Archival. The receiver withdraws the preapproval between the registry's
    read and the sender's submission. resolveDirect's own doc comment says
    fetch pcid aborts in that case.

In all three the user-visible result is the same: a transfer that would have
succeeded as a two-step offer fails outright, with an on-ledger error rather
than a registry response, and no funds move.

This path only became reachable when the service started reading as the
instrument admin. Before that, activePreapprovals read as the operator, which
is not a stakeholder of TokenTransferPreapproval (signatory admin, receiver), so the query always returned empty and every non-self transfer was
answered offer.

Steps to reproduce

  1. Create a TokenTransferPreapproval for receiver::1 on instrument CC
    with expiresAt a few seconds in the future.
  2. POST /registry/transfer-instruction/v1/transfer-factory with
    { transfer: { instrumentId: { admin, id: 'CC' }, sender, receiver: 'receiver::1' } }.
  3. Observe transferKind: "direct" and the preapproval in
    choiceContextData.
  4. Wait for expiresAt to pass, then exercise TransferFactory_Transfer
    with that context.

Expected vs actual behavior

Expected: the registry only recommends the direct path when it will still
be valid by the time a reasonable client can submit it; otherwise it answers
offer, which always works.

Actual: the registry recommends direct up to the last millisecond of the
window, and the ledger rejects the submission.

Proposed fix

Require a safety margin rather than bare validity:

now + DIRECT_TRANSFER_MARGIN_MS < Date.parse(p.payload.expiresAt)

A preapproval inside the margin falls through to offer, which is always
correct and always succeeds. The margin needs a decision on its value: it
should cover client round-trip plus submission and sequencing latency, plus
tolerated clock skew. Something in the tens of seconds is the obvious starting
point, but the number is a product call, not a code-review call. Whatever is
chosen, put it in one named constant with the reasoning in a comment, and cover
it with a boundary test (just inside the margin falls through, just outside
takes the direct path).

The archival race (case 3) is not fixed by a margin. It is inherent to reading
a contract and submitting against it later, and the honest treatment is a
comment saying so plus the same offer-is-always-safe reasoning.

Environment

Registry HTTP service, registry/src/routes/transfer.ts; Daml side in
daml/canton-token-forge/daml/Canton/TokenForge/Registry.daml
(resolveDirect, TokenTransferPreapproval_Send).

Additional context

Found by the high-effort code review of the read-as-admin branch and verified
against both the TypeScript and the Daml source. Deliberately not fixed there:
picking the margin is a behavioral decision, and the branch's scope was the
read-party repoint.

The same branch added the three tests that pin which arms of the match filter
are load-bearing (instrument, receiver, validFrom), plus the pre-existing
expiresAt test, so a boundary test has somewhere obvious to live.

Overlaps #42 (end-to-end transfer against a live node), which is where a real
clock-skew scenario can actually be observed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority: mediumShould be addressed soon

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions