OpenZeppelin / OpenZeppelin/canton-specs

Reference architecture D1 snippets do not match either CIP-112 implementation (fetchByKey registry resolution, attestationCid as a direct choice field)

Open
#82 3 comments 0 reactions 0 assignees View on GitHub

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 TrustedAttesterRegistry by 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 ContractId on the rules contract,
    Registry.daml:42 (requiredAttesterRegistryCid : Optional (ContractId TrustedAttesterRegistry)),
    passed to verification at Registry.daml:392-397, and resolved with a plain fetch at
    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 at D1.daml:95.
  • canton-specs: registryCid is a caller-supplied argument to
    ComplianceAttestation_Verify (Cip112.daml:720), resolved with fetch at :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):

SimpleTokenRules and TransferPreapproval lack 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

  1. Regenerate the D1 snippet in dex.md:797-815 from canton-contracts at the pinned commit,
    showing the choice-context transport (Base.daml:43-44, Registry.daml:388-397) and the pinned
    requiredAttesterRegistryCid rather than fetchByKey.
  2. Replace the "resolves its TrustedAttesterRegistry by key" sentences at dex.md:770-771,
    dex.md:792-794, and confidential-auction.md:562-564 with the actual basis for the property,
    which is the cid pinned on the rules contract plus the registry.admin == factoryAdmin check at
    D1.daml:95.
  3. In the three settle snippets (dex.md:772-776, confidential-auction.md:565-569,
    lending.md:599-603), either show extraArgs carrying 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.
  4. If the intent is that these snippets stay abstract, consider making the disclaimer at
    dex.md:694 explicit 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.