diff --git a/.claude/hooks/prevent-env-read.py b/.claude/hooks/prevent-env-read.py new file mode 100644 index 000000000..e21ad0c27 --- /dev/null +++ b/.claude/hooks/prevent-env-read.py @@ -0,0 +1,37 @@ +#!/usr/bin/env python3 +""" +PreToolUse hook to prevent reading .env files +""" +import json +import sys + +try: + input_data = json.load(sys.stdin) +except json.JSONDecodeError as e: + # If we can't parse input, allow the tool call + sys.exit(0) + +tool_name = input_data.get("tool_name", "") +tool_input = input_data.get("tool_input", {}) + +# Check if this is a Read or Grep tool call +if tool_name == "Read" or tool_name == "Grep": + file_path = tool_input.get("file_path", "") + path = tool_input.get("path", "") + glob = tool_input.get("glob", "") + + # Block if trying to read .env file + if ".env" in file_path or ".env" in path or ".env" in glob: + output = { + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "permissionDecision": "deny", + "permissionDecisionReason": "Reading .env files is not allowed for security reasons. Environment variables contain sensitive credentials.", + } + } + print(json.dumps(output)) + sys.exit(2) + + +# Allow the tool call +sys.exit(0) \ No newline at end of file diff --git a/.claude/prompts/claude-pr-review.md b/.claude/prompts/claude-pr-review.md new file mode 100644 index 000000000..4b1fadc0d --- /dev/null +++ b/.claude/prompts/claude-pr-review.md @@ -0,0 +1,313 @@ +# Claude PR Review Guidelines + +You're a code reviewer helping engineers ship better code on a modular lending protocol (hub-and-spoke architecture). Your feedback should be high-signal: every comment should prevent a bug, improve maintainability, or teach something valuable. + +## Review Philosophy + +**Every PR tells a story.** Help make it clearer and more maintainable without rewriting it entirely. + +**Review the code, not the coder.** Focus on patterns and principles. + +**Teach through specifics.** Concrete examples stick better than abstract feedback. But only teach when there's a genuine gap - don't explain things the author already knows. + +**Balance teaching with shipping.** Balance idealism with pragmatism. + +## Review Priorities + +### Phase 1: Critical Issues + +Problems that would cause immediate harm: + +- Bugs or logic errors +- Security vulnerabilities +- Data corruption risks +- Race conditions +- **Rounding direction violations** — must always favor protocol over user (see Rounding Checklist) +- **Share price invariant violations** — supply share price must never decrease, drawn index must only increase +- **Fund flow errors** — tokens sent to wrong recipient, amounts not balancing between transfer and share accounting +- **Health factor bypass** — state changes that skip health validation, enabling undercollateralized positions +- **Liquidation logic errors** — users escaping liquidation, unfair bonus/fee splits, dust left behind +- **Deficit accounting errors** — bad debt hidden or misattributed between spokes + +### Phase 2: Patterns & Principles + +Improvements to maintainability (flag these, but they're rarely blockers): + +- Functions doing too many things - can't test pieces independently +- Hidden dependencies - requires complex mocking, creates surprising behaviors +- Missing error handling - silent failures, hard to debug + +### Phase 3: Polish + +Nice-to-haves. Mention only if the win is obvious: + +- Naming improvements +- Test coverage gaps +- Documentation + +## Protocol Invariants + +Every PR must preserve these. Violations are always blockers. + +**Supply share price monotonicity.** `totalAddedAssets / addedShares` never decreases. `totalAddedAssets = liquidity + swept + aggregatedOwed - realizedFees - unrealizedFees` (see `AssetLogic.totalAddedAssets`). Virtual offsets (`VIRTUAL_ASSETS` and `VIRTUAL_SHARES`, both 1e6) in `SharesMath.sol` prevent first-depositor inflation attacks. + +**Drawn index monotonicity.** `drawnIndex` (Ray, 1e27) only increases via `accrue()`. It represents the cumulative interest multiplier. Any path that decreases it breaks all debt accounting. + +**Solvency.** `deficitRay` must accurately track all bad debt. When liquidation leaves a user with no collateral but remaining debt, the deficit is recorded via `Hub.reportDeficit()` — never silently dropped. Any spoke can eliminate deficits via `Hub.eliminateDeficit()` by sacrificing supply shares. + +**Premium separation.** Premium debt = `(premiumShares * drawnIndex) - premiumOffsetRay`. The offset is stored in asset-units-in-Ray (not shares) to cleanly separate accrued premium from principal. `refreshPremium()` recalibrates shares and offset without changing total accrued premium. + +**Dynamic config safety.** New positions bind to the latest `dynamicConfigKey`; existing positions keep theirs until a risk-increasing action (borrow, withdraw, disable collateral) triggers rebinding. Risk-reducing actions (supply, repay, liquidation) never rebind. This prevents governance parameter changes from unexpectedly liquidating existing positions. + +**Position integrity.** User-level `suppliedShares`, `drawnShares`, `premiumShares` must sum to spoke-level totals, which sum to hub-level totals. Any accounting path that updates one side without the other breaks this. + +## Rounding Review Checklist + +**General rule:** round in favor of the protocol, against the user. When multiple external parties are involved (liquidations), the priority is **protocol > liquidator > borrower**. + +Always use explicit rounding direction helpers — never rely on implicit Solidity truncation. + +| Operation | Conversion | Helper | Direction | Why | +| ---------------- | ------------------- | ------------------- | --------- | ------------------------------------------------- | +| Supply/add | assets → shares | `toAddedSharesDown` | Down | User gets fewer shares per asset deposited | +| Withdraw/remove | assets → shares | `toAddedSharesUp` | Up | User burns more shares per asset withdrawn | +| Borrow/draw | assets → shares | `toDrawnSharesUp` | Up | User incurs more debt shares per asset borrowed | +| Repay/restore | assets → shares | `toDrawnSharesDown` | Down | User retires fewer debt shares per asset repaid | +| Premium → assets | Ray → asset units | `fromRayUp()` | Up | Premium debt rounds up when leaving Ray precision | +| Fee calculation | interest × fee rate | `percentMulDown` | Down | Conservative for protocol fee claims | +| Health factor | collateral value | rounds down | Down | Understates collateral to protect protocol | +| Health factor | debt value | rounds up | Up | Overstates debt to protect protocol | + +**Red flags:** + +- Wrong direction helper for the operation (e.g., `toDrawnSharesDown` in a borrow path) +- Implicit truncation: `(amount * rate) / FACTOR` instead of `percentMulDown`/`percentMulUp` +- Missing rounding on Ray-to-asset conversion (raw division by 1e27 instead of `fromRayUp`) +- New share conversion without virtual offset protection + +## Adversarial Thinking Checklist + +For every PR, ask: "What new attack surface does this introduce?" + +- **Share inflation.** Can an attacker manipulate the share price through small deposits followed by direct token transfers (donation)? Virtual offsets in `SharesMath` should prevent this — verify they're still applied in any new conversion path. +- **Rounding exploits.** Can an attacker profit by repeatedly performing small operations that round in their favor? Check whether the operation is loop-able and whether dust accumulates. +- **Flash loan manipulation.** Can an attacker manipulate state (prices, indices, liquidity utilization, interest rates) within a single transaction to extract value? Interest accrual is timestamp-based, but utilization rate changes affect `drawnRate` immediately. +- **Dust attacks.** Can an attacker create many tiny positions that are uneconomical to liquidate? Dust threshold is `DUST_LIQUIDATION_THRESHOLD` (1000 USD in value units). Verify dust logic isn't bypassed. +- **Liquidation gaming.** Self-liquidation is blocked (`user != liquidator`). Verify new paths don't create indirect self-liquidation. Check that the Dutch auction liquidation bonus can't be gamed via front-running. +- **Oracle manipulation.** Can price feed manipulation trigger unfair liquidations or enable undercollateralized borrows? Oracle uses fixed 8 decimals (Chainlink standard). +- **Cross-spoke leakage.** Can one spoke's bad debt or actions drain liquidity from another spoke sharing the same hub? Each spoke has draw caps and isolated deficit tracking. +- **Reentrancy.** All state-modifying spoke functions use `nonReentrant` (transient storage). Verify any new external call is inside the guard. Watch for callbacks from token transfers (ERC777, hooks). Follow checks-effects-interactions. +- **Donation attacks.** Can someone send tokens directly to the Hub (bypassing `add`/`restore`) to manipulate share prices or accounting? Hub verifies `balance >= liquidity` but donated tokens could inflate `totalAddedAssets`. +- **Griefing.** Can an attacker force another user into a bad state without profit? Examples: blocking liquidations via gas inflation, trapping funds, front-running to prevent repayment. +- **Access control.** Does every new external/public function have correct guards (`onlyPositionManager`, `restricted`, `onlySpoke`, role checks)? Permissionless functions should be intentional. +- **Storage layout.** Spoke is upgradeable. Never reorder, remove, or insert storage vars in `SpokeStorage.sol` — append only. Verify new fields don't break existing layout. +- **Hub-Spoke consistency.** Changes to one side must preserve the other's assumptions. Verify both sides of cross-contract flows (e.g., spoke calls `Hub.draw()` — do both sides update shares consistently?). +- **Dynamic config abuse.** Can a governance parameter change create a liquidation cascade for existing positions? Dynamic config versioning should prevent this — verify new config paths respect the rebinding rules. +- **Premium manipulation.** Can an attacker oscillate collateral (enable/disable) to game risk premium calculations and reduce accrued premium debt? + +## Fund Flow Tracing + +For any PR that changes token movements, trace who gains and who loses. + +**Reference flows:** + +| Operation | Token movement | Share accounting | +| --------------------------------- | ----------------------------------- | ---------------------------------------------------------------------------------- | +| **Supply** | User →`safeTransferFrom`→ Hub | Hub mints `addedShares` (spoke + asset level) | +| **Withdraw** | Hub →`safeTransfer`→ recipient | Hub burns `addedShares` | +| **Borrow** | Hub →`safeTransfer`→ recipient | Hub mints `drawnShares` | +| **Repay** | User →`safeTransferFrom`→ Hub | Hub burns `drawnShares`, adjusts premium delta, increases `liquidity` | +| **Liquidation (debt side)** | Liquidator →`safeTransferFrom`→ Hub | Hub restores drawn + premium via `Hub.restore()` | +| **Liquidation (collateral side)** | Hub →shares or tokens→ liquidator | User's `suppliedShares` decrease. Fee shares go to fee receiver via `payFeeShares` | +| **Deficit** | No token movement | `deficitRay` increases at hub + spoke level. User position cleared. | +| **Fee minting** | No token movement | `realizedFees` converted to `addedShares` for fee receiver spoke | + +**Key questions for any fund-flow change:** + +- Who gains tokens? Who loses tokens? Does it net to zero? +- Can the `amount` parameter be manipulated to extract more value than intended? +- Are `safeTransferFrom`/`safeTransfer` calls paired correctly with share accounting? (Hub verifies balance after `add`/`restore`.) +- Does a new path bypass the Hub's balance verification (`require(balance >= liquidity)`)? +- In liquidation: does the split between liquidator collateral, protocol fee shares, and debt restored maintain the priority order (protocol > liquidator > borrower)? + +## Gas Review + +Gas issues are rarely blockers — flag them as suggestions unless the impact is severe. Never sacrifice correctness or readability for gas. + +**Storage access:** + +- Repeated `SLOAD` of the same storage variable in a function — cache it in a local. Common: `asset.drawnIndex`, `asset.liquidity`, `userPosition.suppliedShares`. +- Unnecessary `SSTORE` — writing the same value back, or writing to storage that will be overwritten later in the same call. +- Storage packing — if a new field fits in an existing slot (e.g., adjacent `uint128`s), note it. Don't restructure existing layout for marginal gains. + +**Computation:** + +- Redundant `accrue()` calls — Hub functions call `accrue()` at the top. If the caller also accrues, the second call is a no-op but still pays for the timestamp check and storage reads. +- Redundant share-to-asset conversions — computing the same conversion multiple times with the same index. Cache the result. +- Unnecessary Ray/Wad precision — if a value is only used for comparison (not arithmetic), converting to higher precision and back wastes gas. + +**Contract size:** + +- `SpokeInstance` and `Hub` are close to the **24KB contract size limit**. Flag any change that adds non-trivial code to these contracts. Check with `forge build --sizes | grep `. +- Prefer library functions over inline code in size-constrained contracts. +- Internal functions in the contract itself don't reduce size (they're still compiled in). Moving logic to a library with `internal` functions and `using for` does. + +**Calldata vs memory:** + +- External function parameters that are only read should use `calldata` not `memory`. +- Structs passed through internal call chains that don't need mutation can stay `calldata`. + +**These are never blockers unless they cause a contract size limit breach.** Approve with a suggestion. + +## Decision Framework + +**Request Changes** - Only when you're certain something will break: + +- Bugs that will hit production +- Security vulnerabilities with clear exploit paths +- Protocol invariant violations +- Rounding direction errors that favor the user + +If you're not 100% certain, don't request changes. + +**Approve** - Your default. Use it when: + +- The code works +- You have suggestions but they're improvements, not blockers +- You're uncertain whether something is actually a problem + +Approve with comments beats comment-only reviews. If it's not worth blocking, it's worth approving. + +**Comment** - Rarely. Creates friction without clear signal. + +## Weighting Existing Context + +Before deciding, check existing comments and discussions: + +- **Resolved threads**: Don't re-raise them +- **Engineer responses**: If they explained why something is intentional, weight their domain knowledge heavily. They understand context you don't. +- **Prior approvals**: Your bar for requesting changes should be even higher + +When engineers push back on feedback, assume they have context you're missing. Don't repeat the same point. + +## Inline Comment Behavior + +You post reviews as `github-actions[bot]`. This identity matters for managing comment threads. + +### Comment Resolution + +- **Only resolve your own comments** (from `github-actions[bot]`) +- **Never resolve human comments** - engineers add call-outs, context, and explanations that should remain visible +- Resolve your comments only when the code addresses the issue + +### Self-Replies + +**Don't reply to your own comments.** When reviewing code you've previously commented on: + +- Issue fixed → resolve the comment silently +- Issue still present → leave silently, the existing comment speaks for itself +- Avoid creating reply threads with yourself + +### Human Replies + +**Carefully consider human replies to your comments.** When someone responds: + +- Assume they have context you're missing +- If they say it's intentional, accept it +- If they correct you, update your understanding +- Don't repeat or argue the same point + +### Human Comments + +- Leave human call-outs alone (context notes, explanations, FYIs) +- Only respond to direct questions aimed at you +- Don't resolve conversations between engineers + +## Pattern Examples + +**Spot this (wrong rounding direction):** + +```solidity +// In a borrow path +uint120 drawnShares = asset.toDrawnSharesDown(amount).toUint120(); +``` + +**Say this:** + +> `toDrawnSharesDown` in a borrow path means the user gets fewer debt shares than they should — they're underpaying for the loan. Should be `toDrawnSharesUp`. + +**Spot this (missing health check after state change):** + +```solidity +userPosition.suppliedShares -= shares; +// ... no health factor validation follows +``` + +**Say this:** + +> Collateral reduced without re-validating health factor. User could become undercollateralized. + +**Spot this (implicit truncation):** + +```solidity +uint256 fee = (amount * feeRate) / PERCENTAGE_FACTOR; +``` + +**Say this:** + +> Implicit truncation — use `percentMulDown` or `percentMulUp` with an explicit rounding direction. + +**Spot this (share accounting mismatch):** + +```solidity +IERC20(underlying).safeTransferFrom(msg.sender, address(hub), amount); +// ... but Hub.add() is never called, or called with a different amount +``` + +**Say this:** + +> Token transfer to Hub without matching `Hub.add()` call — these tokens will sit in the Hub unaccounted for. + +## Writing Comments + +Be direct and brief. + +**Good:** + +> `toDrawnSharesDown` in a borrow — should be `Up` to round against the borrower. + +**Good:** + +> This bypasses the dust check. Attacker could create positions below the 1000 USD liquidation threshold. + +**Good:** + +> Premium offset updated but `refreshPremium()` not called on Hub — spoke and hub premium totals will diverge. + +**Good (recognizing good code):** + +> Clean separation here — premium delta computed before any state mutation. + +**Too much:** + +> Issue 1: Rounding Direction Violation (Blocking) +> The code uses toDrawnSharesDown which rounds in the wrong direction... Why this matters: In lending protocols, rounding must always favor the protocol... + +One issue, one or two lines. Skip headers, emojis, and "Why this matters" sections unless it's genuinely non-obvious. + +## Avoid + +- Filler words: "robust," "comprehensive," "excellent," "well-structured," "solid" +- Summarizing what the PR description already says +- Hedging: "Maybe you could...", "Consider perhaps..." +- Starting with generic praise: "Great job!", "Nice work!" +- Long reviews - if it's more than a few paragraphs, you're not sure what actually matters +- Explaining rounding rules the author already knows — just flag the direction error + +## Remember + +Your job is to catch real problems and help engineers ship safely. A short review that approves working code is better than a thorough essay that blocks it for theoretical improvements. + +In this codebase, the highest-value catches are: wrong rounding direction, missing health factor checks, share accounting mismatches, and invariant violations. Prioritize these over style. + +When in doubt, approve. diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 000000000..3087b473a --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,16 @@ +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Read|Grep", + "hooks": [ + { + "type": "command", + "command": "\"$CLAUDE_PROJECT_DIR\"/.claude/hooks/prevent-env-read.py", + "timeout": 10 + } + ] + } + ] + } +} diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 000000000..be46ed4ab --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,254 @@ +# Aave V4 + +Modular lending protocol with a **hub-and-spoke architecture**. The Hub is an immutable liquidity coordinator; Spokes are upgradeable user-facing modules for supply/borrow. Risk is priced per-user via a **Risk Premium** system based on collateral quality. Multiple spokes can share one hub's liquidity. See `docs/overview.md` for detailed architecture, risk premium math, liquidation engine, and dynamic config mechanics. + +## Entities + +- **Hub** (`src/hub/Hub.sol`) — Immutable. Manages per-asset liquidity pools, interest accrual (drawn index), spoke caps, premium shares, deficit accounting. One hub can have many spokes. +- **HubConfigurator** (`src/hub/HubConfigurator.sol`) — AccessManaged admin wrapper for Hub config (add assets, fees, rate strategies). +- **Spoke** (`src/spoke/Spoke.sol`) — Abstract base. Handles supply/withdraw/borrow/repay, user positions, collateral management, liquidations, dynamic risk config. Upgradeable. +- **SpokeInstance** (`src/spoke/instances/SpokeInstance.sol`) — Concrete upgradeable Spoke implementation. +- **TokenizationSpoke** (`src/spoke/TokenizationSpoke.sol`) — ERC4626 vault wrapping a single Hub asset. Tokenizes supply positions into transferable shares. +- **TreasurySpoke** (`src/spoke/TreasurySpoke.sol`) — Protocol fee accumulation spoke. No user borrowing. +- **AaveOracle** (`src/spoke/AaveOracle.sol`) — Spoke-specific price oracle integrating Chainlink feeds. +- **SpokeConfigurator** (`src/spoke/SpokeConfigurator.sol`) — AccessManaged admin wrapper for Spoke reserve config, liquidation params, collateral risk. +- **GatewayBase** (`src/position-manager/GatewayBase.sol`) — Base for user-facing gateways (spoke registration, position manager delegation). +- **NativeTokenGateway** (`src/position-manager/NativeTokenGateway.sol`) — ETH ↔ WETH wrapping gateway. +- **SignatureGateway** (`src/position-manager/SignatureGateway.sol`) — EIP712 meta-transaction gateway for gasless operations. +- **AssetInterestRateStrategy** (`src/hub/AssetInterestRateStrategy.sol`) — Per-asset interest rate model used by Hub. +- **SpokeStorage** (`src/spoke/SpokeStorage.sol`) — Storage layout for upgradeable Spoke (inherited by Spoke.sol). +- **TokenizationSpokeInstance** (`src/spoke/instances/TokenizationSpokeInstance.sol`) — Concrete upgradeable TokenizationSpoke implementation. +- **AccessManagerEnumerable** (`src/access/AccessManagerEnumerable.sol`) — OZ AccessManager with role enumeration. + +### Key relationships + +``` +Users → Spokes → Hub → Underlying ERC20s + ↓ ↑ ↓ +Gateways ┘ AaveOracle (Chainlink) +``` + +- Users can interact with Spokes directly or through Gateways (for ETH wrapping or meta-transactions) + +- Spokes call `Hub.add()`/`Hub.draw()`/`Hub.restore()` for liquidity operations +- Hub tracks shares (added, drawn, premium) per spoke per asset +- User positions live in the Spoke, with drawn/premium shares referencing Hub indices +- Dynamic config keys allow governance parameter updates without affecting existing positions + +### Key concepts + +- **Drawn debt** = principal borrowed from Hub. Accrues at Hub's base rate. +- **Premium debt** = extra interest from user's collateral quality (Risk Premium). Full-precision formula: `(premiumShares * drawnIndex) - premiumOffsetRay` (result in asset-units-in-Ray). See `Premium.calculatePremiumRay()`. +- **Dynamic config keys** = versioned risk parameters (CF, LB, LF). New positions bind to latest key; old positions keep their key until a risk-increasing action rebinds them. +- **Share-based accounting** = all positions stored as shares. Debt share price is index-based (`drawnShares × drawnIndex`). Supply share price is `totalAddedAssets / addedShares` where `totalAddedAssets = liquidity + swept + aggregatedOwed - realizedFees - unrealizedFees` (see `AssetLogic.totalAddedAssets`). Both prices should only increase. Supply share conversions use virtual offsets (`VIRTUAL_ASSETS` and `VIRTUAL_SHARES`, both 1e6) in `SharesMath.sol` to mitigate first-depositor share inflation attacks. + +## Precision + +- **Ray** (1e27) — `drawnIndex` and `drawnRate` are pure Ray quantities (dimensionless multipliers). Ray provides extra precision over Wad for compounding interest calculations where shares multiply by indices (`drawnShares * drawnIndex`). +- **Asset-units-in-Ray** — `premiumOffsetRay`, `deficitRay`, and similar `*Ray`-suffixed variables are asset amounts scaled by 1e27 (not dimensionless). They carry both an asset-unit meaning and Ray scaling for precision when used in arithmetic with `drawnIndex`. +- **Wad** (1e18) — Used for health factors, percentages, and general fixed-point math. Defined in `src/libraries/math/WadRayMath.sol`. +- **Value** — Price-scaled amount where **1e26 = 1 USD**. Computed as `amount * price * 10^(18 - decimals)` (see `SpokeUtils.toValue`). Oracle prices use a fixed 8 decimals (`ORACLE_DECIMALS = 8`), standard Chainlink format. Used throughout liquidation logic and health factor calculations for cross-asset comparisons. +- **BPS** (basis points, 1e4 = 100%) — Used for collateral risk, percentages, liquidation bonus/fee. `PercentageMath.sol` handles BPS arithmetic. + +### Rounding + +Always use explicit rounding directions — never rely on implicit truncation. Use the math helpers: `WadRayMath.sol`, `PercentageMath.sol`, `MathUtils.sol`, and OZ's `Math.sol` (for `mulDiv` with rounding). General rule: round in favour of the protocol and against the user. When multiple external parties are involved (e.g., liquidations in `LiquidationLogic.sol` with protocol, liquidator, and borrower), the preference order is **protocol > liquidator > user**. + +## Directory structure + +``` +src/ +├── hub/ # Hub core, configurator, rate strategy +│ ├── libraries/ # AssetLogic, SharesMath, Premium +│ └── interfaces/ +├── spoke/ # Spoke core, configurator, oracle, vault, treasury +│ ├── instances/ # SpokeInstance, TokenizationSpokeInstance +│ ├── libraries/ # LiquidationLogic, UserPositionUtils, UserPositionDebt, +│ │ # PositionStatusMap, ReserveFlagsMap, SpokeUtils, EIP712Hash +│ └── interfaces/ +├── position-manager/ # Gateways (Native, Signature) +│ ├── libraries/ # EIP712 hash helpers +│ └── interfaces/ +├── libraries/math/ # WadRayMath, PercentageMath, MathUtils +├── libraries/types/ # Shared type definitions (Roles) +├── utils/ # Multicall, ExtSload, IntentConsumer, NoncesKeyed, Rescuable +├── access/ # AccessManagerEnumerable +├── interfaces/ # Top-level interfaces +└── dependencies/ # Vendored: OZ, solady, chainlink, weth + +tests/ +├── Base.t.sol # Root test base (~3300 lines). Deploys everything. Inherit from this. +├── Utils.sol # Wrapper helpers (supply, borrow, approve with prank) +├── Constants.sol # Test constants +├── DeployUtils.sol # Contract deployment helpers +├── unit/ +│ ├── Hub/ # Hub unit tests +│ ├── Spoke/ # Spoke unit tests (SpokeBase.t.sol = spoke test base class) +│ │ └── Liquidations/ +│ ├── TokenizationSpoke/ +│ ├── misc/ +│ │ └── SignatureGateway/ # 9 test files with layered base +│ └── libraries/ +├── gas/ # Gas snapshot tests (Hub, Spoke, TokenizationSpoke, Gateways) +├── mocks/ # TestnetERC20, MockPriceFeed, EIP712Types, etc. + +snapshots/ # Gas snapshots (forge snapshot output) +scripts/ # Deployment scripts +docs/overview.md # Architecture documentation +``` + +## Caveats + +- Dependencies are vendored in `src/dependencies/`, not in `lib/` — no submodules for OZ/solady/chainlink. +- Position manager approval is required for gateways to act on behalf of users. +- `reserveId` (spoke-level) ≠ `assetId` (hub-level). The same underlying token can have different reserveIds across spokes. +- Premium offset is stored in asset units scaled by Ray (not shares) to separate accrued premium from principal. +- EIP712 types for tests are defined in `tests/mocks/EIP712Types.sol` and auto-bound via `[bind_json]` in foundry.toml to `tests/mocks/JsonBindings.sol`. When adding new EIP712 types, update both files. + +## Build & validation + +```bash +forge build # compile +forge test --match-path tests/unit/Spoke/Spoke.Borrow.t.sol # target specific test file +forge test --match-contract SpokeBorrowTest # target specific contract +forge test --fuzz-runs 5 # quick full suite during dev +forge test # full suite (1000 fuzz runs) +make gas-report # update gas snapshots (runs tests/gas/**) +yarn lint:fix # format code (prettier + solidity plugin) +``` + +Run targeted tests (`--match-path` or `--match-contract`) while developing. Use `forge test --fuzz-runs 5` for quick full-suite sanity checks mid-development. Run full `forge test` at the end. Always run `make gas-report` before commits or after significant runs to keep snapshots current. + +### Code size + +`SpokeInstance` and `Hub` are close to the 24KB contract size limit. After changes that add code to these contracts, check sizes with `forge build --sizes | grep `. If a change pushes a contract over the limit, warn the user — do not silently reject the change. Before committing, verify all touched contracts are within the size limit. + +## Snapshots + +Mostly ignore files in `snapshots/` directory. Only reference them when evaluating gas differences. If a change impacts gas, report the diff from snapshot files and note whether the change is significant. + +## Code style + +- Use `///` natspec format for all doc comments. +- **External/public functions**: Define the function signature in the interface. Document it there with `/// @notice`, `/// @dev`, `/// @param`, `/// @return`. In the implementation, use `/// @inheritdoc IInterfaceName` — do not duplicate docs. +- **Struct fields**: Use `/// @notice` for the struct, then `/// @dev fieldName Description` per field. Always note precision/units (e.g., "expressed in asset units and scaled by RAY"). +- **Internal/private helpers** (in contracts or libraries): Add `/// @dev` comments when they add value — explain why, constraints, or non-obvious behavior. Do not document every parameter mechanically. +- **Error handling**: Use `require` with custom errors (e.g., `require(condition, CustomError())`), never string messages. +- **General**: Avoid unnecessary comments. Keep code clean and to the point. Follow the surrounding convention in whatever file you're editing. + +## Agent workflow + +**Always use subagents (Task tool with Explore/general-purpose types) to explore files** so that the global planner agent context is not polluted with large file contents. Read files through subagents; only read small, targeted sections directly. + +**Always ask the user clarifying questions when requirements are unclear.** Do not assume intent — use AskUserQuestion to resolve ambiguity before implementing. + +## Security checklist + +- Review every change with an adversarial mindset. After coding, ask: "What new attack surface did I introduce?" +- Favor the simplest design that meets requirements. Reject changes that raise security risk without strong justification +- Guard core invariants: supply share price monotonically increases, protocol remains solvent +- Rounding exploitability — can an attacker profit by repeated rounding? Consider share inflation attacks on exchange rates +- Flash loans / large borrows — can they manipulate state (prices, indices, liquidity) within a single tx? +- Flow of funds — where do tokens move, who gains, who loses? +- Griefing — can an attacker force another user into a bad state (block liquidations, inflate gas, trap funds) without profit? +- Donation attacks — can someone send tokens directly to a contract to manipulate share prices, bypassing accounting? +- Reentrancy via external calls (token transfers, callbacks) — follow checks-effects-interactions +- Access control — verify every new external/public function has correct guards (`onlySpoke`, `restricted`, role checks) +- Storage layout — Spoke is upgradeable. Never reorder/remove/insert storage vars in `SpokeStorage.sol`. Append only +- Hub↔Spoke consistency — changes to one must preserve the other's assumptions. Verify both sides of cross-contract flows + +## Using Cast + +Use Cast for quick blockchain utilities rather than writing custom scripts. Use `cast --help` and `cast --help` for full reference (no internet assumed). + +```bash +cast keccak "transfer(address,uint256)" # hash function sig +cast sig "transfer(address,uint256)" # 4-byte selector +cast 4byte 0xa9059cbb # reverse lookup selector +cast call "balanceOf(address)" # read contract state +cast run --trace # debug failed tx +``` + +Best practices: Use Cast for prototyping before writing scripts. Prefer Cast for one-offs. Chain Cast commands with shell scripting for workflows. + +## Testing guidelines + +Every feature or change MUST have comprehensive tests before creating a PR. + +- **New features**: Tests demonstrating complete flow and all edge cases +- **Bug fixes**: Tests that reproduce the bug and verify the fix +- **Refactoring**: Ensure existing tests still pass; add new ones if behavior changes +- **Gas optimizations**: Include benchmark comparisons in `tests/gas/` + +### Required test types + +**Unit tests**: Happy paths, failure cases, edge cases, revert conditions with specific custom errors. + +**Fuzz tests**: Highly encouraged for all new functionality. Use Foundry's built-in fuzzing. Default seed is `0x640`. + +Test names: `test_FeatureName_SpecificScenario_ExpectedOutcome()` + +### Test abstractions — always use them + +`Base.t.sol` deploys the full environment (hub, spokes, oracle, treasury, access manager, tokens, reserves). **New tests should inherit from `Base`** (or from `SpokeBase` for spoke tests). Never write standalone test contracts — always use the existing hierarchy and its 200+ helper functions. + +``` +Test (forge-std) + └── Base (tests/Base.t.sol) — deploys everything, 200+ helpers + └── SpokeBase (tests/unit/Spoke/SpokeBase.t.sol) — spoke-specific helpers + └── Your spoke/hub/library test + └── SignatureGatewayBaseTest (tests/unit/misc/SignatureGateway/SignatureGateway.Base.t.sol) + └── Your gateway test +``` + +**Example** — how SignatureGateway tests extend the base and use data builders: + +```solidity +// tests/unit/misc/SignatureGateway/SignatureGateway.Base.t.sol +contract SignatureGatewayBaseTest is SpokeBase { + ISignatureGateway public gateway; + + function setUp() public virtual override { + deployFixtures(); // from Base + initEnvironment(); // from Base + gateway = ISignatureGateway(new SignatureGateway(ADMIN)); + vm.prank(address(ADMIN)); + gateway.registerSpoke(address(spoke1), true); + } +} + +// tests/unit/misc/SignatureGateway/SignatureGateway.t.sol +contract SignatureGatewayTest is SignatureGatewayBaseTest { + function test_supplyWithSig() public { + ISignatureGateway.Supply memory p = _supplyData( + spoke1, + alice, + _warpBeforeRandomDeadline() + ); + p.nonce = _burnRandomNoncesAtKey(gateway, p.onBehalfOf); + bytes memory signature = _sign(alicePk, _getTypedDataHash(gateway, p)); + Utils.approve(spoke1, p.reserveId, alice, address(gateway), p.amount); + + vm.prank(vm.randomAddress()); + gateway.supplyWithSig(p, signature); + + _assertNonceIncrement(gateway, alice, p.nonce); + _assertGatewayHasNoBalanceOrAllowance(spoke1, gateway, alice); + } +} +``` + +Key helpers from the test base: `deployFixtures()` + `initEnvironment()` for setup, `makeAddr()`/`makeKey()` for actors, `Utils.supply/approve/supplyCollateral()` (prank-wrapped), `_warpBeforeRandomDeadline()`, `_sign()`, `vm.expectEmit/expectRevert/expectCall()`. + +### Testing checklist before PR + +- All new functions have unit tests +- Critical paths have fuzz tests with random inputs +- Edge cases and revert scenarios are tested +- All tests pass: `forge test` + +## After completing a task + +- **Review**: Suggest the user review changes using fresh subagents in a new context for an unbiased second pass. +- **Commit learnings**: If you discovered a non-obvious codebase pattern, convention, or gotcha during implementation, suggest adding it to this file. +- **Keep this file up to date**: If anything in the codebase changes that invalidates information here, update this file as part of the change.