Cultura de Code Review
Na Solana, uma constraint has_one perdida pode custar milhões. Cultura de code review equilibra ensinar juniors padrões Sealevel com desbloquear entregas - comentários de alto sinal em segurança e semântica de conta, baixo ruído em formatação (rustfmt cuida disso).
Receita
Cartão de receita de referência rápida - pronto para copiar.
## Checklist de Review Solana PR
- [ ] Flags owner/signer/writable de conta corretas
- [ ] Seeds e bumps de PDA validados
- [ ] Program IDs de CPI hardcoded ou allowlisted
- [ ] Aritmética verificada (sem add sem check)
- [ ] Testes: happy + caminho atacante (LiteSVM)
- [ ] Impacto IDL/cliente notado na descrição do PRQuando usar:
- Integrar revisores novos em Anchor
- Pós-incidente "como isso passou no review?"
- Definir SLAs e normas da equipe
- Escalar carga de review em fusos horários
Exemplo Prático
// BOM comentário de review - específico, ensina, sugere correção
// "Atacante pode passar vault estrangeiro como `user_vault` - adicione:
// constraint = user_vault.owner == user.key()
// Veja: sealevel attacks - account substitution"
// BAIXO SINAL - evite
// "Renomeie esta variável para `amt`"Template de descrição de PR:
## Mudança
Adicionar instrução withdraw_v2
## Notas de segurança
Novo CPI para Jupiter; program ID allowlisted em constants
## Plano de teste
- anchor test
- Replay de exploit LiteSVM #42 da auditoriaO que isso demonstra:
- Enquadramento de comentário security-first com material de referência
- Autor do PR expõe CPI e evidência de teste antecipadamente
- Reduz ida-e-volta em constraints de conta
Aprofundamento
SLAs de Review
| Tipo de PR | Primeira resposta | Meta de aprovação |
|---|---|---|
| Hotfix | 1 hora | Mesmo dia |
| Lógica de programa | 1 dia útil | 2 dias |
| UI client | 1 dia útil | 2 dias |
Papéis
- Autor é dono de testes e descrição
- Revisor é dono do checklist de segurança
- Tech lead desempata arquitetura
Erros Comuns
- LGTM sem ler
#[account]- modo de falha #1. Correção: Revisão obrigatória da seção de contas. - Nits de estilo bloqueando merge - Desmoraliza; rustfmt/clippy no CI. Correção: prefixo
[nit]opcional. - Reviews de carimbo - Auditoria acha bugs óbvios. Correção: Rotacione revisores; auditorias spot.
- PRs enormes - Ninguém revisa a fundo. Correção: Diretriz 400 linhas; divida programa/cliente.
- Review como gatekeeping - Juniors param de perguntar. Correção: Pair no primeiro PR Sealevel.
Alternativas
| Alternativa | Use Quando | Não Use Quando |
|---|---|---|
| Mob review | Mudança CPI complexa | Fix rotineiro de client |
| Só auditoria | Programa imutável | Upgrades contínuos |
| Só lint automatizado | Typos | Semântica de conta |
FAQs
Quantos aprovadores para programa?
Dois para programs/ de produção; um para só docs.
Comentários de bot?
Clippy, checks Anchor, cargo audit no CI - humanos focam em semântica.
Revisar próprio código?
Nunca único aprovador no próprio PR para mudanças de programa.
Discordância?
Escale para tech lead; ADR se arquitetural.
Ensinar em comentários?
Link doc interno ou catálogo Sealevel; tom colaborativo.
Revisar mudanças de IDL?
Mesmo rigor que Rust - clientes dependem disso.
Async em fusos?
Vídeo só para RFC; PR async com SLA claro.
Métricas?
Tempo até primeira review, tempo de resolução de comentários, taxa de defeito pós-merge.
PRs de estagiário?
Revisor sênior pareado; comentários explicadores extras gentis.
Bypass de hotfix?
Review pós-merge em 24h; incident commander pode mergear com um sênior.
Relacionado
- Conduzindo Design Reviews - design pré-PR
- Mentoria e Leveling - cultivar revisores
- Catálogo de Ataques Sealevel - referência de review
Versões da stack: Esta página foi escrita para Agave 4.1.1, Solana CLI 3.0.10, Anchor 0.32.1, anchor-lang 0.32.1, Rust 1.91.1, @solana/kit 7.0.0, Surfpool 0.12.0 e LiteSVM 0.6.x.