Back to Details

code-review

Compare original and translation side by side

🇺🇸

Original

English
🇨🇳

Translation

Chinese

Code Review

代码评审

Overview

概述

This skill provides guidance for reviewing code in the Cartridge Controller monorepo, which implements a gaming-specific smart contract wallet ecosystem for StarkNet.
本技能为Cartridge Controller monorepo中的代码评审提供指导，该monorepo为StarkNet实现了游戏专用的智能合约钱包生态系统。

Review Process

评审流程

1. Understand the Change Scope

1. 明确变更范围

bash
undefined
bash
undefined

For a PR

针对PR的操作

gh pr view <PR_NUMBER> gh pr diff <PR_NUMBER>
gh pr view <PR_NUMBER> gh pr diff <PR_NUMBER>

For local changes

针对本地变更的操作

git diff origin/main...HEAD git log origin/main..HEAD --oneline
undefined
git diff origin/main...HEAD git log origin/main..HEAD --oneline
undefined

2. Review Checklist

2. 评审检查清单

Code Quality

代码质量

  • Code follows existing patterns in the codebase
  • No unnecessary complexity or over-engineering
  • Clear variable and function names
  • Appropriate comments for complex logic
  • No dead code or console.log statements
  • 代码符合代码库中的现有模式
  • 无不必要的复杂度或过度设计
  • 变量与函数名称清晰易懂
  • 复杂逻辑配有恰当的注释
  • 无死代码或console.log语句

TypeScript

TypeScript

  • Proper type annotations (avoid 
    any
    )
  • Interfaces/types defined for complex objects
  • No TypeScript errors (
    pnpm build
    passes)
  • 正确添加类型注解（避免使用
    any
    ）
  • 为复杂对象定义接口/类型
  • 无TypeScript错误（
    pnpm build
    可正常通过）

Security (Critical for this codebase)

安全性（针对本代码库至关重要）

  • No secrets or credentials in code
  • Iframe communication properly validated
  • Origin checks for postMessage handlers
  • Session tokens handled securely
  • No XSS vulnerabilities in UI components
  • WebAuthn/Passkey operations follow best practices
  • 代码中未包含密钥或凭证
  • Iframe通信已进行正确验证
  • 为postMessage处理器添加源检查
  • Session令牌的处理符合安全规范
  • UI组件中无XSS漏洞
  • WebAuthn/Passkey操作遵循最佳实践

StarkNet/Blockchain Specific

StarkNet/区块链相关检查

  • Transaction calls properly structured
  • Gas estimation handled appropriately
  • Error handling for chain interactions
  • Address validation where needed
  • 交易调用结构正确
  • 燃气估算处理恰当
  • 针对链上交互的错误处理完善
  • 必要处已添加地址验证

Testing

测试检查

  • New functionality has tests
  • Edge cases covered
  • Tests are meaningful (not just for coverage)
  • Storybook stories updated for UI changes
  • 新功能已配备测试用例
  • 已覆盖边缘场景
  • 测试用例具备实际意义（而非仅为了覆盖率）
  • UI变更已同步更新Storybook示例

Package-Specific Considerations

各包专属检查事项

controller/ (SDK):
  • Public API changes are intentional and documented
  • Backward compatibility considered
  • Iframe communication protocols maintained
keychain/ (Secure iframe):
  • UI components follow design system
  • Sensitive operations properly isolated
  • State management is clean
connector/ (Integration layer):
  • Compatible with starknet-react patterns
  • Minimal dependencies
controller/ (SDK):
  • 公开API的变更为有意为之且已完成文档记录
  • 已考虑向后兼容性
  • 维持Iframe通信协议不变
keychain/ (安全Iframe):
  • UI组件遵循设计系统规范
  • 敏感操作已进行恰当隔离
  • 状态管理清晰合理
connector/ (集成层):
  • 与starknet-react模式兼容
  • 依赖项最少

3. Run Automated Checks

3. 执行自动化检查

bash
undefined
bash
undefined

Lint and format check

代码规范与格式检查

pnpm lint:check
pnpm lint:check

Type checking via build

通过构建进行类型检查

pnpm build
pnpm build

Unit tests

单元测试

pnpm test
pnpm test

Visual regression (if UI changes)

视觉回归测试（若涉及UI变更）

pnpm test:storybook
undefined
pnpm test:storybook
undefined

4. Provide Feedback

4. 提供反馈

Structure your review as:
markdown
undefined
请按以下结构组织评审意见：
markdown
undefined

Summary

总结

Brief overall assessment of the changes.
对变更的简要整体评估。

Positive Aspects

亮点

  • What's done well
  • 做得好的地方

Required Changes

必须修改项

  • Critical issues that must be fixed
  • 必须修复的关键问题

Suggestions

建议

  • Nice-to-have improvements
  • 锦上添花的改进建议

Questions

疑问

  • Clarifications needed
undefined
  • 需要澄清的问题
undefined

Review Severity Levels

评审严重程度等级

  • Blocking: Must be fixed before merge (security issues, bugs, breaking changes)
  • Important: Should be fixed, but can be follow-up PR
  • Suggestion: Optional improvements
  • Nitpick: Style preferences, can be ignored
  • 阻塞性：合并前必须修复（安全问题、Bug、破坏性变更）
  • 重要：应该修复，但可在后续PR中处理
  • 建议：可选的改进项
  • 细节优化：风格偏好，可忽略

Common Issues to Watch For

需重点关注的常见问题

In React Components

在React组件中

  • Missing dependency arrays in useEffect/useMemo/useCallback
  • State updates in loops without proper batching
  • Memory leaks from uncleared subscriptions
  • useEffect/useMemo/useCallback中缺少依赖数组
  • 循环中的状态更新未进行正确批处理
  • 未清理的订阅导致内存泄漏

In Async Code

在异步代码中

  • Missing error handling in try/catch
  • Unhandled promise rejections
  • Race conditions in concurrent operations
  • try/catch中缺少错误处理
  • 未处理的Promise拒绝
  • 并发操作中的竞态条件

In Type Definitions

在类型定义中

  • Overly permissive types (
    any
    , 
    unknown
    without narrowing)
  • Missing null checks for optional fields
  • Incorrect generic constraints
  • 过于宽松的类型（
    any
    、未进行收窄的
    unknown
    ）
  • 可选字段缺少空值检查
  • 不正确的泛型约束

Example Review Comment

评审意见示例

markdown
**[Blocking]** Security concern in `packages/keychain/src/hooks/connection.ts:45`

The origin validation is missing for this postMessage handler. This could allow malicious sites to send messages to the iframe.

Suggested fix:
```typescript
if (event.origin !== expectedOrigin) {
  return;
}
undefined
markdown
**[阻塞性]** 安全问题：`packages/keychain/src/hooks/connection.ts:45`

该postMessage处理器缺少源验证，可能允许恶意网站向iframe发送消息。

建议修复方案：
```typescript
if (event.origin !== expectedOrigin) {
  return;
}
undefined