Review: the four RI architecture reports, round 3 (comments only, do not merge) - #83
Review: the four RI architecture reports, round 3 (comments only, do not merge)#83pepebndc wants to merge 1 commit into
Conversation
## What this does Adds a GitHub Actions workflow that aligns the review-only PR #83 with `main` on demand. The workflow runs only on manual trigger (`workflow_dispatch`, the **Run workflow** button in the Actions tab). When triggered, it: 1. Rebuilds `review-base/ri-reports-r3` from the `main` tip, with the four RI reports removed. 2. Rebuilds `review/ri-reports-r3` on top of it, with the reports re-added from `main`. 3. Force-pushes both branches, so PR #83 shows the current `main` content of the reports as an added-file diff. The workflow exits without changes when `review/ri-reports-r3` no longer exists, so it becomes a no-op once the review round closes and the branches are deleted. ## Why manual instead of automatic Reviewers left feedback on an outdated version of the reports because the review PR had fallen behind `main`, with no easy way to update it. This gives us a one-click update. An automatic push-triggered sync was considered and rejected: a review can take several days, and an automatic sync would change the content under the reviewer's feet mid-review ([Slack thread](https://openzeppelin.slack.com/archives/C0AENAASAUR/p1786636513166079)). With the manual trigger we sync deliberately, between review passes. ## Trade-off When a synced report line differs from the previous snapshot, inline comments on that line become outdated in PR #83. They stay visible in the conversation tab. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
6399c09 to
fa835c3
Compare
8c89ea0 to
ad35ab1
Compare
fa835c3 to
e9b941a
Compare
ad35ab1 to
3ed5b48
Compare
b117f67 to
9512a8b
Compare
e016867 to
6736262
Compare
9512a8b to
f4a6cce
Compare
| `minCollateralRatio`, and a position that later falls under | ||
| `liquidationRatio` becomes liquidatable. The surplus is the buffer that | ||
| keeps the debt fully covered through collateral price movement. | ||
| - **Permissioned.** Every participant acts under a verified identity: |
There was a problem hiding this comment.
| - **Permissioned.** Every participant acts under a verified identity: | |
| - **Permissioned.** Every party acts under a verified identity: |
|
|
||
| State changes by archive-and-recreate rather than in-place mutation, with every signatory co-authorizing the transition (Daml's propose-and-accept pattern). That is why the design resolves the `Vault`, `PriceOracle`, `PauseState`, and the trusted-attester and trusted-issuer registries by **contract key** (reintroduced in [Canton 3.5.1+](https://github.com/digital-asset/canton/releases/tag/v3.5.1)): a key is the identity that survives each recreate. Keys are not unique - the platform accepts two contracts sharing one - so uniqueness stays an application obligation. The vault creation should perform checks against duplicate positions. | ||
|
|
||
| Keys are the design target, not what runs today. The experiment code sits on the workspace's pinned SDK baseline and is keyless, so each choice takes a caller-supplied registry contract id and asserts it shares the factory's admin. By-key resolution lands with the 3.5.1+ SDK migration. |
There was a problem hiding this comment.
I don't understand why contract keys aren't supported. Contract key support is on Dev/Test/MainNet. See https://docs.canton.network/shared/version-compatibility-dashboard.
There was a problem hiding this comment.
@hrischuk-da To clarify: going forward, all our implementations will use contract keys where applicable, since they are supported and the recommended practice. We will also retrofit the existing code, i.e. the current PauseState template does not use keys yet. That is what the paragraph intends to convey: our production implementations will make full use of contract keys; the code we have experimented with and referenced here as examples simply has not adopted them yet, since they were not supported when all these experiments were created.
There was a problem hiding this comment.
@hrischuk-da changed to Contract keys are supported on the network today and our production implementation will use them throughout. The experiment code referenced here predates the workspace's move to the 3.5.1+ SDK and is still keyless, so choices may taks a caller-supplied registry contract id and assert it shares the factory's admin. The experiments and the existing components (such as PauseState) will migrate to by-key resolution with the SDK upgrade..
| edges are runtime interactions, dashed edges are standing governance or | ||
| trust relationships, and keyed contracts are marked with their key. | ||
|
|
||
| The first diagram shows the actors and the lending application's own |
There was a problem hiding this comment.
Should the Custodian and Registry be shown in the first diagram?
There was a problem hiding this comment.
@hrischuk-da It would be clearer indeed, I added them.
|
|
||
| - **Vault Admin / Stablecoin Issuer (`VAULT_ADMIN`)** - underwrites the **stablecoin (debt) token**: operates the `VaultFactory`, configures `VaultParams`, the `TrustedIssuerRegistry` (accepted KYC issuers), and the `TrustedAttesterRegistry` (accepted compliance attesters), and issues the grants that authorize custodian seizure sweeps. The admin can mint the stablecoin, never the collateral, and only inside the vault choices ([section 3](#3-target-design)). The protocol gives the vault admin no path to issuing unbacked stablecoin. | ||
| - **Borrower (`BORROWER`)** - the entity locking collateral and drawing debt. Only the borrower can lock their own holdings into an allocation. To interact with the protocol, the borrower must hold a valid `KycClaim`, verified at vault creation and fetched live by each value-moving vault choice. Visibility is limited to the borrower's own vaults and the public configuration contracts. | ||
| - **Liquidator (`LIQUIDATOR`)** - a role granted via `openzeppelin-access-control-v1`. Each granted liquidator is placed in the observer set of the vaults it polices, so it can monitor the `PriceOracle` and vault solvency off-ledger from its own projection; authorized to liquidate only after the margin-call grace period has elapsed on a flagged, still-unhealthy vault, and only proportionally to the stablecoin it repays. |
There was a problem hiding this comment.
Please adjust Liquidator because it is incredibly hard to parse.
There was a problem hiding this comment.
@hrischuk-da thank you for spotting this, adjusted.
| - **Borrower (`BORROWER`)** - the entity locking collateral and drawing debt. Only the borrower can lock their own holdings into an allocation. To interact with the protocol, the borrower must hold a valid `KycClaim`, verified at vault creation and fetched live by each value-moving vault choice. Visibility is limited to the borrower's own vaults and the public configuration contracts. | ||
| - **Liquidator (`LIQUIDATOR`)** - a role granted via `openzeppelin-access-control-v1`. Each granted liquidator is placed in the observer set of the vaults it polices, so it can monitor the `PriceOracle` and vault solvency off-ledger from its own projection; authorized to liquidate only after the margin-call grace period has elapsed on a flagged, still-unhealthy vault, and only proportionally to the stablecoin it repays. | ||
| - **Oracle Operator(s) (`ORACLE_PROVIDER`)** - the implementation-defined party set that updates the `PriceOracle`, bound by the interface requirements ([section 4](#43-component-price-oracle-interface)): no single party, not even the vault admin, should be able to move or stall the published price. | ||
| - **Insurance Fund (`INSURANCE_FUND`)** - the party that collects protocol revenue: it mints the interest revenue against the vaults' `feeReceivable` records; the accumulated fund is the first absorber of recognized bad debt. |
There was a problem hiding this comment.
Is the insurance fund collecting a portion or all of the revenue?
There was a problem hiding this comment.
@hrischuk-da in this design, it's all of the revenue. Note it does not include the liquidation bonus, which is a reward for the liquidator.
| the vault's choices both signatures arrive automatically, inherited from the | ||
| `Vault`'s own signatories. | ||
|
|
||
| For roles that need to submit routinely (the insurance fund collecting fees), we envision keeping the confirmation threshold at 1, with each such role's powers bounded on-ledger. |
There was a problem hiding this comment.
The confirmation threshold is about the trust architecture. Are you alright with a malicious agent that captures the insurance fund's key being able to steal all of the funds?
I don't think efficiency is the key driver so you may want to adjust the rationale.
There was a problem hiding this comment.
@hrischuk-da good point, the insurance funds should have a higher confirmation threshold.
|
|
||
| Whatever update mechanism the **oracle operators** run ([section 4](#43-component-price-oracle-interface)), an all-of-M quorum should be deliberately avoided: a single offline member, or one whose participant node has unvetted the protocol DAR, would stall every price update until the staleness guard freezes the protocol. | ||
|
|
||
| The **pause authority** is multi-hosted so the brake is always reachable, but its confirmation threshold stays at 1: an emergency stop must be instant, and a quorum would slow it down. The price of that choice is a griefing window: a malicious pauser can freeze in-flight settlements until their deadlines lapse. This griefing is capped by the authorizer's right to reclaim the allocated funds after the expiration deadline. The pause brings the additional risk of not being solvency-neutral: it freezes liquidation while collateral keeps repricing, an exposure tracked in [section 7](#7-open-design-questions). |
There was a problem hiding this comment.
I don't know why but when I see "griefing window" I read "grieving widow". :)
There was a problem hiding this comment.
@hrischuk-da fair point :) to be fair, griefing usually comes together with grieving, by someone at least.
| Taking each element of the codeblock in turn: | ||
|
|
||
| - **Proportional seizure.** `debtRepaid` is the amount the liquidator's own exercise burns in the same transaction, never the vault's full accrued debt, so a liquidator can never take more collateral than their payment (plus bonus) buys. | ||
| - **Restorable vault (`collateralRatio > 1 + liquidationBonus`).** Repaying `x` leaves debt `accruedDebt - x` and collateral value `collateralAmount · price - x · (1 + liquidationBonus)`; `restoreAmount` is the `x` that sets their ratio to exactly `minCollateralRatio`. In this regime every repaid unit improves the ratio, so a payment below the cap partially cures, a payment at the cap fully cures, and nothing beyond it can be taken (no overshoot). The target is `minCollateralRatio`, not `liquidationRatio`, so a cured vault does not restart on the liquidation boundary. |
There was a problem hiding this comment.
This line is unreadable. Please adjust.
There was a problem hiding this comment.
@hrischuk-da noted, I adjusted it to be more clear.
|
|
||
| ### Data and State Flow | ||
|
|
||
| The diagrams below show the four vault flows: **A** collateral deposit, **B** borrow, **C** repay and close, **D** margin call and liquidation. Atomic settlement appears only in the collateral deposit; repayment and liquidation payments burn in place, and everything the protocol releases (minted stablecoin, returned or seized collateral) moves by direct transfer under the vault's joint authority in the same transaction. In each, the `Compliance gate` node stands for the compliance-attestation check and the live KYC-claim fetch ([section 3](#compliance-is-re-checked-on-every-operation)), and keyed contracts are marked with their key. |
There was a problem hiding this comment.
It isn't clear what business operations are atomic. This sentence structure is very strange.
There was a problem hiding this comment.
@hrischuk-da all of the flows are designed to be atomic. The mistake I made here is to say "atomic settlement", where I actually meant "the settlement rail"... the paragraph means to say that only the collateral deposit requires the settlement rail (i.e. create an allocation and settle), the rest can use plain transfers from CIP112.
| Factory -.->|"first deposit:<br/>create the Vault"| Vault | ||
| ``` | ||
|
|
||
| **B. Borrow (mint coupled to debt).** The borrower asks the vault for stablecoin; the vault checks compliance, reads the current price, and assesses whether the locked collateral is worth enough to cover the new debt. If so, it mints the stablecoin to the borrower and records the higher debt, all in one transaction. This flow does not need atomic settlement: the stablecoin's issuer and the borrower both already stand behind the vault, so the vault choice itself carries every signature the mint needs. |
There was a problem hiding this comment.
This flow does not need atomic settlement: the stablecoin's issuer and the borrower both already stand behind the vault, so the vault choice itself carries every signature the mint needs.
But isn't it atomic based on the prior sentence? If so, why have this sentence?
There was a problem hiding this comment.
@hrischuk-da It is atomic, I meant it does not need the settlement rail. I made adjustments for here and the above comment.
| Coin -->|"to borrower"| Borrower | ||
| ``` | ||
|
|
||
| **C. Repay and close.** The borrower pays down debt: the vault checks compliance, burns the whole payment out of the borrower's wallet (the borrower and the stablecoin issuer both already stand behind the vault, so no settlement is needed), records the lower debt, and adds the interest portion to the insurance fund's fee receivable. The insurance fund mints its accumulated fees on its own schedule, backed by that record. On close, the vault hands the remaining collateral back to the borrower in the same transaction. |
There was a problem hiding this comment.
I would think that two operations are needed for this:
- Pay down some debt.
- Close the debt fully, including calculated fees and interest.
These seem separate to me because I would think 2. has the step of the borrower asking for what repayment is needed to fully close the account.
Am I missing something?
There was a problem hiding this comment.
@hrischuk-da we have two operations indeed:
- repayment, which decreases debt.
- close, which only works if all the debt is repaid. Closing the position will return all the collateral inside. I changed the text to clarify these two steps.
I would think 2. has the step of the borrower asking for what repayment is needed to fully close the account.
As we envision it, the borrower does not need to ask how much repayment is needed - he has visibility into the vault, and the backend app/wallet can compute this for him, and initiate the payment.
| 3. **Repay and close.** `Vault_BurnStablecoin` burns the payment via [`TokenRules_Burn`](https://github.com/OpenZeppelin/canton-contracts/blob/7696749737885e25cd88422847105f890f03b00d/experiments/token/tokenCIP112-v1/daml/OpenZeppelin/TokenCIP112V1/Registry.daml#L170), reduces `debtAmount`, and increments `feeReceivable`. The insurance fund collects via `Vault_CollectFees`; `Vault_Close` winds the position down, parking any uncollected receivable in an admin-signed contract for later collection. | ||
| 4. **Margin call and liquidation.** `Vault_FlagForLiquidation` opens the grace window; once it elapses on a still-unhealthy vault, `Vault_Liquidate` names `debtRepaid` (capped by the health-restore formula), consumes a compliance attestation checking the liquidator, burns the payment, releases the proportional collateral, and recreates the residual `Vault`. | ||
|
|
||
| The sequence diagram below traces the margin-call and liquidation flow end to end: |
There was a problem hiding this comment.
This sequence diagram is a work of art.
There was a problem hiding this comment.
@hrischuk-da thank you for the kind words! Let me know if you see any other places that could benefit from a diagram.
| Assumptions and important notes: | ||
|
|
||
| - Between a deposit's allocate and its settle the borrower's collateral is | ||
| locked; the lock is time-bounded and the borrower always has a unilateral |
There was a problem hiding this comment.
Why is the lock time bound? Does that mean the collateral asset is returned to the borrower before the borrower has repaid?
There was a problem hiding this comment.
@hrischuk-da this lock refers to the lock on the collateral allocation that goes through the settlement rail. Once settled, the only way for the borrower to recuperate the collateral is to pay any outstanding debt and close. I changed the text so this is clear.
Please note all the changes are on a separate PR that we didn't merge yet, so you will not see them in your ongoing review, yet. Thanks Curtis!
|
|
||
| The debt paid on repay, close, or liquidation is `principal + accrued interest`; all flows burn the full payment via the registry's `TokenRules_Burn` and record the interest portion in the vault's `feeReceivable`; the insurance fund realises that revenue later by exercising `Vault_CollectFees`, which mints up to the recorded receivable. Those fees are protocol revenue, the on-ledger analogue of interest paid to the lender, and the accumulated fees are the first absorber of any liquidation shortfall. | ||
|
|
||
| Interest creates a structural liquidity gap: the protocol mints only principal, yet borrowers owe principal plus interest, so aggregate debt always exceeds circulating supply by the accrued interest. The stablecoin that pays interest must come from other borrowers' minted principal or from the insurance fund's re-minted fees re-entering circulation. |
There was a problem hiding this comment.
How is the interest paid for with the stablecoin? How does that stablecoin enter the liquidity of this app? I'm confused by this paragraph.
|
|
||
| Collateral is only ever **transferred**, never minted or burned: a deposit settles the borrower's collateral holding into the vault custody account through CIP-0112 settlement, and a withdrawal releases it back by direct transfer under the vault's joint authority. The vault admin therefore needs no issuing authority over the collateral instrument, only over the stablecoin. Institution-supplied, third-party-issued collateral (a custodian bank's deposit token, a tokenized treasury) is therefore compatible. | ||
|
|
||
| To facilitate value transfers, the design assumes that the vault admin **is** the stablecoin's registry admin, which is what lets vault choices mint and burn it directly; a third-party debt token would push borrow and repay onto that token's own registry rails. The deposit rides the allocation rail rather than a receiver-accepted transfer instruction for two reasons: a committed allocation is the standard pre-signed rail a third-party registry offers an executor, and in-flight deposits are the surface the custodian lock-and-sweep control is defined over. |
There was a problem hiding this comment.
Am I right in understanding this assumption is like Circle or Tether being the vault admin? If true, how hard is it to insert a level of indirection where the value admin works with an external stablecoin?
I don't think this assumption holds and can't be part of the design.
This would be a good extension point where you can provide a mock integration implementation but specify the interface(s).
|
|
||
| ### Implementing Smart Contract Upgrades | ||
|
|
||
| For a smart contract upgrade, an existing choice's arguments must never be mutated to require a new field. Extensions are managed via appended `Optional` fields, new serializable types, and **new choices**. An interface definition cannot change once deployed; only an interface instance (its implementation in a template) can, so new capabilities arrive as new templates and choices, never by retroactively re-instancing the deployed `Vault`. |
There was a problem hiding this comment.
A new interface can be added as well.
I don't think this section represents SCU well. The AI is off. Take a look at https://docs.canton.network/appdev/deep-dives/smart-contract-upgrade#what-is-smart-contract-upgrade-scu-
| `requireJointAccount`, `requireFactoryAdmin`, and `requireDepositLeg` (exactly one leg delivers a positive | ||
| `initialCollateral` of the collateral instrument into custody) are | ||
| illustrative helpers in the style of section 4.2. | ||
|
|
There was a problem hiding this comment.
I am going to assume that the Daml implements the design and that there are sufficient DamlScript tests to validate it. I will try to get an engineer to review it.
|
|
||
| ### 4.2 Component: Vault State, Margin Call, and Liquidation | ||
|
|
||
| The `Vault` holds one borrower's CDP state. The state-update logic lives **here**, as consuming choices controlled by the relevant role, which archive this `Vault` and recreate the successor with updated figures. The `Vault` carries a contract key `(vaultAdmin, borrower, vaultId)`, so consumers reference a position by its stable identity rather than by a cid that changes on every operation. Liquidation is **pause-gated** and **margin-called**: it resolves the `PauseState` by key and requires an elapsed grace period on a flagged, still-unhealthy vault. No settlement is involved: the liquidator's payment burns under the choice's own authority, and the custody account is jointly authorized by the vault's own signatories, so the choice releases the seized collateral itself. |
There was a problem hiding this comment.
Is every vault totally independent? For example, there is no super party that sees all vaults?
|
|
||
| ### 4.3 Component: Price Oracle Interface | ||
|
|
||
| The `PriceOracle` is the one contract whose compromise would let an attacker manipulate the price, potentially manufacturing liquidations or borrowing unbacked amounts. This report specifies the interface the vaults consume and the requirements any update mechanism must satisfy; the mechanism itself is an implementation decision ([section 7](#7-open-design-questions)). The oracle carries a contract key `(admin, collateralInstrumentId, stablecoinInstrumentId)`, so vaults resolve the current price by key across its archive-and-recreate update cycle. The requirements on the update mechanism: |
There was a problem hiding this comment.
This is an interesting key because the key can return multiple contracts since uniqueness is not guaranteed. This is not a bad thing but the code needs to be aware of this.
| - Liquidation seizes collateral exactly proportional to the stablecoin the liquidator actually pays: `debtRepaid` is burned in the same transaction that releases the collateral. A liquidator can never take more than their payment (plus bonus) buys. | ||
| - **Margin call before seizure**: | ||
| - Liquidation requires a prior flag plus an elapsed `gracePeriod`, giving the borrower a deterministic cure window instead of a submission-timing race. | ||
| - **Fee integrity**: |
There was a problem hiding this comment.
Wouldn't the borrower need to pay the interest too?
|
|
||
| ### 5.2 Validation Expectations | ||
|
|
||
| The experiments and library packages provide bounded evidence for individual |
There was a problem hiding this comment.
I don't understand what this section is saying.
f4a6cce to
86d82b6
Compare
6736262 to
825ca37
Compare
|
|
||
| ## 2. Architecture Overview | ||
|
|
||
| The auction has three principal business participants: the bidder, issuer, and |
There was a problem hiding this comment.
| The auction has three principal business participants: the bidder, issuer, and | |
| The auction has three principal business personas: the bidder, issuer, and |
|
|
||
| Token Standard V2 defines the common interface for locking and settling assets. | ||
| Institutional services provide eligibility and optional settlement approval. | ||
| Governance roles define application authority. Canton participant nodes host |
There was a problem hiding this comment.
| Governance roles define application authority. Canton participant nodes host | |
| Governance roles define application authority. Canton Network validator nodes host |
86d82b6 to
ad6d9b2
Compare
825ca37 to
d171da8
Compare
3 participants
What this is
A review-only PR to collect a fresh round of external comments on the four OpenZeppelin Canton Reference Implementation (RI) architecture reports, as they currently stand on
main. The base branch is a temporary copy ofmainwith the reports removed, so the full text of each report appears here as an added-file diff and supports inline comments.Do not merge. The reports already live on
mainunderdocs/reference-architectures/. When this review round is done the PR will be closed and the temp branches deleted.The previous round was collected in #40.
Files under review
docs/reference-architectures/dex.md(Privacy-Preserving DEX)docs/reference-architectures/lending.md(Lending Protocol)docs/reference-architectures/cross-chain-stablecoin.md(Cross-Chain Stablecoin)docs/reference-architectures/confidential-auction.md(Confidential Auction Launchpad)How to review
Open the Files changed tab and leave inline comments directly on the report lines. General remarks are welcome as regular PR comments.