diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..58528de --- /dev/null +++ b/CLAUDE.md @@ -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 | diff --git a/audits/audit8/README.md b/audits/audit8/README.md index df0d2d2..f385362 100644 --- a/audits/audit8/README.md +++ b/audits/audit8/README.md @@ -88,25 +88,27 @@ 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 | @@ -114,7 +116,7 @@ The protocol demonstrates mature defensive coding patterns: | **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 | diff --git a/audits/audit9/README.md b/audits/audit9/README.md index de62473..3d2f5b7 100644 --- a/audits/audit9/README.md +++ b/audits/audit9/README.md @@ -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. @@ -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 @@ -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 @@ -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 @@ -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 @@ -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. @@ -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) { @@ -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. @@ -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. @@ -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. @@ -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 | Unchanged — see 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]` | --- diff --git a/contracts/l1/Depository.sol b/contracts/l1/Depository.sol index d28f941..929bdf9 100644 --- a/contracts/l1/Depository.sol +++ b/contracts/l1/Depository.sol @@ -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); } diff --git a/contracts/l1/Distributor.sol b/contracts/l1/Distributor.sol index 7d3886b..247c0dc 100644 --- a/contracts/l1/Distributor.sol +++ b/contracts/l1/Distributor.sol @@ -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; } diff --git a/contracts/l1/Treasury.sol b/contracts/l1/Treasury.sol index 647e2e6..f09694c 100644 --- a/contracts/l1/Treasury.sol +++ b/contracts/l1/Treasury.sol @@ -74,6 +74,9 @@ error Overflow(uint256 provided, uint256 max); /// @dev Caught reentrancy violation. error ReentrancyGuard(); +/// @dev Wrong length of arrays. +error WrongArrayLength(); + /// @title Treasury - Smart contract for treasury contract Treasury is Implementation, ERC6909 { event WithdrawDelayUpdates(uint256 withdrawDelay); @@ -136,6 +139,76 @@ contract Treasury is Implementation, ERC6909 { emit WithdrawDelayUpdates(newWithdrawDelay); } + /// @dev Processes unstake requests for both external and LST staking proxies. + /// @param withdrawDiff Amount to unstake. + /// @param chainIds 2D set of chain Ids with [external staking distributors] and [LST staking proxies]. + /// @param externalAmounts Set of corresponding amounts for staked externals. + /// @param stakingProxies Set of staking proxies corresponding to each chain Id in [LST staking proxies] set. + /// @param bridgePayloads 2D set of bridge payloads corresponding to each chain Id. + /// @param values 2D set of value amounts for each bridge interaction, if applicable. + function _processUnstakes( + uint256 withdrawDiff, + uint256[][] memory chainIds, + uint256[] memory externalAmounts, + address[] memory stakingProxies, + bytes[][] memory bridgePayloads, + uint256[][] memory values + ) internal { + // Track remaining msg.value for bridge interactions + uint256 remainingValue = msg.value; + + // Check if unstake from externals is requested + if (chainIds[0].length > 0) { + // Check that external amounts and values arrays match + if (externalAmounts.length != values[0].length) { + revert WrongArrayLength(); + } + + uint256 totalExternalAmount; + uint256 externalValue; + // Traverse external amounts and values + for (uint256 i = 0; i < externalAmounts.length; ++i) { + totalExternalAmount += externalAmounts[i]; + externalValue += values[0][i]; + } + + // Check that msg.value covers external bridge values + if (remainingValue < externalValue) { + revert Overflow(externalValue, remainingValue); + } + + // Deduct external value from remaining + remainingValue -= externalValue; + + // Check for overflow + if (totalExternalAmount > withdrawDiff) { + revert Overflow(totalExternalAmount, withdrawDiff); + } + + // Update withdraw amount + if (totalExternalAmount > 0) { + withdrawDiff -= totalExternalAmount; + } + + // First, unstake from external proxies + IDepository(depository).unstakeExternal{value: externalValue}( + chainIds[0], externalAmounts, bridgePayloads[0], values[0], msg.sender + ); + } + + // Check for withdrawDiff + if (withdrawDiff > 0) { + // Second, unstake from LST staking proxies, if still required + // Check for remainingValue corresponding to sum(values[1]) is handled in unstake function itself + IDepository(depository).unstake{value: remainingValue}( + withdrawDiff, chainIds[1], stakingProxies, bridgePayloads[1], values[1], msg.sender + ); + } else if (remainingValue > 0) { + // No LST unstaking needed, reject leftover ETH + revert Overflow(remainingValue, 0); + } + } + /// @dev Requests withdraw of OLAS in exchange of provided stOLAS. /// @notice Vault reserves are used first. If there is a lack of OLAS reserves, the backup amount is requested /// to be unstaked from other models. @@ -196,36 +269,12 @@ contract Treasury is Implementation, ERC6909 { // If withdraw amount is bigger than the current one, need to unstake if (stakedBalanceBefore > stakedBalanceAfter) { - uint256 withdrawDiff = stakedBalanceBefore - stakedBalanceAfter; - - // Check if unstake from externals is requested - if (chainIds[0].length > 0) { - uint256 totalExternalAmount; - // Traverse external amounts, if any - for (uint256 i = 0; i < externalAmounts.length; ++i) { - totalExternalAmount += externalAmounts[i]; - } - - // Check for overflow - if (totalExternalAmount > withdrawDiff) { - revert Overflow(totalExternalAmount, withdrawDiff); - } - - // Update withdraw amount - if (totalExternalAmount > 0) { - withdrawDiff -= totalExternalAmount; - } - - // First, unstake from external proxies - IDepository(depository) - .unstakeExternal(chainIds[0], externalAmounts, bridgePayloads[0], values[0], msg.sender); - } - - // Check for withdrawDiff - if (withdrawDiff > 0) { - // Second, unstake from LST staking proxies, if still required - IDepository(depository) - .unstake(withdrawDiff, chainIds[1], stakingProxies, bridgePayloads[1], values[1], msg.sender); + _processUnstakes(stakedBalanceBefore - stakedBalanceAfter, chainIds, externalAmounts, stakingProxies, + bridgePayloads, values); + } else { + // No unstaking needed, reject any ETH sent + if (msg.value > 0) { + revert Overflow(msg.value, 0); } } diff --git a/contracts/l1/bridging/DefaultDepositProcessorL1.sol b/contracts/l1/bridging/DefaultDepositProcessorL1.sol index 113665b..034e1a0 100644 --- a/contracts/l1/bridging/DefaultDepositProcessorL1.sol +++ b/contracts/l1/bridging/DefaultDepositProcessorL1.sol @@ -21,7 +21,6 @@ abstract contract DefaultDepositProcessorL1 is IBridgeErrors { event MessagePosted(uint256 indexed sequence, address indexed target, uint256 amount, bytes32 indexed batchHash); event L2StakerUpdated(address indexed l2StakingProcessor); event LeftoversRefunded(address indexed sender, uint256 leftovers, bool success); - event Drained(address indexed sender, address indexed receiver, uint256 amount); // Stake operation bytes32 public constant STAKE = 0x1bcc0f4c3fad314e585165815f94ecca9b96690a26d6417d7876448a9a867a69; @@ -151,29 +150,6 @@ abstract contract DefaultDepositProcessorL1 is IBridgeErrors { _setL2StakingProcessor(l2Processor); } - /// @dev Drains possible stuck values. - function drain() external { - // Get owner address: it cannot be zero because the proxy is already initialized - address localOwner = owner; - - // Get contract value balance - uint256 amount = address(this).balance; - - // Check for zero value - if (amount == 0) { - revert ZeroValue(); - } - - // solhint-disable-next-line avoid-low-level-calls - (bool success,) = localOwner.call{value: amount}(""); - - if (!success) { - revert TransferFailed(address(0), address(this) , localOwner, amount); - } - - emit Drained(msg.sender, localOwner, amount); - } - /// @dev Gets the maximum number of token decimals able to be transferred across the bridge. /// @return Number of supported decimals. function getBridgingDecimals() external pure virtual returns (uint256) { diff --git a/contracts/l2/ExternalStakingDistributor.sol b/contracts/l2/ExternalStakingDistributor.sol index 337e87a..e766582 100644 --- a/contracts/l2/ExternalStakingDistributor.sol +++ b/contracts/l2/ExternalStakingDistributor.sol @@ -681,28 +681,28 @@ contract ExternalStakingDistributor is Implementation, ERC721TokenReceiver { } _locked = 2; - // Get staking proxy available rewards amount - uint256 availableRewards = IStaking(stakingProxy).availableRewards(); + // Check if service unstake is requested + if (stakingProxy != address(0) && serviceId > 0) { + // Get staking proxy available rewards amount + uint256 availableRewards = IStaking(stakingProxy).availableRewards(); - // Check if service is evicted - IStaking.StakingState stakingState = IStaking(stakingProxy).getStakingState(serviceId); + // Check if service is evicted + IStaking.StakingState stakingState = IStaking(stakingProxy).getStakingState(serviceId); - // Check for unstaked state - if (stakingState == IStaking.StakingState.Unstaked) { - revert ZeroValue(); - } + // Check for unstaked state + if (stakingState == IStaking.StakingState.Unstaked) { + revert ZeroValue(); + } - // Check for zero rewards balance and access: whitelisted managing agent or owner - // msg.sender does not matter if rewards are no longer available, or if the service is evicted - if ( - stakingState == IStaking.StakingState.Staked && availableRewards > 0 - && !(mapManagingAgents[msg.sender] || msg.sender == owner) - ) { - revert UnauthorizedAccount(msg.sender); - } + // Check for zero rewards balance and access: whitelisted managing agent or owner + // msg.sender does not matter if rewards are no longer available, or if the service is evicted + if ( + stakingState == IStaking.StakingState.Staked && availableRewards > 0 + && !(mapManagingAgents[msg.sender] || msg.sender == owner) + ) { + revert UnauthorizedAccount(msg.sender); + } - // Check if service unstake is requested - if (stakingProxy != address(0) && serviceId > 0) { // Calculate how many unstakes are needed uint256 minStakingDeposit = IStaking(stakingProxy).minStakingDeposit(); uint256 fullStakingDeposit = minStakingDeposit * (1 + NUM_AGENT_INSTANCES); @@ -800,8 +800,11 @@ contract ExternalStakingDistributor is Implementation, ERC721TokenReceiver { revert ZeroValue(); } - // Check for access: curating agent or managing agent, or owner - if (!(mapCuratingAgents[msg.sender] || mapManagingAgents[msg.sender] || msg.sender == owner)) { + // Check for access: service curating agent, managing agent, or owner + if ( + !(mapServiceIdCuratingAgents[serviceId] == msg.sender || mapManagingAgents[msg.sender] + || msg.sender == owner) + ) { revert UnauthorizedAccount(msg.sender); } @@ -933,6 +936,9 @@ contract ExternalStakingDistributor is Implementation, ERC721TokenReceiver { revert UnauthorizedAccount(msg.sender); } + // Encode staking hash outside of the loop + bytes32 stakingHash = keccak256(abi.encode(stakingGuard, stakingProxy)); + // Traverse curating agents for (uint256 i = 0; i < numAgents; ++i) { // Check for zero address @@ -940,8 +946,6 @@ contract ExternalStakingDistributor is Implementation, ERC721TokenReceiver { revert ZeroAddress(); } - // Encode staking hash - bytes32 stakingHash = keccak256(abi.encode(stakingGuard, stakingProxy)); // Set curating agent status mapStakingGuardHashCuratingAgents[stakingHash][curatingAgents[i]] = statuses[i]; } diff --git a/hardhat.config.js b/hardhat.config.js index 623bc47..912dd7f 100644 --- a/hardhat.config.js +++ b/hardhat.config.js @@ -252,7 +252,7 @@ module.exports = { solidity: { compilers: [ { - version: "0.8.30", + version: "0.8.34", settings: { optimizer: { enabled: true,