Skip to content

Add solana-program-review skill for on-chain program reviews - #122

Open
nikw9944 wants to merge 2 commits into
mainfrom
nikw9944/onchain-program-review-skill
Open

Add solana-program-review skill for on-chain program reviews#122
nikw9944 wants to merge 2 commits into
mainfrom
nikw9944/onchain-program-review-skill

Conversation

@nikw9944

@nikw9944 nikw9944 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

What

Adds a repo-local Claude Code skill at .claude/skills/solana-program-review/SKILL.md capturing this repo's review conventions for its Solana on-chain programs (passport and revenue-distribution). Claude Code auto-discovers it and applies it when reviewing on-chain program changes in this repo.

Where it comes from

Distilled from this repo's own PR review history — feedback classes (account & signer validation, PDA seeds+bump, CPI safety, checked arithmetic, rent/space/realloc, (de)serialization, testing, error handling, etc.), each with a concrete "what to look for in a diff" checklist and real verbatim review quotes linked to their PRs.

Scope

Scoped to this repo's Solana serialization model (bytemuck zero-copy + 8-byte precomputed discriminator, ZeroCopyAccount loaders). It deliberately excludes the DoubleZero Ledger conventions (borsh serde + 1-byte AccountType enum), which live in malbeclabs/doublezero and would be wrong here. Keeping the skill in-repo means a reviewer only ever sees this repo's conventions.

Part of a set: malbeclabs/doublezero and the other Solana program repo get their own repo-local skills.

Notes

  • Documentation/tooling only, no code change.

🤖 Generated with Claude Code

Adds a repo-local Claude Code skill capturing this repo's review conventions
for its Solana on-chain programs (passport and revenue-distribution), distilled from the repo's own
PR review history. Scoped to this repo's Solana serialization model (bytemuck
zero-copy + 8-byte precomputed discriminator); it deliberately excludes the
DoubleZero Ledger borsh + AccountType enum conventions used in a separate repo.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 7, 2026 02:32
@nikw9944
nikw9944 requested a review from karl-dz as a code owner July 7, 2026 02:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a repo-local Claude Code skill document to standardize how reviewers evaluate Solana on-chain program diffs in this repository, specifically aligned to this repo’s zero-copy/bytemuck + 8-byte discriminator account model and the historical review patterns for the passport and revenue-distribution programs.

Changes:

  • Introduces a new .claude skill (solana-program-review) describing repo-specific on-chain review checklists.
  • Captures concrete “what to look for in a diff” guidance across common Solana program risk areas (accounts/signers, PDAs, CPI, arithmetic, rent/space, testing, etc.).
  • Includes linked, verbatim historical review quotes to anchor conventions to prior PR feedback.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Add a 'How to apply - severity and voice' section and a Sibling-processor
  parity class; add heap-cap hazard, hardcoded-program-id owner check,
  off-chain RPC/tx hygiene, and test-realism guidance. Adapted to this repo's
  bytemuck zero-copy model.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nikw9944

nikw9944 commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Applied the same cross-cutting review feedback that landed on the sibling DZ Ledger skill (malbeclabs/doublezero#3991), adapted to this repo's bytemuck zero-copy model:

  • Severity & voice section (blocking vs nit vs follow-up; frame judgment calls as questions, give exact replacement code, prove serialization claims with a runnable #[test]).
  • Sibling-processor parity class — diff a new handler against its closest sibling for drift in account-loading, authority gating, status/journal handling, and counter discipline.
  • Heap-cap hazard — a diff-grown account collection needs its cap enforced at the mutating instruction (+ near-cap test), or an over-grown account bricks on the 32KB heap.
  • Hardcoded program id — an owner check's expected id must be an in-repo constant, never a caller-supplied arg/account.
  • Off-chain RPC/tx hygiene — no unwrap/expect on RPC data, blockhash inside send, get_multiple_accounts, pre-flight existence checks, no zero-value txs, CU limit from real costs.
  • Test realism — reach state via real instruction sequences (not set_*/byte-poke); flag feature-gated tests CI never runs.

Skipped the DZ-Ledger-specific items (the try_from-isn't-an-owner-check note, the arithmetic converse, and eyre/_info) since they don't apply to this repo's model. Reviewer-added checks are marked (reviewer-curated).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants