Skip to content

Transfer and allocation aborts fail when the escrow was already reclaimed by lock expiry #48

Closed

Description

@lmcorbalan

Description

A two-step transfer leaves two contracts on the ledger: an escrow LockedToken holding
the sender's funds, and a TokenTransferInstruction that points at it. An allocation is
the same shape: an escrow LockedToken plus a TokenAllocation that points at it.

Both escrows are owned by the sender, hold holders = [admin], and expire at the
settlement deadline (transfer.executeBefore for a transfer, settlement.settleBefore
for an allocation). That means the sender has two ways to get the funds back once the
deadline passes, and they open at the same instant:

  • consume the record (TransferInstruction_Withdraw, Allocation_Withdraw), which
    returns the funds and archives the record, or
  • reclaim the escrow directly with LockedToken_ExpireLock, which returns the funds and
    knows nothing about the record.

Taking the second path leaves the record active and pointing at an archived escrow. Every
abort choice on that record then fails, because all four of them reach for the escrow
unconditionally:

  • Instruction.daml:67 reject calls unlockHolding lockedCid
  • Instruction.daml:74 withdraw calls reclaimAtDeadline lockedCid transfer.executeBefore
  • Allocation.daml:48 withdraw calls reclaimAtDeadline lockedCid settleBefore
  • Allocation.daml:55 cancel calls abortEscrow lockedCid settleBefore ...

No value is at risk and there is no double-spend: the funds are already back with the
sender. What remains is a record nobody can clear, which a client can still see and act
on, and whose abort choices fail with an unactionable NotActive contract error rather
than a message.

This is a divergence from the reference implementation, not an inherent property of the
two-template design. See Proposed fix below.

Steps to reproduce

  1. Start a two-step transfer, producing a pending instruction and a sender-owned escrow.
  2. Let transfer.executeBefore pass.
  3. As the sender, exercise LockedToken_ExpireLock on the escrow instead of withdrawing.
  4. Attempt TransferInstruction_Reject (receiver) or TransferInstruction_Withdraw
    (sender) on the still-active instruction.
  5. Repeat with an allocation, using settleBefore and Allocation_Withdraw /
    Allocation_Cancel.

Expected vs actual behavior

Expected: an abort clears the record whether or not the escrow is still there. The
sender's funds are returned exactly once, and the deadline still governs when a
single-party abort is allowed.

Actual: the record stays active and its abort choices fail. Reproduced with a
throwaway script against the tree at 5052460:

RECLAIMED=40.0   INSTRUCTION_STILL_ACTIVE=True
WITHDRAW = Could not find contract ... NotActive
REJECT   = Could not find contract ... NotActive
ACCEPT   = "transfer has expired (executeBefore reached)"

Note the accept line: the settle paths (transferInstruction_acceptImpl,
allocation_executeTransferImpl) already refuse past their own deadline for an unrelated
and correct reason, so this issue is about the four abort paths only.

Proposed fix

Adopt the upstream shape, adapted so it cannot be abused.

The registry already answers a choice-context request before every abort, and it already
looks the escrow up (registry/src/routes/transfer.ts:104,
registry/src/routes/allocation.ts:52). When the escrow is gone it answers 404 today.
Instead, it should say so in the choice context, and the choice should skip the escrow:

  • a new well-known key canton-token-forge/escrow-reclaimed (AV_Bool) meaning "there is
    no escrow left to return"
  • the four abort impls branch on it: with the key, assert the settlement deadline has
    passed and return an empty senderChangeCids; without it, behave exactly as today
  • the deadline assert is extracted from reclaimAtDeadline so both branches share one
    rule and one message
  • Allocation_Cancel gives the new key precedence over the existing
    canton-token-forge/expire-lock early-release key, since there is nothing to release

Why the flag cannot be abused, which is what makes this different from upstream's
version.
It never bypasses a deadline: both branches assert it. A sender who sets it
while the escrow is still live archives their own record and leaves their own funds
locked, recovering them at the deadline through LockedToken_ExpireLock. A receiver who
sets it on reject archives the record while the sender's escrow stays put and stays
reclaimable, which is no worse for the sender than the receiver never rejecting. This
preserves the property that 04c4f7d was written to protect: a caller-suppliable boolean
never releases an escrow ahead of its deadline on a single-party choice.

This fix depends on LockedToken_ExpireLock remaining available to the owner, since it is
the backstop in both cases above.

Alternatives considered

  • Refuse LockedToken_ExpireLock on an escrow (add a field to LockedToken, set it at
    both escrow sites, keep expiry for standalone InstrumentConfig_LockHolding locks). The
    orphan becomes unconstructible and the surface is smaller. Rejected because it removes
    the backstop that makes the proposed fix safe, and because it is the opposite of what
    upstream does: they keep owner-side expiry routine and make abort tolerate it.
  • Adopt upstream's branch verbatim, where a post-deadline abort archives the record and
    returns nothing, leaving the sender to reclaim separately. Rejected because it costs the
    one-step withdraw on the common path. Amulet can afford it because it has DSO-driven lock
    expiry and auto-unlock of expired locks used as transfer inputs; we have neither.
  • Link the two contracts so consuming one archives the other. Not implementable here:
    the escrow is created before the record, LF 2.1 has no contract keys, and a failed
    fetch cannot be caught, so the escrow cannot learn the record's id after the fact.
  • Document it and close. Already effectively done (SPEC.md:419, Registry.daml:263,
    registry/src/routes/lookup.ts:75), and it leaves us less robust than the reference
    implementation in a state upstream considered worth handling.

Acceptance criteria

Daml:

  • canton-token-forge/escrow-reclaimed is defined once, beside the existing
    expireLockContextKey, with a lookupEscrowReclaimed reader that treats an absent
    or non-boolean value as False
  • the settlement-deadline assert is extracted so the escrow-touching and
    escrow-skipping branches share one rule and one failure message
  • transferInstruction_rejectImpl skips the escrow when the key is set, and still
    asserts executeBefore has passed in that branch
  • transferInstruction_withdrawImpl skips the escrow when the key is set, and remains
    deadline-gated in both branches
  • allocation_withdrawImpl and allocation_cancelImpl do the same against
    settleBefore, with the new key taking precedence over expire-lock on cancel
  • the settle paths (transferInstruction_acceptImpl, allocation_executeTransferImpl)
    are unchanged
  • the unflagged behaviour of all four abort paths is byte-identical to today,
    including reject staying unconditional before the deadline

Registry service:

  • the transfer reject and withdraw choice-context routes answer 200 with the new
    key set and no disclosure when the escrow is absent, instead of 404 escrow not found
  • the allocation withdraw and cancel choice-context routes do the same
  • the transfer accept and allocation execute-transfer routes keep answering 404
    when the escrow is absent, since those choices genuinely need it
  • a present escrow still yields today's response on all six routes

Tests:

  • a Daml scenario builds the orphan for the transfer path (expire the escrow, then
    reject and withdraw) and asserts both now succeed and return no holdings
  • the same for the allocation path (withdraw and cancel)
  • a scenario asserts the flagged branch still refuses before the deadline, so the key
    cannot be used to abort early
  • registry unit tests cover the four rerouted contexts and the two that still 404
  • every new assertion is mutation-checked to redden exactly the scenario that names it

Docs:

  • the SPEC.md "inert instruction" gap bullet is removed or rewritten to match
  • the findEscrow comment in registry/src/routes/lookup.ts and the escrow comment at
    Registry.daml:263 describe the new behaviour
  • ARCHITECTURE.md's choice-context section lists the new key alongside
    transfer-preapproval and expire-lock

Technical notes

  • Upstream hit this and solved it off-ledger. abortTwoStepTransfer
    (deps/splice-daml/splice-amulet/daml/Splice/Amulet/TwoStepTransfer.daml:126-134) reads
    expire-lock from the choice context: true unlocks the amulet, false asserts the
    deadline and returns [] without touching the lock. Their reference registry computes
    that boolean by looking the lock up, with the comment "check whether the locked amulet is
    still active, so that aborts still work even if the owner of the LockedAmulet expired it"
    (deps/token-standard/splice-token-standard-test/daml/Splice/Testing/Registries/AmuletRegistry.daml:260-282,
    and again for allocations at :285-305).
  • Their invariant holds here too. That handler treats "lock archived while the transfer
    is not yet expired" as impossible, because owner-side expiry is deadline-gated. Our
    escrows expire at exactly executeBefore / settleBefore, so the same reasoning applies
    and the flagged branch can always assert the deadline.
  • Their flag is looser than ours needs to be. Amulet's true branch skips the deadline
    entirely, so their boolean can be forged to pull funds back mid-window. Ours must mean
    only "there is nothing to return", never "release early".
  • AV_Bool has never been exercised against a live participant. The direct-transfer
    path verified AV_ContractId under Complete a direct transfer using the service's choice context verbatim #86; this is the first change that would put a
    boolean choice-context value on the wire, so an e2e test here closes that gap.
  • The two escrow creation sites are Registry.daml:267 (transfer) and Allocation.daml:80
    (allocation); both are owner = sender, holders = [admin], expiresAt = the
    settlement deadline.
  • An orphaned record is not permanently unarchivable today: admin and sender are its
    signatories and can archive it out of band. That is a manual recovery, not a client path.

Activity

  1. changed the title [-]Reclaiming an escrow by lock expiry leaves an orphaned transfer instruction[/-] [+]Transfer and allocation aborts fail when the escrow was already reclaimed by lock expiry[/+] on Aug 19, 2026
  2. lmcorbalan commented on Aug 19, 2026

    @lmcorbalan
    CollaboratorAuthor

    Re-scoped 2026-08-19 after grading the issue against main at 5052460 and reproducing it
    with a throwaway Daml script. The original text is superseded by the body above; recording
    what changed and why.

    Confirmed. The orphan is constructible today. The probe expired a 40.0 escrow after
    executeBefore, left the instruction active, and both reject and withdraw failed with
    Could not find contract ... NotActive.

    Three corrections.

    1. "Every choice on it fails" conflated two causes. Accept fails on its own expiry assert
      (Instruction.daml:40) and would fail with or without the escrow, so it is not evidence
      of this defect. Only the abort paths fail because of the dangling escrow.
    2. The same defect exists on the allocation path, which the original text never mentioned.
      Allocation.daml:80 builds an identical escrow, and both Allocation_Withdraw and
      Allocation_Cancel route through it. A transfer-only fix would leave half the defect in
      place.
    3. The original "two ways out" (link the contracts, or document withdrawal as canonical)
      missed the exit upstream actually takes, which is to let the registry tell the choice
      whether an escrow is still there. That is now the proposal, hardened so the flag cannot
      bypass a deadline.

    Retitled because the old title named the trigger (lock expiry) rather than the defect
    (the aborts that cannot cope with it), and it read as transfer-only.

    Not taken: refusing LockedToken_ExpireLock on an escrow. It would make the orphan
    unconstructible with a smaller change, but it removes the backstop that makes the proposed
    fix safe, and it inverts upstream's model. Recorded under Alternatives considered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingpriority: lowNice to have, can wait

    Type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions