Repository navigation
Conversation
## What this does Changes the manual sync workflow so that it maintains one review-only PR per RI instead of the single PR #83 with four reports. The DEX report is out of scope because its review is complete. | RI | Review PR | Branches | | --- | --- | --- | | Lending | #205 | `review-base/ri-lending`, `review/ri-lending` | | Cross-chain stablecoin | #206 | `review-base/ri-cross-chain-stablecoin`, `review/ri-cross-chain-stablecoin` | | Confidential auction | #207 | `review-base/ri-confidential-auction`, `review/ri-confidential-auction` | The workflow runs a matrix job for each RI. For each RI, it: 1. Rebuilds `review-base/ri-<ri>` from the `main` tip, with that RI report removed. 2. Rebuilds `review/ri-<ri>` on top of it, with the report re-added from `main`. 3. Force-pushes both branches. A new `ri` input (`all`, `lending`, `cross-chain-stablecoin`, `confidential-auction`) selects which PRs to sync. The default is `all`. A matrix job exits without changes when its `review/ri-<ri>` branch does not exist. ## Why External reviewers find it difficult to follow four interleaved discussions in one PR ([Slack thread](https://openzeppelin.slack.com/archives/C0AMGKVKTH7/p1790867786955579)). ## Verification - The YAML parses. - The three review PRs were created with the same branch logic, run locally against the current `main`. ## Caveats After this merges, PR #83 no longer syncs. Its comments stay available for reference.
bc67d14 to
001aa8e
Compare
912ab4a to
f718800
Compare
f718800 to
2a8f023
Compare
001aa8e to
1fb6321
Compare
meiersi-da
left a comment
There was a problem hiding this comment.
Thanks. This reads much better already. There is one change that should probably be done: streamline the allocation management as per the comments.
Also if possible, ensure that the auction operator has a well-defined business model, so that bidders are less prone to being front-run by the operator.
Otherwise, it seems that just polish remains.
| not guarantee admission or fair ordering of requests. | ||
|
|
||
| The **clear** is one atomic transaction. It validates the complete result, | ||
| cancels the supply and winners' payment locks, creates allocations for the exact |
There was a problem hiding this comment.
Why does this need to create allocations?
| The registries use [Token Standard V2](https://github.com/canton-foundation/cips/blob/6f37c896a5a76ec3bc1aa67bc045623ae5df41e5/cip-0112/cip-0112.md) | ||
| **allocations** to authorize movements and reserve holdings. The issuer reserves | ||
| the supply before opening. Each bidder reserves its maximum payment before | ||
| acceptance. These **committed allocations** restrict account-authorized |
There was a problem hiding this comment.
| acceptance. These **committed allocations** restrict account-authorized | |
| acceptance. These **committed allocations** restrict funds |
| arrival time of an off-ledger request or a bid's synchronizer record time. | ||
| Retries retain the numbers already assigned. | ||
|
|
||
| Clearing includes every accepted bid. Authenticated credential expiry or |
There was a problem hiding this comment.
Note that implementing this requires expired/revoked credentials to leave tombstone contracts so that their revocation can be verified on-ledger.
| All auction allocations disable iterated settlement with | ||
| `nextIterationFunding = None`. Executors therefore cannot add transfer sides | ||
| through iterated settlement. |
There was a problem hiding this comment.
Why do you do that? isn't that counter to the need of specifying the exact transfers once they are known?
| that transfer because it lacks receiver-side authorization. The clear cancels | ||
| this reservation and uses `SaleAuthority` to authorize actual winner deliveries. | ||
| Bid payment locks use the same form for the bidder's payment account. |
There was a problem hiding this comment.
This solution was used for V1 allocations, as there was no better option. However with V2 allocations you can make them iterated and committed, and then the clear can just compute all the transfers that must be settled and register them as
FinalizedAllocation with
allocationCid
extraTransferLegSides = <clearing legs>
nextIterationFunding = None
the last line ensures that the unused funds will be returned to the authorizer of the allocation.
There was a problem hiding this comment.
If you want to make the workflows work with tokens that use account-custody then I'd suggest that you also have bidders and issuers provide the allocations for receiving their funds in case of a successful bid. These allocations are comitted and iterated, but do not reserve any funding amounts.
The benefit of this is then, that after they are created, a multi-step approval flow can happen on the registry-side, and once that completes and creates the expected allocations, they can be accepted in the round.
On settlement no allocations need to be created. Just compute the transfer legs and call SettlementFactory_SettleBatch once for the inventory registry and once for the payment registry.
| #### Validate Allocation Successors | ||
|
|
||
| Opening, admission, clear, and recovery check the current allocation against | ||
| the recorded root. The backend discovers successors from authenticated registry | ||
| events. On-ledger checks verify an active allocation from an approved registry | ||
| implementation and validate its lineage. An initial allocation's ID must equal | ||
| the root. A successor's `originalAllocationCid` must equal it, and the registry | ||
| must preserve a single active continuation and prevent forged lineage. | ||
|
|
||
| The allocation must retain the authorized account, instrument, amount, sides, | ||
| commitment, executor set, settlement reference, and deadline, with | ||
| `nextIterationFunding = None`. Registry-specific evidence supplies any | ||
| required status absent from the standard view. Resolve stale IDs through the | ||
| registry's authenticated events before evaluating funding. A consumed lock | ||
| without a valid funded successor prevents the complete clear, even for a bid | ||
| that would otherwise receive zero fill. Registry restrictions never authorize | ||
| the operator to remove an accepted bid. |
There was a problem hiding this comment.
You probably don't need that. You already have the round's contract-id as the unique id for the settlement.
|
|
||
| If an application attester is configured, `BatchApproval` must bind the full | ||
| settlement reference and exact payment and delivery legs, with an unexpired | ||
| validity period. For several settlement references, approval must cover the |
There was a problem hiding this comment.
why would there be multiple settlement references?
| and calls no factory with an empty leg list. Configured application approval | ||
| still covers the settlement reference and empty movement set. | ||
|
|
||
| ### 3.5 Release Locked Assets |
There was a problem hiding this comment.
I suspect there may be a simpler option: introduce an AbortedRound state, and offer a choice for the ao there to call V2.Allocation_Cancel on any allocation naming av as the executor and referring to that rounds identifying contract-id in its settlement-id. This allows for early cleanup. Expiry based cleanup runs anyways after the settlement deadline.
Use the same combination of early expiry via AbortedRound and guaranteed expiry for the bids and other round related state (including AbortedRound).
| health, and outstanding recovery. It separates auction assets from traffic | ||
| funding and application rewards. | ||
|
|
||
| ### 6.1 Traffic and Application Rewards |
There was a problem hiding this comment.
This opens up a question: how does the operator of the auction venue make money? I'd suggest to add a fee model from the get-go, as otherwise they will find another way: the obvious one being exploiting the information advantage. Not good.
| active network rules. Governance defines sharing, and operations must be funded | ||
| without assuming rewards cover costs. | ||
|
|
||
| ### 6.2 Production Readiness |
There was a problem hiding this comment.
Not sure what to make of this section.... reads very "AI".
What this is
A review-only PR to collect external comments on the OpenZeppelin Canton Confidential Auction Launchpad Reference Implementation (RI) architecture report, as it currently stands on
main. The base branch is a temporary copy ofmainwithout this report, so the full text of the report appears here as an added-file diff and supports inline comments.Do not merge. The report already lives on
mainatdocs/reference-architectures/confidential-auction.md. When the review is done, the PR will be closed and the temporary branches deleted.Each RI has its own review PR, so that the discussion threads for each RI stay separate. Earlier comments on this report are in #83.
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.
Sync with main
The Sync review PRs with main workflow rebuilds this PR from the current
mainon manual trigger. Selectconfidential-auctionorallin theriinput.