Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions .claude/CLAUDE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Claude Code Instructions

## Prohibited

**NEVER** perform any git or graphite commands that modify repository state:
- No commits (add, commit, amend)
- No pushing or pulling
- No rebasing, merging, cherry-picking
- No branch creation/deletion

**Allowed:** Read-only commands for context retrieval and debugging (status, log, diff, show, blame, etc.)

## Planning

Keep the scope for the current task small enough to target ~100-200 lines of code (excluding comments, tests, line spacing, etc) unless explicitly stated otherwise. If a task would exceed this, suggest reducing the scope. User makes the final decision if it's not sensible to reduce the atomic changeset.

## Specs

Discovery service specs live in `.claude/specs/discovery-service/`.

The **source of truth** for privacy pool contract interface and semantics (encryption, hashing, etc.) is the Cairo code.

**When planning** discovery-core or discovery-service changes:
1. Always check against the Cairo code and service specs
2. If any divergence between existing code and spec:
- Prompt for changing the code (if the spec is right), OR
- Update the stale spec (if the code is right)

**MUST after verification** of discovery-core or discovery-service changes:
- Review relevant specs and update if the implementation changed any documented behavior
- Specs must stay in sync with verified code - no exceptions
104 changes: 0 additions & 104 deletions .claude/commands/code-guidelines.md

This file was deleted.

15 changes: 15 additions & 0 deletions .claude/rules/auto-update-guidelines.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
## Auto-update Code Guidelines

**MUST trigger when:**
- Receiving feedback from the user or PR reviewers about code quality, style, or best practices
- After verification of any task where a reusable lesson was learned

**Actions:**
1. Implement the requested fix (if applicable)
2. **Generalize the lesson** and update `.claude/rules/code-style.md`:
- Extract the underlying principle, not the specific fix
- Frame past fixups as illustrative examples, not as the rule itself
- Add under the appropriate section (Naming, Documentation, Edge Cases, Comments, Testing)
- If no section fits, create a new one or add to the WIP section

Lessons from code reviews and task completion must accumulate as reusable principles - no exceptions.
12 changes: 12 additions & 0 deletions .claude/rules/code-review.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
## Receiving Code Review

**MANDATORY:** When addressing GitHub PR comments or terminal review followups, invoke `/receiving-code-review` BEFORE making changes. No exceptions.

## Reviewing PRs

**MANDATORY:** When reviewing someone else's PR, invoke `/code-reviewer` BEFORE providing feedback.

**Review quality requirements:**
- For each change, provide concise context: "what" it does and "why" it's needed
- Analyze based on the codebase state at the commit prior to the PR, not just the diff
- Consider how the change fits into the existing architecture and patterns
156 changes: 156 additions & 0 deletions .claude/rules/code-style.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
# Code Guidelines

Apply these guidelines when writing or reviewing code in this codebase.

---

## Naming

### Every name must answer "what is this?"
- A variable name should be a noun or noun phrase that identifies what it holds — readable without surrounding context
- The core test: if you see the name on its own line, can you tell what it represents?

### No standalone adjectives
- Adjectives describe a quality but not the thing itself; always pair with the noun
- *Bad:* `pending`, `remaining`, `discovered`, `complete`, `invalid`
- *Good:* `pending_futures`, `remaining_channels`, `discovered_channels`, `complete_entries`, `invalid_keys`

### No single-letter variables
- Single letters carry no meaning outside of trivial closures passed to stdlib combinators
- *Bad:* `i`, `s`, `n`, `f` as local variables
- *Good:* `channel_offset`, `channel_slots`, `num_items`, `felt_value`
- *Exception:* Single-letter closure params where the type and context make the meaning obvious (e.g., `.map(|x| x + 1)`, `.filter(|c| c.is_complete())`)

### No contractions or abbreviations
- Spell out the full word; the saved keystrokes aren't worth the mental tax on readers
- *Bad:* `ch`, `sc`, `futs`, `addr` (when the domain doesn't use it), `ctx`, `val`
- *Good:* `channel`, `subchannel`, `pending_futures`, `address`, `context`, `value`
- *Exception:* Abbreviations established by the domain (e.g., `addr` in Cairo/StarkNet, `pk` for public key) — follow the domain's convention

### Disambiguate counts from collections
- When a name represents a count, make that explicit with a prefix (`n_`, `num_`) or suffix (`_count`)
- *Bad:* `channels` (is it a Vec or a count?), `total` (total of what?)
- *Good:* `num_channels`, `total_channels`, `channel_count`

### Consistent terminology
- Use the same term for the same concept across parameters, fields, and documentation
- If a term is renamed, rename it everywhere — partial migrations create confusion
- *Example:* If a struct field is `decryption_key`, use `decryption_key` everywhere, not `viewing_key` in some function parameters

---

## Documentation

### Match documentation to code identifiers
- Doc comments should use the exact parameter and field names from the code
- *Example:* If parameter is `addr`, doc should say `addr`, not `recipient_addr`

### Document semantic meaning, not just types
- Clarify behavioral details that the type signature doesn't convey
- *Example:* For indices: inclusive vs exclusive; for optionals: what `None` means; for ranges: whether bounds are included

---

## Edge Cases

### Treat all user-provided values as adversarial
- Any value deserialized from an HTTP request, cursor, query parameter, or other external input must be assumed hostile
- Trace user-controlled values through the full call graph and analyze whether they can cause DoS, OOM, panics, or other resource exhaustion
- Cap allocations derived from user input with hard limits (e.g., `const MAX_CAPACITY: usize = 1024`)
- *Example:* A cursor field `total_n_channels: u64` can be set to `u64::MAX` by an attacker; using it directly in `Vec::with_capacity` causes OOM before any budget check runs

### Never panic on data reachable from requests
- Code reachable from HTTP handlers, RPC calls, or any external input must never use `.unwrap()`, `.expect()`, or unchecked indexing on values derived from that input
- Reserve panics for compile-time invariants (hardcoded constants, static strings) where failure is a programmer bug, not a runtime possibility
- For fallible operations: return `Result` with a descriptive error variant, or use saturating/capping alternatives when the exact value doesn't matter
- *Example:* `check_slots_len(&values, 3)?` before indexing a returned Vec

### Prefer intuitive semantics over internal convenience
- Design APIs so callers don't need to know implementation details
- *Example:* A `start_index` should work as-is; avoid requiring `start_index + 1` adjustments

### Simplify when defaults add no value
- If `None` just means a default value, consider using a plain type instead
- *Example:* `start_index: u64` with default 0 is simpler than `Option<u64>` where `None` means 0

### Prefer defensive arithmetic
- Use operations that handle edge cases gracefully, even if guards exist
- *Example:* `saturating_sub` instead of subtraction that could underflow if guards are later refactored

---

## Brevity

### Inline expressions that save >2 lines
- Inline expressions where doing so saves more than 2 lines without making the resulting line excessively long
- Prefer `map_or(default, |x| x + 1)` over `map(|x| x + 1).unwrap_or(default)` - it's shorter and more idiomatic
- Use `.or()` to update optional values instead of `if let Some(x) = ... { field = Some(x) }`
- *Example:* `cursor.last_index = result.last_index.or(cursor.last_index);` instead of a 3-line `if let`

### Inline trivial expressions
- Avoid separate `let` bindings for trivial expressions like `.clone()` when used immediately
- Inline directly in function arguments if it doesn't hurt readability
- *Example:* `spawn(foo.clone(), bar.clone())` instead of `let foo = foo.clone(); let bar = bar.clone(); spawn(foo, bar)`

### Check for existing utilities before adding new ones
- Before writing a local helper function, search the codebase for existing shared utilities
- If similar code exists in multiple places, extract to a shared module (e.g., `test_fixtures.rs` for test helpers)
- *Example:* Test helpers like `get_channel_key()` belong in `test_fixtures.rs`, not duplicated in each test module

---

## Comments

### Explain WHY, not WHAT
- Add comments where the reasoning isn't obvious from context
- Focus on decisions, constraints, and non-obvious requirements

### No visual separator comments
- Don't use decorative comment lines to delineate sections (e.g., `// -----------`, `// =========`, `// ***`)
- If a file needs visual structure, that's a signal to split into separate modules or use doc comments on items
- Rely on blank lines and module organization for readability, not ASCII art

### Document implicit structures
- When code relies on conventions or layouts, make them explicit
- *Example:* Storage layouts, protocol-specific ordering, cryptographic choices

---

## Testing

### Never mask test failures
- Failing tests indicate real problems; hiding them hides bugs
- Fix the root cause instead of using `#[ignore]`, `.skip()`, or similar
- *Example:* If a test fails due to stale fixture data, regenerate the fixture - don't ignore the test

### Keep integration test setup minimal
- Inline setup in each test rather than building complex helper structs
- Query dynamic values (addresses, ports) at runtime instead of hardcoding
- *Example:* Instead of `DevnetWithIndexer` helper struct, inline devnet spawn and address queries directly in each test

### Cover edge cases systematically
- Test boundary conditions, empty inputs, and failure modes
- *Example:* For pagination, test with no items, exactly one page, and partial pages

### Use accurate test names
- Test names should precisely describe the scenario being verified
- *Example:* `test_empty_collection` vs `test_no_new_items` convey different conditions

### Match assertions to fixture guarantees
- Only assert on data presence when the fixture explicitly guarantees that data exists
- For structural/protocol tests, verify correctness of whatever data is returned without assuming specific content
- *Example:* Assert `channels_done == true` (protocol correctness) separately from asserting `!channels.is_empty()` (data presence)

### Verify base state before debugging failures
- When a test fails unexpectedly, first verify the code at HEAD actually compiles and tests pass
- Broken imports or syntax errors can mask that tests were never working
- *Example:* `git stash && cargo build` to check if the original code even compiles before investigating test logic

---

## Module Organization

### Public entry points first
- Place public functions at the top of the module, before their private helpers
- Readers should encounter the high-level orchestration first and drill into details top-down
- *Example:* `pub async fn sync_incoming_state(...)` at the top, followed by `process_channel(...)`, then `process_subchannel(...)`
1 change: 1 addition & 0 deletions .claude/rules/debugging.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
**MANDATORY:** When encountering any bug, test failure, or unexpected behavior, invoke `/debugging-wizard` BEFORE proposing fixes. No exceptions.
12 changes: 12 additions & 0 deletions .claude/rules/testing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
**MANDATORY:** Always add unit and/or integration tests for any code change unless clearly not applicable (e.g., documentation, interface definitions without implementation body).

**NEVER** ignore or skip tests:
- No `#[ignore]` attributes in Rust
- No `.skip()` in JavaScript/TypeScript
- If a test is failing, fix the root cause - don't hide it

**Test quality requirements:**
- Tests must be meaningful - `assert_ne` or trivial assertions give false coverage impression
- Use reference vectors when available in the codebase; if none exist, ask user to provide them or instructions to generate them
- Think about edge cases: empty inputs, boundary values, error conditions
- Always assume the worst - test failure modes, not just happy paths
18 changes: 18 additions & 0 deletions .claude/rules/verification.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
**MANDATORY:** Before claiming work is complete, fixed, or passing, invoke `/verification-before-completion`. Run fresh verification commands and confirm output before making any success claims. Evidence before assertions, always.

**Rust verification checklist:**
- `cargo fmt --check` - code formatting
- `cargo clippy` - lints (0 warnings required), including integration tests (all targets)
- `cargo test` - all tests pass

## Cross-layer consistency

When changing Rust code, always check whether the change must be reflected in other layers:
- **TypeScript SDK** (`sdk/`): API contracts, request/response shapes, service semantics
- **E2E tests** (`e2e/`): CLI args, config, spawn logic, test assertions

Propagate changes to affected layers before claiming done.

## E2E tests

E2E tests (`cd e2e && npm test`) are slow. Run them only once, at the very end, after all edits across all layers are complete. Do not run them after each incremental change.
27 changes: 19 additions & 8 deletions .claude/specs/discovery-service/05-security-considerations.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,29 @@ Timing and side-channel attacks are out of scope for the initial implementation.

**RPC fallback budget:** When the service falls back to RPC (during cold start or reorg), a stricter budget MUST apply to prevent amplification attacks. The fallback budget SHOULD be configurable and significantly lower than the cache-served budget.

**Rate limiting:** Per-IP rate limiting is required. Implementation details are left to the deployment configuration.
**Rate limiting:** Per-IP rate limiting and `Retry-After` headers are handled at the reverse proxy / infrastructure level, not by the service itself.

### 5.3.1 Known Attack Vectors (audit 2026-02-04)

The following vectors have been identified and require mitigation:

| Vector | Severity | Status |
|--------|----------|--------|
| Unbounded task spawning from `cursor.channels` / `cursor.subchannels` HashMaps — each entry spawns a tokio task, attacker can pack ~50K entries in a 2MB body | CRITICAL | TODO |
| No explicit request body size limit — Axum 2MB default is ~4000× larger than a legitimate request | CRITICAL | TODO |
| No HTTP-level request timeout — slow RPC responses block worker threads up to 100min per request | HIGH | TODO |
| HashMap deserialization memory spike from large cursors | MEDIUM | mitigated by body limit once set |
| `max_reads: 0` accepted, wastes snapshot creation | LOW | TODO |

## 5.4 Input Validation

All request fields MUST be validated:
The following request fields are validated by the service:

- **block_ref:** Must be a valid block hash. The referenced block must exist and have block number greater than `last_synced_block`. Invalid or unknown block hashes result in an error.
- **last_synced_block:** Must be a valid block hash or empty string for initial sync.
- **private_key:** Must be a valid key format. Invalid format results in an error; incorrect key (decryption failure) is handled per section 8.2.
- **cursor:** Must conform to expected structure. Malformed cursors result in an error.
- **max_reads:** Must be a positive integer within allowed bounds.
- **Address fields:** Must be valid Starknet addresses.
- **max_reads:** Must be within allowed bounds (default 50, max 100). Zero is currently accepted but wastes work.
- **last_known_block:** If provided, checked for canonical status (reorg detection). Returns `BLOCK_REORGED` if no longer canonical.
- **block_ref:** If provided, used as-is for querying. No separate existence check — an invalid hash surfaces as an RPC error.
- **cursor:** Structural validation via serde deserialization. Malformed JSON results in `INVALID_REQUEST`. **Note:** cursor HashMap sizes (`channels`, `subchannels`) are NOT validated — an attacker can submit arbitrarily large maps that spawn unbounded concurrent tasks. Size caps MUST be enforced before task spawning.
- **recipient_address, decryption_key:** Accepted as Felt values without format validation.

## 5.5 Privacy Model

Expand Down
Loading
Loading