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.

Review Process

1. Understand the Change Scope

For a PR

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

For local changes

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

2. Review Checklist

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

TypeScript

• Proper type annotations (avoid any )

• Interfaces/types defined for complex objects

• No TypeScript errors (pnpm build passes)

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

StarkNet/Blockchain Specific

• 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

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

3. Run Automated Checks

Lint and format check

pnpm lint:check

Type checking via build

pnpm build

Unit tests

pnpm test

Visual regression (if UI changes)

pnpm test:storybook

4. Provide Feedback

Structure your review as:

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

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

Common Issues to Watch For

In React Components

• Missing dependency arrays in useEffect/useMemo/useCallback

• State updates in loops without proper batching

• Memory leaks from uncleared subscriptions

In Async Code

• Missing error handling in try/catch

• Unhandled promise rejections

• Race conditions in concurrent operations

In Type Definitions

• Overly permissive types (any , unknown without narrowing)

• Missing null checks for optional fields

• Incorrect generic constraints

Example Review Comment

[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:

if (event.origin !== expectedOrigin) {
  return;
}