docs(token): document safe->C2C migration via CMA#532
Conversation
Expand the NonFungibleToken migration-plan header to explain how the safe/_unsafe deprecation is carried out on-chain through Midnight's Contract Maintenance Authority (CMA). Key rationale: the recipient type is the final Either<Bytes<32>, ContractAddress> from day one because the CMA can rotate a circuit's verifier key but cannot migrate ledger state. That makes the future migration a pure VK rotation (VerifierKeyRemove + VerifierKeyInsert for transfer, VerifierKeyRemove for _unsafeTransfer) with the contract address and all ledger state preserved. Widening the types later would require a ledger-layout rewrite the CMA cannot do. Refs: #118
Replace the generic FN/_unsafeFN deprecation note with the concrete guarded circuits and explain how the migration runs on-chain through Midnight's Contract Maintenance Authority (CMA). Rationale: the recipient type is the final Either<Bytes<32>, ContractAddress> from day one because the CMA can rotate a circuit's verifier key but cannot migrate ledger state, so the migration is a pure VK rotation with address and state preserved. Refs: #118
Add a structured migration-plan section to the MultiToken header (it previously only noted "revise logic once C2C is available"), covering the guarded circuits, their _unsafe variants, and how the migration is carried out on-chain through Midnight's Contract Maintenance Authority (CMA). Rationale: the recipient type is the final Either<Bytes<32>, ContractAddress> from day one because the CMA can rotate a circuit's verifier key but cannot migrate ledger state, so the migration is a pure VK rotation with address and state preserved. Refs: #118
Add a structured migration-plan section to the Ownable header (it previously only noted the unsafe circuits are "planned to become deprecated"), covering transferOwnership / _transferOwnership, their _unsafe variants, and how the migration is carried out on-chain through Midnight's Contract Maintenance Authority (CMA). Rationale: the owner type is the final Either<Bytes<32>, ContractAddress> from day one because the CMA can rotate a circuit's verifier key but cannot migrate ledger state, so the migration is a pure VK rotation with address and state preserved. Refs: #118
Add a structured migration-plan section to the AccessControl header (it previously only noted the unsafe circuit is "planned to become deprecated"), covering grantRole / _grantRole, _unsafeGrantRole, and how the migration is carried out on-chain through Midnight's Contract Maintenance Authority (CMA). Rationale: the account type is the final Either<Bytes<32>, ContractAddress> from day one because the CMA can rotate a circuit's verifier key but cannot migrate ledger state, so the migration is a pure VK rotation with address and state preserved. Refs: #118
WalkthroughThis PR updates contract documentation across five modules in the OpenZeppelin Compact Contracts library. Each module's header documentation is extended with detailed migration plans explaining how to evolve from "safe" single-recipient flows to contract-to-contract (C2C) capable flows, describing verifier-key rotation strategies and ledger-state preservation. No executable logic or public interface changes are made. ChangesC2C Migration Planning Documentation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
contracts/src/token/NonFungibleToken.compact (1)
34-54: 💤 Low valueConsider clarifying which circuits the migration plan covers.
The migration plan mentions "
transfer" and "_unsafeTransfer" specifically (lines 46, 48, 50), but the module has multiple guarded circuits (transfer,transferFrom,_transfer,_mint) and their corresponding_unsafevariants (_unsafeTransferFrom,_unsafeTransfer,_unsafeMint).For consistency with
FungibleToken.compact(lines 43-54), which uses placeholder notation like "FN" to represent the family of guarded circuits and explicitly lists all affected circuits, consider either:
- Listing all affected circuits explicitly: "ship
transfer,transferFrom,_transfer, and_mintwith theisContractguard removed and drop their_unsafecounterparts"- Using clearer language to indicate the migration applies to all guarded transfer/mint circuits, not just the public
transfercircuitThis improves clarity for implementers planning upgrades.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contracts/src/token/NonFungibleToken.compact` around lines 34 - 54, Update the migration plan wording to clarify which circuits are covered by the C2C migration: either explicitly list all guarded circuits and their unsafe counterparts (e.g., transfer, transferFrom, _transfer, _mint and their _unsafe variants _unsafeTransfer, _unsafeTransferFrom, _unsafeMint) where you currently mention only `transfer` and `_unsafeTransfer`, or replace the singular names with a clear family-level phrase such as "all guarded transfer/mint circuits (transfer, transferFrom, _transfer, _mint) and their corresponding _unsafe* variants" so readers know the plan applies to every guarded transfer/mint circuit including their _unsafe versions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@contracts/src/token/NonFungibleToken.compact`:
- Around line 34-54: Update the migration plan wording to clarify which circuits
are covered by the C2C migration: either explicitly list all guarded circuits
and their unsafe counterparts (e.g., transfer, transferFrom, _transfer, _mint
and their _unsafe variants _unsafeTransfer, _unsafeTransferFrom, _unsafeMint)
where you currently mention only `transfer` and `_unsafeTransfer`, or replace
the singular names with a clear family-level phrase such as "all guarded
transfer/mint circuits (transfer, transferFrom, _transfer, _mint) and their
corresponding _unsafe* variants" so readers know the plan applies to every
guarded transfer/mint circuit including their _unsafe versions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fcca365c-0f5f-4d82-9be5-4527e9293758
📒 Files selected for processing (5)
contracts/src/access/AccessControl.compactcontracts/src/access/Ownable.compactcontracts/src/token/FungibleToken.compactcontracts/src/token/MultiToken.compactcontracts/src/token/NonFungibleToken.compact
<!-- Thank you for your interest in contributing to OpenZeppelin! -->
Types of changes
What types of changes does your code introduce to OpenZeppelin Midnight Contracts?
Put an `` in the boxes that apply
Fixes #118
PR Checklist
Further comments
If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...
Summary by CodeRabbit