fix(simple-token): resolve CIP-0056 internal-review findings (56-1 – 56-6 + 56-Q1)

[amarzeppelin]

Summary

Resolves all findings from the CIP-0056 internal review of the simple-token reference — two
correctness fixes, three cheap hardenings, and three documented scope decisions. Every code change
has a regression test. daml test → 48/48 scripts ok (SDK 3.4.10).

Finding	Severity	Class	Resolution
56-4 direct-transfer receiver redirect	LOW/MEDIUM	fund misdirection	bind the preapproval to transfer.receiver
56-6 forged-holding supply inflation	MEDIUM	unbacked mint	reject non-registry holding templates as inputs
56-1 holding ↔ instrument admin binding	NOTE	integrity	ensure admin == instrumentId.admin
56-5 non-atomic DvP test	NOTE	test correctness	settle legs in one tx + rollback test
56-3a TransferPreapproval_Send trusts totalInput	NOTE (C)	defense-in-depth	assert totalInput >= amount
56-3 SimpleAllocationRequest validation	NOTE	scope	document as unused demo stub; checklist [FUTURE]
56-2 per-sender preapproval scoping	NOTE	scope	keep open-by-default; scoped variant [FUTURE]
56-Q1 settlement atomicity model	Question	scope	document trusted-executor atomicity

Guided by the repo scope: protect the honest happy path and untrusted-actor exploits; harden reusable
primitives cheaply; document scope boundaries rather than gold-plate. Trusted roles (admin / executor /
relayer) are inside the trust model.

---

Correctness fixes

56-4 — direct-transfer receiver redirect (LOW/MEDIUM). transferFactory_transferImpl reads the
preapproval id from untrusted choice context and passed it to directTransfer, which minted the
holding for the preapproval's own receiver — never checked against transfer.receiver. Whoever
assembled the context could redirect an honest transfer's funds. Fix: TransferPreapproval_Send gains
expectedReceiver and asserts receiver == expectedReceiver; directTransfer passes transfer.receiver.

56-6 — forged-holding supply inflation (MEDIUM). archiveAndSumInputs trusted the shared Holding
interface view of its inputs; a foreign template with a spoofed view could inflate totalInput and
cause a real, backed holding to be minted for a fake amount. Fix: each input must down-cast
(fromInterface) to SimpleHolding / LockedSimpleHolding; foreign templates are rejected. (The
regression test demonstrates a real unbacked mint on the pre-fix path — severity MEDIUM.)

Cheap hardenings

56-1 — holding ↔ instrument admin binding (NOTE). ensure amount > 0.0 && admin == instrumentId.admin
on both holding templates, so a holding can't advertise an instrument it isn't backed by. Distinct from
56-6 (a foreign look-alike never runs our ensure; 56-6's gate covers that).

56-5 — non-atomic DvP test (NOTE). test_dvpTwoLegs executed the two legs in two separate
transactions. Now both Allocation_ExecuteTransfers run in one submission (applicative *>), and
test_dvpAtomicRollback proves that a failing leg rolls back the other (balances unchanged).

56-3a — TransferPreapproval_Send trusts totalInput (NOTE, defense-in-depth). Unlike
SimpleTransferInstruction / SimpleAllocation (which derive totalInput from the fetched+archived
locked holding, i.e. self-backing), this choice mints from a caller-supplied totalInput. It now asserts
totalInput >= amount, mirroring the factory's invariant #18, so a direct caller can't mint a receiver
holding it hasn't backed. The residual (an admin-authorized caller lying about totalInput) requires the
trusted registry's own signature and is out of the core threat model.

Documented scope decisions (no behaviour change)

56-3 — SimpleAllocationRequest. Has zero references in the package; the real flow is
AllocationFactory → SimpleAllocation. It ships only to witness the standard AllocationRequest
interface and has no-op choice bodies. Documented as a demo stub; production-hardening checklist recorded
[FUTURE].

56-2 — preapproval sender scoping. TransferPreapproval is open-by-default by design (any vetted
sender may pay the receiver). Per-sender scoping is a deliberate opt-in extension (ScopedTransferPreapproval),
[FUTURE] — the default is not changed.

56-Q1 — settlement atomicity. Documented (module doc + tests): atomic multi-leg DvP relies on a
trusted executor batching each leg's Allocation_ExecuteTransfer into one transaction; no on-ledger
settlement coordinator, by design.

---

Tests (simple-token-test)

New module SimpleToken/Test/ReviewFindings.daml:

• test_directTransferReceiverRedirectRejected (56-4)
• test_forgedHoldingInputRejected (56-6) + test_genuineHoldingInputStillWorks (positive control)
• test_holdingAdminMustMatchInstrumentAdmin (56-1)
• test_preapprovalTotalInputMustCoverAmount (56-3a)

SimpleToken/Test/Allocation.daml: test_dvpTwoLegs made atomic; new test_dvpAtomicRollback (56-5).
SimpleToken/Test/Negative.daml: Test 28 updated for the new expectedReceiver arg.

daml test → 48/48 scripts ok.

How to review

• Preapproval.daml — expectedReceiver + receiver assert (56-4), totalInput >= amount (56-3a), open-by-default doc (56-2).
• Rules.daml — directTransfer passes receiver (56-4); archiveAndSumInputs template gate (56-6).
• Holding.daml — admin↔instrument ensure (56-1).
• Allocation.daml — trusted-executor atomicity doc (56-Q1).
• AllocationRequest.daml — demo-stub scope doc (56-3).
• simple-token-test/.../{ReviewFindings,Allocation,Negative}.daml — tests.

Notes

• TransferPreapproval_Send gains a field (source-breaking for direct callers); the one in-tree caller and
  the one test call-site are updated.
• The identical simple-token reference also lives in the sibling OZ repo
  (canton-stablecoin ⇄ canton-token-template); the same change is opened there to keep the two byte-identical.

🤖 Generated with Claude Code