Skip to content
Open
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
126 changes: 126 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
# CLAUDE.md

This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.

## Project Overview

stOLAS is an ERC4626 liquid staking vault for OLAS tokens. Users deposit OLAS on L1 (Ethereum) and receive stOLAS tokens. The deposited OLAS is bridged to L2 chains (Gnosis, Base, Mode) for active staking in the Autonolas service ecosystem. Rewards flow back from L2 to L1, increasing the stOLAS price-per-share.

## Build & Test Commands

```bash
make install # Install all dependencies (poetry, forge, yarn)
make build # forge build
make fmt # forge fmt && forge fmt --check
make lint # solhint on contracts/**/*.sol
make tests # forge test -vvv
make tests-hardhat # Hardhat JS tests (temporarily renames LzOracle.sol)
make tests-coverage # forge coverage -vvvv

# Run a single Forge test file
forge test --match-path "test/LiquidStaking.t.sol" -vv

# Run a single Forge test function
forge test --match-test "testDepositAndStake" -vvv

# Hardhat test variants
npm run test:hardhat # Full JS test suite
npm run test:fast # Optimized subset
npm run test:original # Original comprehensive suite
```

**Note:** Hardhat tests temporarily rename `LzOracle.sol` to avoid compilation conflicts. The npm scripts handle this automatically.

## Solidity Configuration

- **Compiler:** 0.8.30, optimizer enabled (1M runs), viaIR, EVM target: Prague
- **Source directory:** `contracts/`
- **Key remappings** (in `foundry.toml`): `@openzeppelin`, `@solmate`, `@registries`, `@layerzerolabs`, `@gnosis.pm`

## Architecture

### L1 Contracts (`contracts/l1/`)

| Contract | Role |
|---|---|
| **stOLAS** | ERC4626 vault. Tracks `stakedBalance`, `vaultBalance`, `reserveBalance`. PPS = totalReserves / totalSupply |
| **Depository** | Routes deposits to L2 staking models. Manages model lifecycle (Active/Inactive/Retired) and product types (Alpha/Beta/Final) |
| **Treasury** | Withdrawal requests via ERC6909 tokens. Triggers L2 unstaking if vault liquidity is insufficient |
| **Distributor** | Receives bridged rewards. Splits between veOLAS lock and stOLAS vault top-up |
| **Lock** | veOLAS management for governance voting power |
| **UnstakeRelayer** | Receives OLAS from retired L2 staking models, returns to stOLAS reserve |

### L2 Contracts (`contracts/l2/`)

| Contract | Role |
|---|---|
| **StakingManager** | Orchestrates service deployment/staking. Creates ActivityModule BeaconProxy instances per service |
| **ExternalStakingDistributor** | Manages external staking on third-party staking proxies. Deploys services (creates Safe multisigs with self as module), stakes/unstakes/re-stakes them, claims and distributes rewards (split between Collector, protocol, and curating agent per configurable factors). Supports V1 (rewards on multisig) and V2 (rewards on contract) staking types. Access-controlled via owner, whitelisted managing agents (for unstakes), and per-proxy curating agents guarded by staking guards. Receives OLAS deposits from L2 staking processor and handles withdraw/unstake requests back through Collector |
| **StakingTokenLocked** | Restricted StakingToken that only allows StakingManager as staker |
| **ActivityModule** | Per-service BeaconProxy. Verifies liveness, shields staking funds, triggers reward claims |
| **Collector** | Collects L2 rewards and bridges to L1 (REWARD→Distributor, UNSTAKE→Treasury, UNSTAKE_RETIRED→UnstakeRelayer) |

### Bridging (`contracts/l1/bridging/`, `contracts/l2/bridging/`)

- LayerZero V2 for cross-chain messaging
- Chain-specific processors: `GnosisDepositProcessorL1`/`L2`, `BaseDepositProcessorL1`/`L2`, `DefaultDepositProcessorL1`/`L2`
- `LzOracle` for LayerZero-driven staking model management

### Core Patterns

- **Proxy architecture:** UUPS-style via `Implementation.sol` (owner management + upgrade logic) and `Beacon.sol` (BeaconProxy for ActivityModules)
- **ERC standards:** ERC4626 (vault), ERC6909 (withdrawal tickets), ERC721 (service NFTs from Autonolas registry)
- **Libraries:** Solmate (ERC4626, ERC6909, ERC721), OpenZeppelin (security utilities), Autonolas Registries (staking infra)

### Cross-Chain Flow

```
Stake: Depository (L1) → DepositProcessor → Bridge → StakingProcessorL2 → StakingManager (L2)
Rewards: Collector (L2) → Bridge → Distributor (L1) → stOLAS
Unstake: Collector (L2) → Bridge → Treasury/UnstakeRelayer (L1) → stOLAS
```

## Deployment

- Scripts in `scripts/deployment/` follow numbered sequences: `deploy_l1_01_*` through `deploy_l1_15_*`, `deploy_l2_01_*` through `deploy_l2_09_*`
- Configuration scripts: `script_l1_*`, `script_l2_*` for post-deployment setup
- Contract addresses: `doc/configuration.json`
- Finalized ABIs: `abis/0.8.30/`
- Static audit: `./scripts/deployment/script_static_audit.sh eth_mainnet NETWORK_mainnet`

## ERC4626 Caveat

`deposit` is meant to be called only via **Depository**, and `redeem` only via **Treasury**. The `mint`/`withdraw` functions are non-standard and not for external use.

## Audit Findings & Resolutions

The project has undergone 9 internal audits (`audits/audit1` through `audits/audit9`) and 1 external audit (CODESPECT).

### audit8 (2026-03-08) — all informational, all fixed
- **INFO-1**: Treasury `requestToWithdraw` didn't forward `msg.value` to unstake calls → Fixed: validates and forwards ETH correctly
- **INFO-2**: Distributor `_increaseLock` left dangling OLAS approval on failure → Fixed: resets approval to 0
- **INFO-3**: ERC6909 withdrawal tokens are transferable → By design
- **INFO-4**: Permissionless trigger functions → By design

### audit9 (2026-03-18) — 5 Low, 5 Informational, all resolved
- **L-1**: `reStake` used dead `mapCuratingAgents` mapping → Fixed: uses `mapServiceIdCuratingAgents[serviceId]`
- **L-2**: `stOLAS.initialize()` no access control → By design (atomic deployment)
- **L-3**: `DefaultDepositProcessorL1.drain()` sends ETH to `address(0)` → Fixed: removed `drain()` entirely (ETH cannot get stuck)
- **L-4**: Distributor dangling approval → Fixed (same as audit8 INFO-2)
- **L-5**: `StakingTokenLocked.maxNumInactivityPeriods` unused → By design (backward compatibility)
- **INFO-1**: `setCuratingAgents` `stakingHash` in loop → Fixed: moved before loop
- **INFO-2**: `unstakeAndWithdraw` missing entry validation → Fixed: added zero-checks at entry
- **INFO-3**: `claim()` permissionless → By design
- **INFO-4**: `unstakeRetired()` permissionless → By design
- **INFO-5**: `LzOracle._lzReceive` trusts LZ Read → By design

## Modified Contracts (not yet re-deployed)

The following contracts have been modified after audit8/audit9 fixes and require re-deployment:

| Contract | Layer | Changes |
|---|---|---|
| `contracts/l1/Treasury.sol` | L1 | `requestToWithdraw` now validates `msg.value`, forwards ETH to `unstakeExternal`/`unstake`; added `WrongArrayLength` error |
| `contracts/l1/Distributor.sol` | L1 | `_increaseLock` resets OLAS approval to 0 on failure |
| `contracts/l1/bridging/DefaultDepositProcessorL1.sol` | L1 | Removed `drain()` function and `Drained` event |
| `contracts/l2/ExternalStakingDistributor.sol` | L2 | `reStake` uses `mapServiceIdCuratingAgents` instead of dead `mapCuratingAgents`; `setCuratingAgents` computes `stakingHash` outside loop; `unstakeAndWithdraw` validates `stakingProxy`/`serviceId` at entry |
12 changes: 7 additions & 5 deletions audits/audit8/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,33 +88,35 @@ The protocol demonstrates mature defensive coding patterns:
### Medium: 0
### Low: 0

### Informational: 4
### Informational: 4 (all fixed)

#### INFO-1: Missing ETH Forwarding in Treasury.requestToWithdraw
#### INFO-1: Missing ETH Forwarding in Treasury.requestToWithdraw — FIXED
| | |
|---|---|
| **Location** | Treasury.sol:220-221, 227-228 |
| **Description** | `requestToWithdraw` is `payable` but doesn't forward `msg.value` to `Depository.unstake`/`unstakeExternal` calls |
| **Impact** | None currently — Optimism and Gnosis bridges don't require ETH for L1→L2 messages. If a bridge requiring ETH is added, unstaking from Treasury would fail. |
| **Recommendation** | Add `{value: ...}` forwarding or document that future bridges must not require ETH for L1→L2 UNSTAKE messages |
| **Resolution** | Fixed. `requestToWithdraw` now validates `msg.value`, tracks `remainingValue`, and forwards `{value: externalValue}` to `unstakeExternal` and `{value: remainingValue}` to `unstake`. Reverts if leftover ETH remains. |

#### INFO-2: Dangling OLAS Approval in Distributor
#### INFO-2: Dangling OLAS Approval in Distributor — FIXED
| | |
|---|---|
| **Location** | Distributor.sol:75 |
| **Description** | If `Lock.increaseLock()` fails via low-level call, OLAS approval to Lock remains until next `distribute()` call |
| **Impact** | None — Lock is immutable trusted address. Approval overwritten on next distribute(). |
| **Recommendation** | Consider resetting approval to 0 after failed lock call |
| **Resolution** | Fixed. Added `IToken(olas).approve(lock, 0)` in the failure branch of `_increaseLock()`. |

#### INFO-3: ERC6909 Withdrawal Tokens Are Transferable
#### INFO-3: ERC6909 Withdrawal Tokens Are Transferable — by design
| | |
|---|---|
| **Location** | Treasury.sol:190, solmate ERC6909 |
| **Description** | Withdrawal claim tokens can be transferred. New holder can finalize withdrawal after delay. |
| **Impact** | Feature, not bug. Enables secondary market for withdrawal claims. |
| **Recommendation** | Document this behavior for users |

#### INFO-4: Multiple Permissionless Trigger Functions
#### INFO-4: Multiple Permissionless Trigger Functions — by design
| | |
|---|---|
| **Location** | Depository.sol:763,829; Distributor.sol:125; UnstakeRelayer.sol:60; Collector.sol:317; ActivityModule.sol:303 |
Expand Down
36 changes: 23 additions & 13 deletions audits/audit9/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ This audit combines:
1. **Delta review**: Changes since audit8 — 2 contracts modified (+150 / -24 lines)
2. **Full project re-audit**: All 25 production contracts reviewed against Security Audit Playbook v2.9 (checklists L1-L65, T13-T35, DeFi items 1-140)

**Result: 1 Medium, 5 Low, 5 Informational findings.**
**Result: 1 Medium, 5 Low, 5 Informational findings. All actionable findings fixed.**

The project has strong security fundamentals: stOLAS correctly uses internal accounting (`totalReserves`) making it immune to share inflation attacks, bridge contracts properly validate message sources with replay protection, and reentrancy guards are comprehensive across all contracts (transient on L1, uint256 on L2). No critical or high-severity issues found.

Expand Down Expand Up @@ -60,12 +60,13 @@ Total: 160 + 16 + 16 + 16 + 8 = 216 bits (fits uint256).

## Part 2: Full Project Findings

### L-1: `reStake` uses dead `mapCuratingAgents` mapping — curating agents cannot restake
### L-1: `reStake` uses dead `mapCuratingAgents` mapping — curating agents cannot restake — FIXED

| | |
|---|---|
| **Location** | ExternalStakingDistributor.sol:804 |
| **Severity** | Low |
| **Resolution** | Fixed. Replaced `mapCuratingAgents[msg.sender]` with `mapServiceIdCuratingAgents[serviceId] == msg.sender`, which checks the curating agent who originally staked the service. |

**Code (line 804):**
```solidity
Expand Down Expand Up @@ -96,12 +97,13 @@ if (!hasAccess) {

---

### L-2: `stOLAS.initialize()` has no access control — front-run risk
### L-2: `stOLAS.initialize()` has no access control — front-run risk — by design

| | |
|---|---|
| **Location** | stOLAS.sol:82-105 |
| **Severity** | Low |
| **Resolution** | By design. Deployment is atomic — initialization is checked offchain before integration with other contracts. |

**Code (line 82):**
```solidity
Expand All @@ -126,12 +128,13 @@ function initialize(address _treasury, address _depository, address _distributor

---

### L-3: `DefaultDepositProcessorL1.drain()` sends ETH to `address(0)` after `setL2StakingProcessor`
### L-3: `DefaultDepositProcessorL1.drain()` sends ETH to `address(0)` after `setL2StakingProcessor` — FIXED

| | |
|---|---|
| **Location** | DefaultDepositProcessorL1.sol:155-175 |
| **Severity** | Low |
| **Resolution** | Fixed. Removed `drain()` and its `Drained` event entirely — ETH cannot get stuck in deposit processors (all msg.value is either forwarded to the bridge or refunded as leftovers in the same transaction). |

**Code:**
```solidity
Expand All @@ -150,12 +153,13 @@ function drain() external {

---

### L-4: `Distributor._increaseLock()` leaves dangling OLAS approval on failure
### L-4: `Distributor._increaseLock()` leaves dangling OLAS approval on failure — FIXED

| | |
|---|---|
| **Location** | Distributor.sol:70-92 |
| **Severity** | Low |
| **Resolution** | Fixed. Added `IToken(olas).approve(lock, 0)` in the failure branch to reset the dangling approval. |

**Code:**
```solidity
Expand All @@ -179,12 +183,13 @@ function _increaseLock(uint256 olasAmount) internal returns (uint256 remainder)

---

### L-5: `StakingTokenLocked` declares `maxNumInactivityPeriods` but never implements eviction
### L-5: `StakingTokenLocked` declares `maxNumInactivityPeriods` but never implements eviction — by design

| | |
|---|---|
| **Location** | StakingTokenLocked.sol:246 |
| **Severity** | Low |
| **Resolution** | By design. Kept for backward compatibility with the Autonolas staking interface. StakingManager controls all services in the LST model. |

**Problem**: The variable `maxNumInactivityPeriods` is declared (line 246) with comment "Max number of accumulated inactivity periods after which the service is evicted", but `checkpoint()` does not implement eviction logic. Inactive services cannot be force-evicted.

Expand All @@ -194,12 +199,13 @@ function _increaseLock(uint256 olasAmount) internal returns (uint256 remainder)

---

### INFO-1: `setCuratingAgents` computes `stakingHash` inside loop — gas waste
### INFO-1: `setCuratingAgents` computes `stakingHash` inside loop — gas waste — FIXED

| | |
|---|---|
| **Location** | ExternalStakingDistributor.sol:944 |
| **Severity** | Informational |
| **Resolution** | Fixed. Moved `stakingHash` computation before the loop. |

```solidity
for (uint256 i = 0; i < numAgents; ++i) {
Expand All @@ -220,12 +226,13 @@ for (uint256 i = 0; i < numAgents; ++i) {

---

### INFO-2: `unstakeAndWithdraw` checks `stakingProxy != address(0) && serviceId > 0` but these are not validated at entry
### INFO-2: `unstakeAndWithdraw` checks `stakingProxy != address(0) && serviceId > 0` but these are not validated at entry — FIXED

| | |
|---|---|
| **Location** | ExternalStakingDistributor.sol:684-705 |
| **Severity** | Informational |
| **Resolution** | Fixed. Added early validation `if (stakingProxy == address(0) || serviceId == 0) revert ZeroValue()` at function entry, removed the redundant conditional guard. |

The function first calls `IStaking(stakingProxy).availableRewards()` and `getStakingState(serviceId)` (lines 685-688), which will revert if `stakingProxy == address(0)`. Then at line 705, it checks `if (stakingProxy != address(0) && serviceId > 0)` before the unstake block.

Expand All @@ -235,12 +242,13 @@ The conditional at line 705 is effectively dead code — the function always rev

---

### INFO-3: `ExternalStakingDistributor.claim()` is permissionless — timing of reward claims uncontrollable
### INFO-3: `ExternalStakingDistributor.claim()` is permissionless — timing of reward claims uncontrollable — by design

| | |
|---|---|
| **Location** | ExternalStakingDistributor.sol:1030-1071 |
| **Severity** | Informational |
| **Resolution** | By design. Permissionless claiming cannot cause fund loss. |

Any external caller can trigger `claim()` for any staking proxy and service ID. Rewards are distributed to correct recipients (curating agent, collector, protocol), but the timing cannot be controlled by stakeholders.

Expand All @@ -254,17 +262,19 @@ Any external caller can trigger `claim()` for any staking proxy and service ID.
|---|---|
| **Location** | Depository.sol:763-823 |
| **Severity** | Informational |
| **Resolution** | By design. Safe permissionless cleanup. |

Anyone can call `unstakeRetired()` and trigger unstaking of retired models. This is safe (requires `msg.value` for bridge fees, flows through legitimate deposit processors) and appears intentional for permissionless cleanup.

---

### INFO-5: `LzOracle._lzReceive` trusts LayerZero Read responses without independent verification
### INFO-5: `LzOracle._lzReceive` trusts LayerZero Read responses without independent verification — by design

| | |
|---|---|
| **Location** | LzOracle.sol:152-210 |
| **Severity** | Informational |
| **Resolution** | By design. Expected trust model for LayerZero Read. |

The function trusts decoded response data from LZ Read for `bytecodeHash`, `isEnabled`, and `availableRewards`. This is the expected trust model for LayerZero Read, but a compromised DVN set could feed false staking parameters.

Expand Down Expand Up @@ -355,11 +365,11 @@ The function trusts decoded response data from LZ Read for `bytecodeHash`, `isEn
| audit7 Critical: Incorrect mapServiceIdCuratingAgents | Fixed |
| audit7 Critical: abi.encodePacked(address(0)) | Fixed |
| audit7 Medium: changeRewardFactors() timing | Fixed |
| audit8 INFO-1: Missing ETH forwarding in Treasury | Unchangedsee L-3 for related pattern in DefaultDepositProcessorL1 |
| audit8 INFO-2: Dangling OLAS approval in Distributor | **Confirmed still present — now L-4** |
| audit8 INFO-1: Missing ETH forwarding in Treasury | **Fixed**`requestToWithdraw` now forwards `msg.value` correctly |
| audit8 INFO-2: Dangling OLAS approval in Distributor | **Fixed** — approval reset to 0 on failure |
| audit8 INFO-3: ERC6909 withdrawal tokens transferable | Unchanged — by design |
| audit8 INFO-4: Permissionless trigger functions | `unstakeAndWithdraw` now permissionless for evicted/zero-reward — by design |
| audit8 Analysis item 1: `reStake` uses dead mapping | **Upgraded to L-1** — actively harmful after refactor |
| audit8 Analysis item 1: `reStake` uses dead mapping | **Fixed** — now uses `mapServiceIdCuratingAgents[serviceId]` |

---

Expand Down
2 changes: 1 addition & 1 deletion contracts/l1/Depository.sol
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ contract Depository is Implementation {
if (refund > 0) {
// If the call fails, ignore to avoid the attack that would prevent this function from executing
// solhint-disable-next-line avoid-low-level-calls
(bool success,) = msg.sender.call{value: refund}("");
(bool success,) = sender.call{value: refund}("");

emit ValueRefunded(sender, refund, success);
}
Expand Down
3 changes: 3 additions & 0 deletions contracts/l1/Distributor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ contract Distributor is Implementation {

emit Locked(msg.sender, olasAmount, lockAmount, remainder);
} else {
// Reset dangling approval
IToken(olas).approve(lock, 0);

// lock amount is not locked, letting all olas amount be transferred to stOLAS
remainder = olasAmount;
}
Expand Down
Loading
Loading