|
| 1 | +--- |
| 2 | +name: review-sui-contracts |
| 3 | +description: "Review Sui Move code that integrates OpenZeppelin Contracts for Sui against the library's own patterns and conventions. Use this when a developer wants their integration checked before shipping. Triggers: \"review my Sui Move code\", \"am I using OpenZeppelin correctly\", \"check this integration\", \"is this idiomatic\", \"review before audit\". Checks correct use of OZ primitives, deviations from examples/doc-comments, upheld invariants, test coverage, and code-quality/style. This is an AI code review, not a formal security audit." |
| 4 | +license: AGPL-3.0-only |
| 5 | +metadata: |
| 6 | + author: OpenZeppelin |
| 7 | +--- |
| 8 | + |
| 9 | +# Review Sui Contracts |
| 10 | + |
| 11 | +AI-assisted review of a developer's Sui Move code against **OpenZeppelin Contracts for Sui**: does it use the library's on-chain primitives correctly, and where does it deviate from the library's `examples/`, doc-comments, and documented conventions? |
| 12 | + |
| 13 | +> **This is an AI code review, not a formal security audit.** It supports the audit process by flagging misuse and deviations early; it does not replace an independent audit, and a clean review is not an assurance of safety. |
| 14 | +
|
| 15 | +For **building** an integration use `develop-secure-contracts`; for **scaffolding** a project use `setup-sui-contracts`. This skill reviews code that already exists. |
| 16 | + |
| 17 | +## Establish the reference before reviewing |
| 18 | + |
| 19 | +You cannot flag "incorrect use" or "deviation" without the correct pattern in hand — so discover it from the library's own metadata, never from memory, using the discovery chain the **`develop-secure-contracts`** skill defines. Read the primitive's **API and behavior at the revision the integrator builds against** — resolve each OZ dependency's pinned rev from `Move.lock` and use that source / those doc-comments, not `main`. Read the library-wide docs — `llms.txt`, the catalogs, `ARCHITECTURE.md`, `STYLEGUIDE.md` — from the repo's current `main` instead: they evolve continuously and often do not exist at an older pinned rev. For each OZ primitive the code uses, load its authoritative pattern from: |
| 20 | + |
| 21 | +- the package's **`examples/` if it has them, otherwise the module-header doc-comment's idiomatic-usage block** — the canonical recipe (some packages ship only the latter); |
| 22 | +- the modules' **doc-comments** and generated API reference (reached via the catalog's `Docs` link); |
| 23 | +- **`ARCHITECTURE.md`** (capability model, object ownership, initialization) and **`STYLEGUIDE.md`** (conventions). |
| 24 | + |
| 25 | +Review the code against those, not against assumptions. |
| 26 | + |
| 27 | +## What to review |
| 28 | + |
| 29 | +- **Correct use of OZ primitives.** Does the code thread the capability/witness the component defines (rather than hand-rolling `sender` checks or re-implementing what the library already provides), call functions with the right arguments, and handle the documented abort conditions? |
| 30 | +- **Who chooses the type argument — and is it the right one.** For each OZ primitive parameterized by a witness or generic/`phantom` type, check *who* supplies it and whether it is the correct authority. A caller-chosen type is an authority the caller controls, so the trusted party must fix it, not the caller. And a developer-fixed type must be the *right* one for the action — a privileged operation gated by a lower-privilege role's `Auth` (or any weaker witness than the guarantee needs) is a privilege-escalation bug. |
| 31 | +- **Deviations from the reference pattern.** Compare the flow against the closest `examples/` recipe (or the doc-comment's idiomatic-usage block) and flag divergences that weaken the guarantees it preserves — e.g. an object shared vs. embedded against its intended ownership model, a required setup step skipped, a returned value or receipt ignored, or an invariant the doc-comment states being bypassed. |
| 32 | +- **Copied library source.** Flag any OZ module source copied into the project instead of imported via MVR — copies miss security updates. |
| 33 | +- **Invariants upheld.** Check that the code preserves the invariants the OZ components document (in their doc-comments and `ARCHITECTURE.md`) — e.g. an object that must stay shared, a value that must remain bounded, a capability that must not be exposed. Flag any path that can break one. |
| 34 | +- **Test coverage of the integration.** Check that the tests exercise the abort and branch paths the OZ usage introduces (role-denied, expired, over-limit, wrong-type, and similar), not just the happy path — untested failure paths are where integration bugs hide. |
| 35 | +- **Known issue classes (heuristic).** Check the code for the classes of issue that recur in OZ-on-Sui / Move code — capability handling, rounding, object ownership, initialization — and raise suspicion where the code touches them. (The library's published audits, under `audits/`, inform these classes; the reports themselves are PDFs, not a findings feed.) A suspicion-raising self-check, not a reproduction of the audits or a security audit. |
| 36 | + |
| 37 | +## Code quality — enforce the conventions |
| 38 | + |
| 39 | +Treat OpenZeppelin's Sui conventions as a first-class review dimension: report violations as **findings, not optional suggestions**. The rules and the review procedure are the single source of truth — apply them, don't restate them: |
| 40 | + |
| 41 | +- **[`STYLEGUIDE.md`](https://raw.githubusercontent.com/OpenZeppelin/contracts-sui/main/STYLEGUIDE.md)** — the codified rules (naming, section ordering, imports, idiomatic Move 2024, testing, docs, lint suppression). |
| 42 | +- **[`.claude/commands/code-quality.md`](https://raw.githubusercontent.com/OpenZeppelin/contracts-sui/main/.claude/commands/code-quality.md)** — the canonical procedure that checks `.move` files against `STYLEGUIDE.md`; run it against the code under review. |
| 43 | +- **Lint is not optional.** Build and test with the project's `--build-env` and `--lint` (see `setup-sui-contracts` for the exact invocation); per the styleguide, warnings must be **fixed**, never suppressed with `#[allow(...)]`. `--lint` enforces the compiler-checkable subset independently, so if the `STYLEGUIDE.md` fetch is momentarily unreachable, retry rather than skipping conventions. |
| 44 | + |
| 45 | +Report each violation against the `STYLEGUIDE.md` section it breaks. |
| 46 | + |
| 47 | +## Verify before reporting |
| 48 | + |
| 49 | +The checks above optimize for catching issues, so before writing anything, try to *disprove* each finding: quote the exact line and confirm the problem is really there; confirm it is a genuine deviation and not an intentional, documented choice; and confirm you can state a concrete one-line fix. Drop whatever does not survive all three, and keep the severity bar high — a short review a developer trusts beats a long one they skim. |
| 50 | + |
| 51 | +## Output |
| 52 | + |
| 53 | +Produce one review, findings ordered by severity. Each finding: `file:line` — what's wrong (one clause) → the fix (one clause) — and the OZ `example` or doc-comment it deviates from, so the developer can compare directly. A short preamble is useful — the scope/boundary call and the build/test/lint status — and on a clean integration a single "verified and cleared" line is fine; keep the body to findings, not a recap. |
| 54 | + |
| 55 | +If the project builds, run its checks first (`sui move build` / `test`, per `setup-sui-contracts`) so the review reflects compiling code, and report build/test failures separately from the pattern findings. |
| 56 | + |
| 57 | +## Boundaries |
| 58 | + |
| 59 | +- Reviews the **integrator's** code — how it uses the library — not the OpenZeppelin library internals themselves. |
| 60 | +- **Not a security audit** — no exhaustive threat modeling or completeness guarantee; recommend a formal audit for production code. |
| 61 | +- Complements `develop-secure-contracts` (integration) and `setup-sui-contracts` (scaffolding); it neither builds nor scaffolds. |
0 commit comments