Repository navigation
midnight: bump docs to v0.3.0 - #258
andrew-fleming wants to merge 10 commits into
Conversation
✅ Deploy Preview for openzeppelin-docs-v2 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Landing page still says "highly experimental, use at your own risk"
Where: content/contracts-compact/index.mdx:14-17
Description: 🔴 blocking. The warn callout says the repo "contains highly experimental code … Use at your own risk." The v0.3.0 README scopes that warning to -alpha / -rc builds and lists two audits for 0.3.0. This PR already retitled the "Experimental features" sections on the Ownable and AccessControl pages, so this one was missed.
Why it matters: The first thing a reader sees contradicts the release the page documents and undercuts the audited-stable positioning.
Fix: Replace with the README wording: pre-release versions are experimental and unaudited; for production use a stable release on the ledger it targets, with a link to the audits table. Unblocks on that edit.
added by claude (dev3-code-review + midnight-basic-review skills)
| Contract-initiated mints and refunds do not create coin ciphertexts, so a recipient's wallet cannot discover | ||
| those coins by scanning the chain. The `ShieldedCoinInfo` a circuit returns is the only copy of the coin: | ||
| the DApp must capture it and deliver it to the recipient (or persist it, when the contract is the recipient). | ||
| Dropping it strands the value irrecoverably. |
There was a problem hiding this comment.
Page contradicts itself on coin ciphertexts
Where: content/contracts-compact/native-shielded-token.mdx:86 vs :304-306
Description: 🔴 blocking. This callout says contract-initiated mints and refunds "do not create coin ciphertexts, so a recipient's wallet cannot discover those coins." Lines 304-306 say midnight-js encrypts an output to its recipient when it can resolve the key, including via additionalCoinEncPublicKeyMappings. The midnight-js option doc describes it as resolving encryption keys "for coins created during circuit execution", so the softer statement is the right one.
Why it matters: A DApp author reading the callout builds an out-of-band delivery path they may not need, or skips the mapping that would have made the coin discoverable.
Fix: Qualify the callout: no ciphertext unless the DApp supplies the recipient's encryption key through that mapping. The same absolute sentence sits in the NativeShieldedTokenCore module header and is worth fixing there too. Unblocks on the callout edit.
added by claude (dev3-code-review + midnight-basic-review skills)
| * ***Every point is constrained into the prime-order subgroup.*** The runtime assigns each `JubjubPoint` reaching a curve operation | ||
| by cofactor clearing at the constraint level, so an off-curve, low-order, or mixed-order point makes the circuit unsatisfiable. | ||
| This holds for points a caller passes as arguments or witnesses (a recipient `pk`, the message point of `encryptPoint`), | ||
| which is why the modules perform no in-circuit membership check and why the homomorphic algebra is sound. | ||
| The identity point is a valid subgroup member, so it is NOT caught by this guarantee; that is why the modules assert against it explicitly. |
There was a problem hiding this comment.
Subgroup guarantee stated as a fact; the source calls it an unaudited assumption
Where: content/contracts-compact/crypto.mdx:154-158
Description: 🔴 blocking. "Every point is constrained into the prime-order subgroup" is presented as a runtime property and as the reason the modules skip in-circuit membership checks. The ElGamal.compact header labels the same claim "NOT a team-confirmed or audited guarantee", source-verified against midnight-circuits 6.0.0, pinned by crypto/test/CurveRuntimeInvariants.test.ts, and to be re-verified on every runtime bump.
Why it matters: Consumers inherit the "no membership check needed" decision without knowing it rests on unaudited runtime behaviour that a runtime bump can change.
Fix: Add one sentence after the bullet: verified against the pinned runtime and regression-tested, not audited; re-verify before using the modules on a newer runtime. Unblocks on that sentence.
added by claude (dev3-code-review + midnight-basic-review skills)
|
|
||
| {/* links */} | ||
| [FungibleToken]: ./fungible-token.mdx | ||
| [Module/Contract Pattern]: ./extensibility.mdx#the_module_contract_pattern |
There was a problem hiding this comment.
Dead anchor copied into three new pages
Where: confidential-fungible-token.mdx:7, multisig.mdx:6, native-shielded-token.mdx:6
Description: ⚪ nitpick. All three define [Module/Contract Pattern] as ./extensibility.mdx#the_module_contract_pattern. The rendered heading id is the-modulecontract-pattern, which non-fungible-token.mdx already uses. The same anchor pre-exists in fungible-token.mdx:8 and multi-token.mdx:8.
How to see it: curl -s https://deploy-preview-258--openzeppelin-docs-v2.netlify.app/contracts-compact/extensibility | grep -o 'id="the[^"]*"' prints the-modulecontract-pattern.
Fix: Point all five at ./extensibility#the-modulecontract-pattern. Worth checking why pnpm run check passed, since lint:links validates fragments.
added by claude (dev3-code-review + midnight-basic-review skills)
| import "./node_modules/@openzeppelin/compact-contracts/utils/Utils" prefix Utils_; | ||
|
|
||
| const accountId = Utils_computeAccountId(secretKey); |
There was a problem hiding this comment.
Identity-model snippets are neither Compact nor TypeScript
Where: fungible-token.mdx:78-80, multi-token.mdx:96-98, non-fungible-token.mdx:87-89, access-control.mdx:75-78, ownable.mdx:67-70
Description: ⚪ nitpick. Each block is fenced as typescript, opens with a Compact import … prefix Utils_; and follows with a TS-style const accountId = Utils_computeAccountId(secretKey);. Neither toolchain accepts it as written.
Why it matters: It is the snippet a reader copies to derive the identifier they pass to the constructor.
Fix: Link to the two working paths the Utils guide shows (re-export as a pure circuit and call pureCircuits.computeAccountId, or the compact-runtime persistentHash snippet), or show the Compact re-export directly.
added by claude (dev3-code-review + midnight-basic-review skills)
Documentation Pull Request
Summary
Syncs the Contracts for Compact docs to the v0.3.0 release
v0.3.0tagType of Change
Checklist
pnpm run buildpnpm run checkAdditional Notes