Parse the registrar and the label into Holding

[gyorgybalazsi]

What this PR does

Old behavior

A Holding contract on the ledger carries eight fields. token::holding::Holding
read three of them — the amount, the owner and the ticker — and discarded the
other five. It also kept the contract id, which comes from the ledger event
rather than from the payload.

Two of the discarded fields matter. registrar names the party that administers
the instrument. label names the account the holding sits in.

A caller cannot tell two registrars' instruments apart without registrar. Any
registrar can issue a token called CBTC. On devnet one party already holds
legacy CBTCV0RC8 holdings beside real ones. A caller that matched on the ticker
alone selected holdings that the registry then rejected, with
400 Given holdings are invalid.

Nothing could filter holdings by account without label, because the account a
holding sits in is exactly what that field records.

Separately, InstrumentId lived in common::transfer, where it had sat since
the initial commit because Transfer was the first type that needed it.

New behavior

Holding now keeps five of the eight fields instead of three. It reads
registrar and label as well.

The parser puts registrar into instrument_id. That field used to be a plain
string holding the ticker, such as "CBTC". It is now a pair: the ticker, and
the party that issued it. This library calls that party the admin, and the
Daml template calls the same party the registrar. They are one party under
two names. That is why the parser reads one name and writes the other.

The parser puts label into a new field called account_label.

So a caller can now answer two questions it could not answer before. Who issued
this holding? Which account does it sit in?

A caller that compares the ticker alone no longer compiles. That is deliberate.
The ticker does not identify an instrument on its own, so the old comparison
accepted another registrar's CBTC without saying so.

Separately, InstrumentId now lives in common::instrument rather than
common::transfer.

Verification

• cargo test --workspace — 192 tests ran, zero failures. holding.rs had no
  tests at all and now carries 19, and every one of those 19 runs. They cover
  the five parsed fields, all eight error messages, and the lock check.
  The suite spans 18 binaries, but nine of those hold no test, and another 39
  tests across the workspace carry #[ignore].

• cargo clippy --workspace --all-targets -- -D warnings — clean, on a fresh
  --target-dir. That run took 22 seconds, so it is a real lint pass rather than
  a cache hit.

• cargo fmt --check — clean.

• The ignored integration suite — 27 of 27 against devnet, in 655 seconds:
  registry 2 and token 25, run with
  cargo test -p registry -p token -- --ignored --test-threads=1 integration_.
  These tests read real Holding payloads off a live ledger and parse them
  through the widened struct, so the JSON field names are measured rather than
  inferred.

  The run covers commit 8be135e. Every commit since changes doc comments and
  nothing else, so none of them can move a ledger result: git diff 8be135e..HEAD shows only comment lines.

  Two of the 27 carry most of the weight: client::integration_check_and_consolidate,
  in both its V1 and V2 forms. They are the only tests that exercise the parser
  on payloads the ledger actually sends. consolidate_utxos is the only production path that builds a
  Holding, and it takes its contracts from an interface-filtered ACS query. If
  that query returned no createArgument, every parse would fail. The test
  guarantees at least two UTXOs and then asserts consolidated, utxos_after == 1 and a preserved balance, so reaching those assertions proves real payloads
  parsed through the new required reads.

What this does not cover. This repository has no CI. There is no .github
directory, so no automated gate runs on this PR and gh pr checks reports
nothing. Every number above comes from a local run on one machine. Issue #35
tracks that, and I have added this PR to it as a worked example.

The 19 unit tests on their own do not prove the JSON field names are right,
because they read a hand-written fixture. The devnet run above is what closes
that, and the fixture also matches the Daml template, which I read from source.

The devnet run proves parsing, not discrimination. Every holding it met was
CBTC under one admin, so it never met two registrars issuing the same ticker,
and it never met a non-empty label. It shows the parser reads what the ledger
actually sends. That two different registrars now produce two different
InstrumentId values is shown by the unit tests alone — reads_the_id_out_of_the_instrument_object
and the_admin_comes_from_registrar_not_from_instrument_source are the two that
carry it.

The cbtc-lib devnet suite has not run against this change. It runs on
cbtc-lib PR 66, which repins to this release.

The change that enables it

• Holding.instrument_id changes type from String to
  common::instrument::InstrumentId, which carries an admin beside the ticker.
• Holding gains account_label: String, read from the payload's label.
• The parser reads the payload's top-level registrar for the admin.
• InstrumentId moves from common::transfer to common::instrument, and
  derives PartialEq, Eq and Hash.

Caveats

• This breaks every consumer that compares a ticker, and every consumer that
  imports InstrumentId. canton-lib releases it as 0.8.0 for that reason.
  cbtc-lib has five ticker-comparison sites plus two doc comments, and
  cbtc-lib PR 66 fixes them in the same repin.

• The migration has a wrong answer that compiles. Writing
  h.instrument_id.id == "CBTC" builds fine and reintroduces the exact bug this
  removes. Compare the whole InstrumentId. The changelog says so too.

• account_label holds a label, not an account, and that is a deliberate bet
  on the registry changing. A Token Standard Account is owner, provider
  and id. The utility registry populates only two: holdingV1Metadata builds
  every account with provider = None as a literal (Holding.daml:247,
  registry-holding 0.3.2). So owner plus this label determines the account
  today, and a single String is enough.

  Digital Asset intends to use the provider in a later iteration, and the
  utility implementation of Token Standard V2 is not final. When that lands, a
  label stops determining an account, and three construction sites in this
  library change: this field, TokenClient::account_for, and the batch CSV
  receiver. #51 tracks
  it, and explains why we cannot build the fix before DA ships theirs.

  This is the reason the field is not called account_id. A field named for the
  label it holds keeps its meaning when the account model grows; a field named
  account_id would have to change meaning.

  The field has already paid for itself, on devnet, the day it landed. A
  reviewer of cbtc-lib PR 66 found that its V2 integration run burned a
  labelled holding while check_balance read the unlabelled account, so the
  balance never moved and step 18 failed with "Balance did not decrease after
  withdrawal". list_holdings filters by instrument and not by account, and
  registry-holding does write labels, so this is reachable rather than
  theoretical. The fix selects unlabelled holdings for the burn — expressible
  only because this field exists. Filtering holdings by instrument alone can
  still pick a holding from an account you did not mean to touch.

• One unparseable holding will fail a whole call once a consumer filters on
  these fields. That behaviour lives in cbtc-lib's list_holdings, not in this
  diff, and the design records it as accepted there.

• Re-date the changelog at merge if it slips. The 0.8.0 heading reads
  2026-09-14, following this repo's convention of dating a release the day it
  ships.

Details

Three things the diff does not show

1. instrument_id keeps its name and changes its meaning. The type moves
   from String to InstrumentId. String held the ticker alone, and InstrumentId holds the
   admin and the ticker. No rename marks the change, so read every use of the
   field rather than scanning for renamed ones.
2. The test fixture renames instrument.admin to instrument.source. The
   Daml type InstrumentIdentifier declares source, id and scheme, and no
   admin. An earlier branch's fixture taught a field name that does not exist.
3. The parser reads payload.registrar and writes instrument_id.admin. Both
   names are correct, and they denote the same party. Holding.daml:57 maps them
   the same way when it builds the V1 interface view. The doc comment on
   InstrumentId.admin records the mapping, and it names the third spelling too:
   InstrumentIdentifier.source.

Why InstrumentId moved

It is not a transfer type. Nineteen files across token, common and registry
reach for it, and allocations, splits, consolidations and holdings are not
transfers. The Token Standard itself declares InstrumentId in
Splice.Api.Token.HoldingV1, so the old location had this repo's own doc comment
pointing at a Holding module while the file sat in a transfer one.

The move is import paths only. Fields and JSON encoding do not change. There is
no compatibility re-export, because this release already breaks Holding and a
lingering pub use would preserve the wrong mental model for another release.

Why account_label is not called account_id

It holds the template's label verbatim, and a label is not an account. A V2
account is owner, provider and id together. A caller who writes
h.account_label == account.id is visibly comparing a label to an id, and is
skipping the provider check that token::active_contracts::matches_account
performs.

Through registry-holding 0.3.2 the account carries no provider, so owner plus
the label determines it. holdingV1Metadata builds the account with
provider = None as a literal (Holding.daml:247), so the payload's provider is
never read into it. The ensure clause at Holding.daml:50 tests the same
property through isRegistryAccount and isNone account.provider
(TokenApiUtilsV2.daml:116), which cannot fail while the value it tests is a
constant.

That is a fact about today's registry, not about the standard. The Token
Standard's Account has a provider, and Digital Asset has said it intends to use
it in a later iteration. When it does, a label stops determining an account on
its own. Naming the field for what it holds means that day changes what callers
build rather than what the field means — which is the main reason not to call it
account_id.

Do not read the payload's own top-level provider field as the account's
provider. That party is an observer (Holding.daml:42). The parser ignores it
deliberately, and a reader who wired it into the account would build the wrong
one.

Why label parses as required

Every registry-holding version from 0.0.1 to 0.3.2 declares label as a
required Text, 0.3.2 being the current one, shipped in utility 0.14.4. I read
that from Digital Asset's source. All 519,386 active mainnet holdings carried the
field on 11 Sep 2026.

A required read separates two cases that an optional read merges. An absent field
means the contract is not a registry Holding at all. An empty string means a
real holding in an unlabelled account. The tests cover both.

Why the parser reads registrar rather than instrument.source

Both fields hold the same party. The template's ensure clause calls
hasValidInstrumentIdentifier, which forces the two to agree, so a check that
they match could never fail. The parser reads registrar, because that is the
field the interface view reads.

The parser also ignores the payload's top-level provider. holdingV1Metadata
builds the account with provider = None, so the payload's provider is not the
account's provider.

Why there is no holding::v2 module

This type parses createArgument, which is the concrete template payload. That
payload is identical under Token Standard V1 and V2. The V1 and V2 split in the
other eight modules exists because registry routes and choice arguments differ,
and holding parsing touches neither.

What the move did to the rest of the diff

consolidate.rs is the only non-test consumer of Holding, and its logic needs
no change, because it reads .amount and .contract_id only. It does appear in
the diff, with 11 insertions and 11 deletions, and every one of them is an
InstrumentId import path. The same is true of most of the 24 changed files.

Notes

• The workspace version moves to 0.8.0, which rewrites eight version lines in
  Cargo.lock. Eight of the nine workspace members inherit it; crates/ledgrpc
  is canton-proto-rs and pins its own.
• PR Test the Holding parser #49 is closed unmerged. This PR carries its 14 parser tests, with the
  fixture corrected as described above, beside 5 new ones.

---

Design: cip-112 docs/specs/2026-09-11-canton-lib-holding-struct-design.md ·
Consumer: cbtc-lib PR 66 · CI gap: #35 ·
Closes the parser half of DLC-link/cbtc-lib#74

🤖 Generated with Claude Code

[Copilot]

🔵 Needs a closer look

The breaking API change affects downstream consumers and warrants final human review.

Pull request overview

Updates Holding parsing to retain registrar-based instrument identity and account labels, with a breaking 0.8.0 API change.

Changes:

• Parses registrar and label.
• Adds InstrumentId equality support and tests.
• Documents and versions the breaking change.

File summaries

File	Summary
crates/token/src/holding.rs	Extends holding parsing and adds tests.
crates/common/src/transfer.rs	Adds InstrumentId equality and documentation.
CHANGELOG.md	Documents the breaking API change.
Cargo.toml	Bumps the workspace version.
Cargo.lock	Updates workspace package versions.

Review details

• Files reviewed: 4/5 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Resolve the advertised account_id/implemented account_label mismatch and correct the active-contracts verification claim.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

crates/token/src/active_contracts.rs:86

• The verification text says active_contracts::integration_get_by_party and its _v2 twin exercise Holding::from_active_contract, but these tests only call active_contracts::get and compare contract IDs. That path filters interface-view JSON in wanted and never invokes the concrete-payload parser; the parser is exercised elsewhere by consolidation tests, not by the two tests named here. Please correct this verification claim or add the promised direct parse assertion to these tests.

    instrument: &common::instrument::InstrumentId,

• Files reviewed: 23/24 changed files
• Comments generated: 1
• Review effort level: Lite

[hubagaspar91]

[pragmatic] Review — 3 comments, all on doc comments. No code findings.

The parser change is sound, and this pass found no code defect. All three comments below concern the new doc comments.

One general request. Several new comments cite exact code lines in Digital Asset's Daml sources (Holding.daml:57, Holding.daml:247, TokenApiUtilsV2.daml:116, HoldingV1.daml:16-19). A line number drifts with any upstream edit, and nothing detects the drift. Please cite a symbol name plus a pinned package version instead, in every new comment.

[hubagaspar91]

[pragmatic] This comment mixes two kinds of content. The caller contract belongs here: the field holds the template's label verbatim, a label is not an account, and a comparison against Account.id skips the provider check in active_contracts::matches_account. The version-pinned proof does not belong here. It cites registry-holding 0.3.2 by Daml line numbers, and nothing in this repo detects when those citations go stale. The drift has already started: this comment cites Holding.daml:43 for the observer fact, and the PR body cites Holding.daml:42 for the same fact. Please trim the comment to the caller contract plus one pointer to issue #51. Please move the Daml proof and the roadmap paragraphs into issue #51 and the design doc. Where a citation must stay, name the symbol and the package version, for example holdingV1Metadata in registry-holding 0.3.2.

[hubagaspar91]

[pragmatic] Please drop the census sentence. The count was stale one day after the measurement, and a future reader cannot re-check it. The PR body already carries it, where it supports the one-time merge decision. Keep the first sentence about the eight declared fields. Also, this comment says the versions run 0.0.1 to 0.3.1, and the parser comment in from_active_contract says 0.0.1 to 0.3.2. Please align the two ranges.

[hubagaspar91]

[pragmatic] Holding.daml:57 here, and HoldingV1.daml:16-19 below, cite another organization's repo by line number. Any upstream edit invalidates them silently. Please cite the symbol and pin the version instead, for example: holdingV1Metadata in registry-holding 0.3.2 maps registrar to admin. The explanation that registrar, admin and source are one party under three names is worth keeping.