OpenZeppelin / OpenZeppelin/canton-specs
Reference architecture D1 snippets do not match either CIP-112 implementation (fetchByKey registry resolution, attestationCid as a direct choice field)
Nobody has claimed this yet.
- Dominant language
- Haskell
- Stars
- 3
- Forks
- 1
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 9
Description
Reviewed at canton-specs commit e4b36690e75f6df2cdea80889fb6c1203d29bed7 and
canton-contracts commit 7696749737885e25cd88422847105f890f03b00d (the exact commit the
reference architectures permalink into).
Three of the four reference architectures show a D1 settlement snippet whose registry resolution
and choice signature do not correspond to either published CIP-112 implementation. Because the
surrounding prose states the security property as a consequence of the mechanism shown, and
because the mechanism shown is unavailable on the targeted Daml LF version, I think this is worth
correcting rather than leaving to the general "illustrative" disclaimer at
docs/reference-architectures/dex.md:694.
1. Registry resolution shown as fetchByKey, but neither implementation uses contract keys
docs/reference-architectures/dex.md:810:
(_, registry) <- fetchByKey @TrustedAttesterRegistry factoryAdmin
The accompanying prose at dex.md:792-794 states:
It resolves its
TrustedAttesterRegistryby key (keyed by the factory admin), so the attester
must be trusted by the factory's own registry, not one the caller supplies.
The same claim appears as an inline comment in two further designs:
dex.md:770-771: "The factory resolves its TrustedAttesterRegistry by key, so no caller-supplied
registry is trusted."confidential-auction.md:562-564: same sentence.
Neither implementation resolves the registry by key. grep for fetchByKey, lookupByKey, and
key declarations across canton-contracts/experiments/token/ and
canton-specs/experiments/settlement/ returns no matches.
What the implementations actually do:
- canton-contracts: the registry is pinned as a
ContractIdon the rules contract,
Registry.daml:42(requiredAttesterRegistryCid : Optional (ContractId TrustedAttesterRegistry)),
passed to verification atRegistry.daml:392-397, and resolved with a plainfetchat
D1.daml:92. The "not one the caller supplies" property is real here, but it comes from the cid
being pinned on the rules contract, plus the explicit admin equality check atD1.daml:95. - canton-specs:
registryCidis a caller-supplied argument to
ComplianceAttestation_Verify(Cip112.daml:720), resolved withfetchat:724. The registry
here is caller-supplied; the defence is solely the admin equality assertion at:726. So for
this implementation the sentence "not one the caller supplies" describes the outcome but not the
mechanism, and the mechanism named is the one doing the work in the reader's mental model.
This matters beyond style because contract keys are not available on the targeted Daml LF version.
The sibling repository OpenZeppelin/canton-token-template records this in its own verification
report at docs/AUDIT.md:126-127 (commit c96456fa43e4e41d436be39c28ead00e28fd2397):
SimpleTokenRulesandTransferPreapprovallack contract keys, permitting duplicate instances.
Daml LF 2.1 dropped contract keys entirely.
and
Status: Acknowledged. Not patchable under Daml LF 2.1.
A reader building from the reference architecture would attempt a pattern that cannot compile on
that LF version, and would then have to rediscover the pinned-cid approach that
canton-contracts already uses.
2. attestationCid shown as a direct field of SettlementFactory_SettleBatch
dex.md:772-776, confidential-auction.md:565-569, and lending.md:599-603 all show:
exercise settlementFactoryId SettlementFactory_SettleBatch with
settlement; transferLegs
allocationCids = [...]
actors = [...]
attestationCid
SettlementFactory_SettleBatch is a Token Standard V2 interface choice, so its argument record is
fixed by the standard and has no attestationCid field. canton-contracts handles this by
transporting the attestation through the choice context: the key is defined at Base.daml:43-44
(ozNamespace <> "d1-attestation") and read at Registry.daml:388-391 out of
arg.extraArgs.context.values, failing closed when absent.
canton-specs takes a different route again: the attestation is a typed argument, but on a
separate choice, SettlementFactory_SettleBatchWithAttestation (Cip112.daml:218-225), not on
SettlementFactory_SettleBatch.
So the snippet matches neither. The choice-context mechanism is arguably the more interesting of
the two for a reader, since it is the general answer to extending a fixed standard interface, and
the current snippet hides it.
3. The ComplianceAttestation_Verify signature shown is incomplete for canton-contracts
dex.md:804-808 shows:
choice ComplianceAttestation_Verify : Text
with
settlement : SettlementInfo; transferLegs : [TransferLeg]
factoryAdmin : Party
controller settlement.executors
Against canton-contracts D1.daml:83-90 this omits registryCid and maxValidity, and the
controller differs: the implementation uses authorizedExecutors (a field on the attestation),
not settlement.executors. The shown controller matches canton-specs Cip112.daml:722 instead.
maxValidity is not incidental. It is what bounds an attester's self-chosen validity window
(D1.daml:111-112, sourced from Registry.daml:47-49), so omitting it from the illustrated
signature drops the mechanism that prevents an effectively permanent attestation.
Consequence
The D1 sections are the parts of these documents that a compliance-sensitive reader will study most
closely, and the linked permalinks resolve into canton-contracts while the described behaviour is
a blend of both implementations plus a key-based resolution that exists in neither. The net effect
is that the security property is stated accurately but attributed to the wrong mechanism.
Suggested resolution
- Regenerate the D1 snippet in
dex.md:797-815fromcanton-contractsat the pinned commit,
showing the choice-context transport (Base.daml:43-44,Registry.daml:388-397) and the pinned
requiredAttesterRegistryCidrather thanfetchByKey. - Replace the "resolves its
TrustedAttesterRegistryby key" sentences atdex.md:770-771,
dex.md:792-794, andconfidential-auction.md:562-564with the actual basis for the property,
which is the cid pinned on the rules contract plus theregistry.admin == factoryAdmincheck at
D1.daml:95. - In the three settle snippets (
dex.md:772-776,confidential-auction.md:565-569,
lending.md:599-603), either showextraArgscarrying the attestation under the documented
context key, or add a one-line note that the attestation travels in the choice context because
the interface argument record is fixed. - If the intent is that these snippets stay abstract, consider making the disclaimer at
dex.md:694explicit that identifiers and resolution mechanisms in snippets may not correspond
to any implementation, since at present the snippets read as near-verbatim excerpts.
Happy to be told any of this is deliberate simplification. Flagging it because the LF 2.1 point in
particular makes the illustrated pattern unbuildable rather than merely abbreviated.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the D1 sections in docs/reference-architectures/dex.md, confidential-auction.md, and lending.md, then compare them with the pinned canton-contracts files Base.daml, Registry.daml, and D1.daml. Check how the choice context, pinned registry CID, admin check, and maxValidity are represented. Done means the snippets and explanatory sentences match the cited implementation or clearly identify their abstraction.
Written by the indexing model from the issue text.
Assessment
- Domain
- documentation, security
- Issue type
- Documentation
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 52/100