diff --git a/AGENTS.md b/AGENTS.md index 571c96eb..09831efb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,9 +1,93 @@ # AGENTS.md -This file provides guidance to Codex when working with code in this repository. +This file provides guidance to coding agents working in this repository. -## Documentation Conventions +## Project Overview +Automated Redemption Manager (ARM) by Origin Protocol. Solidity smart contracts that manage swapping and liquidity provision for LST/LRT pairs using a dual-pricing AMM model with async withdrawals. + +**ARM Contracts:** +- **LidoARM** - stETH/WETH on Ethereum +- **EtherFiARM** - eETH/WETH on Ethereum +- **EthenaARM** - sUSDe/USDe on Ethereum +- **OriginARM** - OS/wS on Sonic, OETH/WETH on Ethereum + +## Build & Test Commands + +```bash +make install # foundryup + soldeer + pnpm +make # forge fmt && forge build +make test # All tests except Fuzzer, with --fail-fast -vvv +make test-c-TestName # Run specific test contract +make test-f-funcName # Run specific test function +make test-unit # Unit tests only (test/unit/**) +make test-fork # Fork tests only (test/fork/**) +make test-smoke # Smoke tests only (test/smoke/**) +make test-invariants # All invariant/fuzz tests +make gas # Gas report +make coverage # LCOV coverage report +``` + +Linting: `forge fmt --check` (Solidity), `pnpm lint` (JS), `pnpm prettier:check` (JS) + +## Environment Setup + +Copy `.env.example` to `.env` and set `PROVIDER_URL` (Ethereum RPC) and `SONIC_URL` (Sonic RPC). Fork tests require these RPC endpoints. + +## Architecture + +### Contract Hierarchy + +`AbstractARM.sol` is the core (~1000 LOC). It implements: +- Uniswap V2 Router compatible swap interface +- ERC-4626-like LP interface (deposit/requestRedeem/claimRedeem with async 10-min claim delay) +- Dual pricing: `traderate0` (buy), `traderate1` (sell), `crossPrice` (anchor). All scaled to 1e36 +- Withdrawal queue with FIFO processing +- Performance fee collection +- Lending market allocation (deposit excess liquidity, withdraw on demand) + +Concrete implementations (`LidoARM.sol`, `EtherFiARM.sol`, `EthenaARM.sol`, `OriginARM.sol`) override `_externalWithdrawQueue()` and implement protocol-specific withdrawal/claim logic. + +### Market Adapters (`src/contracts/markets/`) + +`Abstract4626MarketWrapper.sol` wraps ERC-4626 lending markets (Morpho, Silo) so the ARM can deposit idle liquidity. Concrete: `MorphoMarket.sol`, `SiloMarket.sol`. + +### Proxy Pattern + +All ARMs are deployed behind `Proxy.sol` (EIP-1967) for upgradeability. + +### Access Control + +`OwnableOperable.sol` provides owner + operator roles. Owner can set prices, manage markets, upgrade. Operator can execute operational tasks. + +### Off-Chain Automation (`src/js/`) + +- **`src/js/tasks/actions/`** - Talos scheduled actions. +- **`src/js/tasks/`** - Hardhat tasks for admin and operational jobs. + +**Scheduled actions (Talos):** the runner's cron/manual actions live in `src/js/tasks/actions/*.ts`, are scheduled in `migrations/seed_schedules.sql`, and are catalogued in `docs/ACTIONS.md`. When you add, remove, or change the behaviour of a scheduled action, update `docs/ACTIONS.md` in the same change. + +## Deployment + +Deployment scripts are in `script/deploy/` organized by chain (`mainnet/`, `sonic/`). `DeployManager.sol` orchestrates execution based on chain ID. + +```bash +make deploy # Mainnet with verification +make deploy-sonic # Sonic chain +make simulate-deploys # Dry run mainnet +make simulate-sonic-deploys # Dry run Sonic +``` + +## Key Conventions + +- Solidity 0.8.23, optimizer enabled (200 runs) +- Prices are always scaled to 1e36 (`PRICE_SCALE`) +- Fee scale is 10,000 = 100% (`FEE_SCALE`) +- `token0` = swap input (bought by ARM), `token1` = swap output (sold by ARM) +- `baseAsset` = the asset being redeemed (e.g., stETH), `liquidityAsset` = the LP/quote asset (e.g., WETH) +- Test base class: `test/Base.sol` with standard accounts (alice, bob, charlie) and shared setup +- Dependencies managed by Soldeer (not npm) for Solidity libs +- Prefer flat structure with early returns over deeply nested if/else blocks - When writing NatSpec for scaled or non-obvious numeric parameters, include concrete examples. For example: `10,000 = 100% fee`, `500 = 5% fee`, `1e18 = 100% buffer`, `0.1e18 = 10% buffer`. - When adding custom errors under `src/contracts`, include the 4-byte selector in an inline comment next to diff --git a/CLAUDE.md b/CLAUDE.md deleted file mode 100644 index c4b3fe54..00000000 --- a/CLAUDE.md +++ /dev/null @@ -1,92 +0,0 @@ -# CLAUDE.md - -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. - -## Project Overview - -Automated Redemption Manager (ARM) by Origin Protocol. Solidity smart contracts that manage swapping and liquidity provision for LST/LRT pairs using a dual-pricing AMM model with async withdrawals. - -**ARM Contracts:** -- **LidoARM** - stETH/WETH on Ethereum -- **EtherFiARM** - eETH/WETH on Ethereum -- **EthenaARM** - sUSDe/USDe on Ethereum -- **OriginARM** - OS/wS on Sonic, OETH/WETH on Ethereum - -## Build & Test Commands - -```bash -make install # foundryup + soldeer + pnpm -make # forge fmt && forge build -make test # All tests except Fuzzer, with --fail-fast -vvv -make test-c-TestName # Run specific test contract -make test-f-funcName # Run specific test function -make test-unit # Unit tests only (test/unit/**) -make test-fork # Fork tests only (test/fork/**) -make test-smoke # Smoke tests only (test/smoke/**) -make test-invariants # All invariant/fuzz tests -make gas # Gas report -make coverage # LCOV coverage report -``` - -Linting: `forge fmt --check` (Solidity), `pnpm lint` (JS), `pnpm prettier:check` (JS) - -## Environment Setup - -Copy `.env.example` to `.env` and set `PROVIDER_URL` (Ethereum RPC) and `SONIC_URL` (Sonic RPC). Fork tests require these RPC endpoints. - -## Architecture - -### Contract Hierarchy - -`AbstractARM.sol` is the core (~1000 LOC). It implements: -- Uniswap V2 Router compatible swap interface -- ERC-4626-like LP interface (deposit/requestRedeem/claimRedeem with async 10-min claim delay) -- Dual pricing: `traderate0` (buy), `traderate1` (sell), `crossPrice` (anchor). All scaled to 1e36 -- Withdrawal queue with FIFO processing -- Performance fee collection -- Lending market allocation (deposit excess liquidity, withdraw on demand) - -Concrete implementations (`LidoARM.sol`, `EtherFiARM.sol`, `EthenaARM.sol`, `OriginARM.sol`) override `_externalWithdrawQueue()` and implement protocol-specific withdrawal/claim logic. - -### Market Adapters (`src/contracts/markets/`) - -`Abstract4626MarketWrapper.sol` wraps ERC-4626 lending markets (Morpho, Silo) so the ARM can deposit idle liquidity. Concrete: `MorphoMarket.sol`, `SiloMarket.sol`. - -### Proxy Pattern - -All ARMs are deployed behind `Proxy.sol` (EIP-1967) for upgradeability. - -### Access Control - -`OwnableOperable.sol` provides owner + operator roles. Owner can set prices, manage markets, upgrade. Operator can execute operational tasks. - -### Off-Chain Automation (`src/js/`) - -- **`src/js/tasks/actions/`** - Talos scheduled actions. -- **`src/js/tasks/`** - Hardhat tasks for admin and operational jobs. - -**Scheduled actions (Talos):** the runner's cron/manual actions live in `src/js/tasks/actions/*.ts`, are scheduled in `migrations/seed_schedules.sql`, and are catalogued in `docs/ACTIONS.md`. When you add, remove, or change the behaviour of a scheduled action, update `docs/ACTIONS.md` in the same change. - -## Deployment - -Deployment scripts are in `script/deploy/` organized by chain (`mainnet/`, `sonic/`). `DeployManager.sol` orchestrates execution based on chain ID. - -```bash -make deploy # Mainnet with verification -make deploy-sonic # Sonic chain -make simulate-deploys # Dry run mainnet -make simulate-sonic-deploys # Dry run Sonic -``` - -## Key Conventions - -- Solidity 0.8.23, optimizer enabled (200 runs) -- Prices are always scaled to 1e36 (`PRICE_SCALE`) -- Fee scale is 10,000 = 100% (`FEE_SCALE`) -- `token0` = swap input (bought by ARM), `token1` = swap output (sold by ARM) -- `baseAsset` = the asset being redeemed (e.g., stETH), `liquidityAsset` = the LP/quote asset (e.g., WETH) -- Test base class: `test/Base.sol` with standard accounts (alice, bob, charlie) and shared setup -- Dependencies managed by Soldeer (not npm) for Solidity libs -- Prefer flat structure with early returns over deeply nested if/else blocks -- When writing NatSpec for scaled or non-obvious numeric parameters, include concrete examples. - For example: `10,000 = 100% fee`, `500 = 5% fee`, `1e18 = 100% buffer`, `0.1e18 = 10% buffer`. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 120000 index 00000000..47dc3e3d --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +AGENTS.md \ No newline at end of file diff --git a/script/deploy/mainnet/043_UpgradeARMsPauseRolesScript.s.sol b/script/deploy/mainnet/043_UpgradeARMsPauseRolesScript.s.sol new file mode 100644 index 00000000..c3ab5e44 --- /dev/null +++ b/script/deploy/mainnet/043_UpgradeARMsPauseRolesScript.s.sol @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.36; + +// Contracts +import {Proxy} from "contracts/Proxy.sol"; +import {Mainnet} from "contracts/utils/Addresses.sol"; +import {AbstractARM} from "contracts/AbstractARM.sol"; +import {MultiAssetARM} from "contracts/MultiAssetARM.sol"; + +// Deployment +import {AbstractDeployScript} from "script/deploy/helpers/AbstractDeployScript.s.sol"; + +/// @title Split the ARM pause and unpause roles +/// @notice Upgrades the WETH and USDC ARMs to the AbstractARM implementation that separates pausing +/// from unpausing, then wires the two new roles: +/// - `guardian` = 2/8 multisig, which hosts the threat-detection module. Can pause only. +/// - `adminMultisig` = 5/8 multisig. Can pause and unpause. +/// `owner` keeps doing upgrades and stays a valid caller on both, so governance is always a +/// fallback but never on the fast path. The result is that no single 2/8 key can both +/// re-open a paused ARM and change its code. +/// @dev Scope. Only the ARMs that can safely receive a current-source implementation are included: +/// +/// - LIDO_ARM / ETHER_FI_ARM: their current source exceeds the EIP-170 runtime limit, so no new +/// implementation can be deployed for them at all. +/// - ETHENA_ARM: EXCLUDED because the deployed implementation's storage layout does not match +/// the current source. The deployed AbstractARM still carries the `_deprecatedTraderate0/1`, +/// `_deprecatedCrossPrice`, `_deprecatedWithdrawsQueued/Claimed` and +/// `_deprecatedLastAvailableAssets` placeholders, so on-chain `feeCollector` sits at slot 57, +/// `activeMarket` at 59 and `armBuffer` at 61. The current source places them at 59, 53 and 55. +/// Upgrading would therefore corrupt live state. This predates this change; script 034 only +/// avoids it because its idempotency check short-circuits before upgrading. +/// - OETH_ARM (legacy, no pause) and ETH_ARM (unused, holds only the dead-shares seed). +/// +/// Both ARMs in scope are owned by a multisig directly, so no governance proposal is needed. +contract $043_UpgradeARMsPauseRolesScript is AbstractDeployScript("043_UpgradeARMsPauseRolesScript") { + MultiAssetARM public wethARMImpl; + MultiAssetARM public usdcARMImpl; + + function _execute() internal override { + uint256 claimDelay = 10 minutes; + + // Constructor args are unchanged from the scripts that deployed the current implementations + // (038 for WETH, 039 for USDC) so the pause roles are the only behavioural change. + + wethARMImpl = new MultiAssetARM({ + _liquidityAsset: Mainnet.WETH, _claimDelay: claimDelay, _minSharesToRedeem: 1e7, _allocateThreshold: 1 ether + }); + _recordDeployment("WETH_ARM_IMPL", address(wethARMImpl)); + + usdcARMImpl = new MultiAssetARM({ + _liquidityAsset: Mainnet.USDC, _claimDelay: claimDelay, _minSharesToRedeem: 1e6, _allocateThreshold: 100e6 + }); + _recordDeployment("USDC_ARM_IMPL", address(usdcARMImpl)); + } + + /// @notice Both ARMs are owned by a multisig directly, so we simulate their upgrade with a prank. + /// On real deployment the multisig executes upgradeTo + setPauseRoles as a single batched + /// Safe transaction. Batching matters: between the two calls the role slots are still + /// address(0), so unpause would briefly narrow back to owner-only. + function _fork() internal override { + _upgradeAndSetRoles("WETH_ARM", "WETH_ARM_IMPL"); + _upgradeAndSetRoles("USDC_ARM", "USDC_ARM_IMPL"); + + // Behavioural assertions (2/8 can pause but not unpause, 5/8 can unpause) live in the smoke + // tests. They must not run here: _fork() executes inside the test's setUp(), so anything + // that mutates ARM state or arms a vm.expectRevert would leak into the test body. + _assertRolesSet("WETH_ARM"); + _assertRolesSet("USDC_ARM"); + } + + function _upgradeAndSetRoles(string memory proxyName, string memory implementationName) internal { + Proxy armProxy = Proxy(payable(resolver.resolve(proxyName))); + address armImpl = resolver.resolve(implementationName); + + // Idempotent: the deployment runner can replay pending multisig actions on forks. + if (armProxy.implementation() == armImpl) return; + + // Guard the storage assumption this upgrade depends on. The new `guardian` and + // `adminMultisig` occupy slots 62 and 63, taken from the AbstractARM gap. If the deployed + // layout ever diverges from the current source, those slots hold live data and this upgrade + // would corrupt it, so fail loudly instead. This is exactly the condition that rules + // ETHENA_ARM out of scope. + require(vm.load(address(armProxy), bytes32(uint256(62))) == 0, "slot 62 not free"); + require(vm.load(address(armProxy), bytes32(uint256(63))) == 0, "slot 63 not free"); + + // Prank the live owner rather than a hardcoded Safe. Ownership of these proxies has moved + // since they were deployed, so the deploy scripts are not the source of truth for it. + vm.startPrank(armProxy.owner()); + armProxy.upgradeTo(armImpl); + AbstractARM(payable(address(armProxy))).setPauseRoles(Mainnet.MULTISIG_2_OF_8, Mainnet.MULTISIG_5_OF_8); + vm.stopPrank(); + } + + /// @dev Confirm the roles landed. Read-only: this must not mutate ARM state, because the smoke + /// tests run against whatever state _fork() leaves behind. + function _assertRolesSet(string memory proxyName) internal view { + AbstractARM arm = AbstractARM(payable(resolver.resolve(proxyName))); + + require(arm.guardian() == Mainnet.MULTISIG_2_OF_8, "guardian not set"); + require(arm.adminMultisig() == Mainnet.MULTISIG_5_OF_8, "adminMultisig not set"); + } +} diff --git a/src/abis/MultiAssetARM.json b/src/abis/MultiAssetARM.json index cdba63b9..521ad398 100644 --- a/src/abis/MultiAssetARM.json +++ b/src/abis/MultiAssetARM.json @@ -99,6 +99,19 @@ "outputs": [], "stateMutability": "nonpayable" }, + { + "type": "function", + "name": "adminMultisig", + "inputs": [], + "outputs": [ + { + "name": "", + "type": "address", + "internalType": "address" + } + ], + "stateMutability": "view" + }, { "type": "function", "name": "allocate", @@ -557,6 +570,19 @@ ], "stateMutability": "view" }, + { + "type": "function", + "name": "guardian", + "inputs": [], + "outputs": [ + { + "name": "", + "type": "address", + "internalType": "address" + } + ], + "stateMutability": "view" + }, { "type": "function", "name": "initialize", @@ -932,6 +958,24 @@ "outputs": [], "stateMutability": "nonpayable" }, + { + "type": "function", + "name": "setPauseRoles", + "inputs": [ + { + "name": "_guardian", + "type": "address", + "internalType": "address" + }, + { + "name": "_adminMultisig", + "type": "address", + "internalType": "address" + } + ], + "outputs": [], + "stateMutability": "nonpayable" + }, { "type": "function", "name": "setPrices", @@ -1368,6 +1412,19 @@ ], "anonymous": false }, + { + "type": "event", + "name": "AdminMultisigChanged", + "inputs": [ + { + "name": "newAdminMultisig", + "type": "address", + "indexed": false, + "internalType": "address" + } + ], + "anonymous": false + }, { "type": "event", "name": "Allocated", @@ -1563,6 +1620,19 @@ ], "anonymous": false }, + { + "type": "event", + "name": "GuardianChanged", + "inputs": [ + { + "name": "newGuardian", + "type": "address", + "indexed": false, + "internalType": "address" + } + ], + "anonymous": false + }, { "type": "event", "name": "Initialized", @@ -2026,6 +2096,16 @@ "name": "OnlyOwner", "inputs": [] }, + { + "type": "error", + "name": "OnlyPauser", + "inputs": [] + }, + { + "type": "error", + "name": "OnlyUnpauser", + "inputs": [] + }, { "type": "error", "name": "QueuePendingLiquidity", diff --git a/src/contracts/AbstractARM.sol b/src/contracts/AbstractARM.sol index a0d3675e..36d8dad2 100644 --- a/src/contracts/AbstractARM.sol +++ b/src/contracts/AbstractARM.sol @@ -135,7 +135,16 @@ abstract contract AbstractARM is OwnableOperable, ERC20Upgradeable, ReentrancyGu /// @notice Maximum liquidity assets reserved for outstanding LP withdrawal requests. uint128 public reservedWithdrawLiquidity; - uint256[50] private _gap; + /// @notice Account that can pause but never unpause. Held by the 2/8 Guardian multisig, + /// which hosts the threat-detection module that trips the pause automatically. + address public guardian; + /// @notice Account that can pause and unpause. Held by the 5/8 Admin multisig. + /// @dev Named `adminMultisig` rather than `admin` on purpose. `Proxy` inherits `Ownable` and + /// declares its own `admin()` returning the proxy owner, so a variable named `admin` would + /// generate a getter the proxy permanently shadows and it could never be read through the proxy. + address public adminMultisig; + + uint256[48] private _gap; //////////////////////////////////////////////////// /// Errors @@ -162,6 +171,8 @@ abstract contract AbstractARM is OwnableOperable, ERC20Upgradeable, ReentrancyGu error MarketActive(); // 0xaeb31949 error InvalidARMBuffer(); // 0x06f77af9 error ContractPaused(); // 0xab35696f + error OnlyPauser(); // 0x75df51dc + error OnlyUnpauser(); // 0x794821ff error Insolvent(); // 0xfc220038 error ZeroShares(); // 0x9811e0c7 error ClaimDelayNotMet(); // 0x4a1eec28 @@ -214,6 +225,8 @@ abstract contract AbstractARM is OwnableOperable, ERC20Upgradeable, ReentrancyGu event Allocated(address indexed market, int256 targetLiquidityDelta, int256 actualLiquidityDelta); event Paused(address indexed account); event Unpaused(address indexed account); + event GuardianChanged(address newGuardian); + event AdminMultisigChanged(address newAdminMultisig); //////////////////////////////////////////////////// /// Modifiers @@ -224,6 +237,29 @@ abstract contract AbstractARM is OwnableOperable, ERC20Upgradeable, ReentrancyGu _; } + /// @dev Single authorization check for both `pause()` and `unpause()`, dispatching on the + /// selector of the call it guards. Pausing is the safe direction, so its caller list is + /// deliberately wide: the owner, operator, guardian or adminMultisig can all trip the circuit + /// breaker. Unpausing narrows to the owner or adminMultisig, so that no single hot key or 2/8 + /// key can both re-open a paused ARM and, where it is also the owner, change its code. + /// @dev Only ever apply this to the external `pause()` and `unpause()` entry points. It reads + /// `msg.sig`, so on any other function it would silently fall through to the unpause rules. + modifier onlyPauserOrUnpauser() { + _checkPauserOrUnpauser(); + _; + } + + function _checkPauserOrUnpauser() internal view { + bool isPause = msg.sig == this.pause.selector; + if ( + msg.sender != _owner() && msg.sender != adminMultisig + && (!isPause || (msg.sender != operator && msg.sender != guardian)) + ) { + if (isPause) revert OnlyPauser(); + revert OnlyUnpauser(); + } + } + //////////////////////////////////////////////////// /// Constructor //////////////////////////////////////////////////// @@ -1223,18 +1259,34 @@ abstract contract AbstractARM is OwnableOperable, ERC20Upgradeable, ReentrancyGu /// Admin Functions //////////////////////////////////////////////////// - /// @notice Pause user-facing ARM actions. - function pause() external onlyOperatorOrOwner { + /// @notice Pause user-facing ARM actions. Callable by the owner, operator, guardian or + /// adminMultisig. + function pause() external onlyPauserOrUnpauser { paused = true; emit Paused(msg.sender); } - /// @notice Unpause user-facing ARM actions. - function unpause() external onlyOwner { + /// @notice Unpause user-facing ARM actions. Callable by the owner or adminMultisig only. + function unpause() external onlyPauserOrUnpauser { paused = false; emit Unpaused(msg.sender); } + /// @notice Set the accounts that can pause and unpause. + /// @dev Both roles are set in one call so that an upgrade and its role configuration fit in a + /// single governance action. Setting them separately would leave a window where the slots are + /// still address(0) and unpause has silently narrowed to owner-only. + /// @param _guardian The 2/8 Guardian multisig, which can pause but not unpause. + /// address(0) disables the role. + /// @param _adminMultisig The 5/8 Admin multisig, which can pause and unpause. + /// address(0) disables the role. + function setPauseRoles(address _guardian, address _adminMultisig) external onlyOwner { + guardian = _guardian; + adminMultisig = _adminMultisig; + emit GuardianChanged(_guardian); + emit AdminMultisigChanged(_adminMultisig); + } + /// @notice Set the CapManager contract. /// @param _capManager CapManager contract address, or address(0) to disable caps. function setCapManager(address _capManager) external onlyOwner { diff --git a/test/smoke/PaxosARMSmokeTest.t.sol b/test/smoke/PaxosARMSmokeTest.t.sol index fe8173ba..153d3d00 100644 --- a/test/smoke/PaxosARMSmokeTest.t.sol +++ b/test/smoke/PaxosARMSmokeTest.t.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.36; import {AbstractSmokeTest} from "./AbstractSmokeTest.sol"; import {IERC20} from "contracts/Interfaces.sol"; +import {AbstractARM} from "contracts/AbstractARM.sol"; import {MultiAssetARM} from "contracts/MultiAssetARM.sol"; import {PaxosAssetAdapter} from "contracts/adapters/PaxosAssetAdapter.sol"; import {CapManager} from "contracts/CapManager.sol"; @@ -234,6 +235,53 @@ contract Fork_PaxosARM_Smoke_Test is AbstractSmokeTest { usdcARM.setOwner(RANDOM_ADDRESS); } + ////////////////////////////////////////////////////// + /// --- pause roles + ////////////////////////////////////////////////////// + + /// @notice The 043 upgrade wires the 2/8 as guardian and the 5/8 as adminMultisig. + function test_PauseRolesConfigured() external view { + assertEq(usdcARM.guardian(), Mainnet.MULTISIG_2_OF_8, "guardian is the 2/8"); + assertEq(usdcARM.adminMultisig(), Mainnet.MULTISIG_5_OF_8, "adminMultisig is the 5/8"); + } + + /// @notice The 2/8 gets a no-delay pause but must never be able to re-open the ARM. + function test_GuardianCanPauseButNotUnpause() external { + vm.prank(Mainnet.MULTISIG_2_OF_8); + usdcARM.pause(); + assertTrue(usdcARM.paused(), "guardian paused"); + + vm.prank(Mainnet.MULTISIG_2_OF_8); + vm.expectRevert(AbstractARM.OnlyUnpauser.selector); + usdcARM.unpause(); + assertTrue(usdcARM.paused(), "still paused after guardian tried to unpause"); + + vm.prank(Mainnet.MULTISIG_5_OF_8); + usdcARM.unpause(); + assertFalse(usdcARM.paused(), "adminMultisig unpaused"); + } + + /// @notice Storage-layout proof: the roles landed in the gap at slots 62/63 and the live + /// variables bracketing them still read back sane. + function test_StorageLayoutPreservedAcrossUpgrade() external view { + assertEq(usdcARM.feeCollector(), Mainnet.BUYBACK_OPERATOR, "feeCollector (slot 59) intact"); + assertGe(usdcARM.withdrawsQueuedShares(), usdcARM.withdrawsClaimedShares(), "slot 60 queue invariant"); + + assertEq( + uint256(vm.load(address(usdcARM), bytes32(uint256(62)))), + uint256(uint160(Mainnet.MULTISIG_2_OF_8)), + "guardian at slot 62" + ); + assertEq( + uint256(vm.load(address(usdcARM), bytes32(uint256(63)))), + uint256(uint160(Mainnet.MULTISIG_5_OF_8)), + "adminMultisig at slot 63" + ); + + assertGt(usdcARM.totalAssets(), 0, "totalAssets intact"); + assertEq(usdcARM.liquidityAsset(), Mainnet.USDC, "liquidityAsset intact"); + } + /// @dev Assert `expected` appears in the ARM's `getBaseAssets()` list. A membership check /// rather than exact array equality keeps the assertion robust to registration order and /// to additional base assets being registered by future deployments. diff --git a/test/smoke/WETHARMSmokeTest.t.sol b/test/smoke/WETHARMSmokeTest.t.sol index 3a17303d..33d8b053 100644 --- a/test/smoke/WETHARMSmokeTest.t.sol +++ b/test/smoke/WETHARMSmokeTest.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.36; import {AbstractSmokeTest} from "./AbstractSmokeTest.sol"; +import {AbstractARM} from "contracts/AbstractARM.sol"; import {MultiAssetARM} from "contracts/MultiAssetARM.sol"; import {CapManager} from "contracts/CapManager.sol"; import {Proxy} from "contracts/Proxy.sol"; @@ -86,6 +87,80 @@ contract Fork_WETHARM_Smoke_Test is AbstractSmokeTest { assertEq(wethARM.activeMarket(), address(morphoMarket), "active market"); } + ////////////////////////////////////////////////////// + /// --- pause roles + ////////////////////////////////////////////////////// + + /// @notice The 043 upgrade wires the 2/8 as guardian and the 5/8 as adminMultisig. + function test_PauseRolesConfigured() external view { + assertEq(wethARM.guardian(), Mainnet.MULTISIG_2_OF_8, "guardian is the 2/8"); + assertEq(wethARM.adminMultisig(), Mainnet.MULTISIG_5_OF_8, "adminMultisig is the 5/8"); + } + + /// @notice The 2/8 gets a no-delay pause, but must not be able to re-open the ARM. Together with + /// owner staying the upgrade admin, that stops any single 2/8 key from both unpausing and + /// changing the code. + function test_GuardianCanPauseButNotUnpause() external { + vm.prank(Mainnet.MULTISIG_2_OF_8); + wethARM.pause(); + assertTrue(wethARM.paused(), "guardian paused"); + + vm.prank(Mainnet.MULTISIG_2_OF_8); + vm.expectRevert(AbstractARM.OnlyUnpauser.selector); + wethARM.unpause(); + assertTrue(wethARM.paused(), "still paused after guardian tried to unpause"); + + // The 5/8 recovers with no delay and no governance vote. + vm.prank(Mainnet.MULTISIG_5_OF_8); + wethARM.unpause(); + assertFalse(wethARM.paused(), "adminMultisig unpaused"); + } + + /// @notice The Talos relayer is a hot key: it keeps its pause, but never gains unpause. + function test_OperatorCannotUnpause() external { + vm.prank(Mainnet.ARM_TALOS_RELAYER); + wethARM.pause(); + assertTrue(wethARM.paused(), "operator paused"); + + vm.prank(Mainnet.ARM_TALOS_RELAYER); + vm.expectRevert(AbstractARM.OnlyUnpauser.selector); + wethARM.unpause(); + + vm.prank(Mainnet.MULTISIG_5_OF_8); + wethARM.unpause(); + } + + /// @notice Storage-layout proof. `guardian`/`adminMultisig` were taken from the AbstractARM gap + /// at slots 62 and 63. Read the live variables that bracket them and confirm they still + /// hold sane values: a layout shift shows up here first. + function test_StorageLayoutPreservedAcrossUpgrade() external view { + // Slots 59-61, immediately before the new roles. + assertEq(wethARM.feeCollector(), Mainnet.BUYBACK_OPERATOR, "feeCollector (slot 59) intact"); + assertEq( + uint256(vm.load(address(wethARM), bytes32(uint256(59)))), + uint256(uint160(Mainnet.BUYBACK_OPERATOR)), + "slot 59 raw" + ); + assertGe(wethARM.withdrawsQueuedShares(), wethARM.withdrawsClaimedShares(), "slot 60 queue invariant"); + + // The new roles themselves, at slots 62 and 63. + assertEq( + uint256(vm.load(address(wethARM), bytes32(uint256(62)))), + uint256(uint160(Mainnet.MULTISIG_2_OF_8)), + "guardian at slot 62" + ); + assertEq( + uint256(vm.load(address(wethARM), bytes32(uint256(63)))), + uint256(uint160(Mainnet.MULTISIG_5_OF_8)), + "adminMultisig at slot 63" + ); + + // Live accounting still reads back sane, so nothing downstream shifted either. + assertGt(wethARM.totalAssets(), 0, "totalAssets intact"); + assertEq(wethARM.liquidityAsset(), Mainnet.WETH, "liquidityAsset intact"); + assertEq(wethARM.getBaseAssets().length, 4, "base assets intact"); + } + function _assertBaseAssetConfig(address baseAsset, string memory adapterName, bool pegged) internal view { (,,,,,, bool peggedToLiquidityAsset, uint8 baseAssetDecimals, address adapter) = wethARM.baseAssetConfigs(baseAsset); diff --git a/test/unit/MultiAssetARM/concrete/Pause.t.sol b/test/unit/MultiAssetARM/concrete/Pause.t.sol index fa4aa267..4f018bbb 100644 --- a/test/unit/MultiAssetARM/concrete/Pause.t.sol +++ b/test/unit/MultiAssetARM/concrete/Pause.t.sol @@ -7,14 +7,19 @@ import {Unit_MultiAssetARM_Shared_Test} from "../Shared.t.sol"; // Contracts import {AbstractARM} from "contracts/AbstractARM.sol"; import {Ownable} from "contracts/Ownable.sol"; -import {OwnableOperable} from "contracts/OwnableOperable.sol"; -/// @notice Coverage for `pause()` (operator or owner) and `unpause()` (owner only). +/// @notice Coverage for `pause()` (owner, operator, guardian or adminMultisig), `unpause()` +/// (owner or adminMultisig only) and `setPauseRoles()` (owner only). /// The downstream `whenNotPaused` reverts on user-facing functions are /// already covered in the per-function test files (Deposit, ClaimRedeem, /// RequestRedeem, Swap*). Here we focus on the access control, the /// `paused` state flip, and the events. contract Unit_MultiAssetARM_Pause_Test is Unit_MultiAssetARM_Shared_Test { + /// @dev The 2/8 Guardian multisig: can pause, must never unpause. + address public guardian = makeAddr("guardian"); + /// @dev The 5/8 Admin multisig: can pause and unpause. + address public adminMultisig = makeAddr("adminMultisig"); + ////////////////////////////////////////////////////// /// --- pause ////////////////////////////////////////////////////// @@ -42,9 +47,43 @@ contract Unit_MultiAssetARM_Pause_Test is Unit_MultiAssetARM_Shared_Test { assertEq(arm.paused(), true, "paused post"); } + function test_Pause_ByGuardian() public { + assertEq(arm.paused(), false, "paused pre"); + + vm.expectEmit(address(arm)); + emit AbstractARM.Paused(guardian); + + vm.prank(guardian); + arm.pause(); + + assertEq(arm.paused(), true, "paused post"); + } + + function test_Pause_ByAdminMultisig() public { + assertEq(arm.paused(), false, "paused pre"); + + vm.expectEmit(address(arm)); + emit AbstractARM.Paused(adminMultisig); + + vm.prank(adminMultisig); + arm.pause(); + + assertEq(arm.paused(), true, "paused post"); + } + function test_Pause_RevertWhen_NotAuthorized() public { vm.prank(alice); - vm.expectRevert(OwnableOperable.OnlyOperatorOrOwner.selector); + vm.expectRevert(AbstractARM.OnlyPauser.selector); + arm.pause(); + } + + /// @notice Clearing a role revokes its pause rights. + function test_Pause_RevertWhen_GuardianCleared() public { + vm.prank(governor); + arm.setPauseRoles(address(0), adminMultisig); + + vm.prank(guardian); + vm.expectRevert(AbstractARM.OnlyPauser.selector); arm.pause(); } @@ -65,14 +104,42 @@ contract Unit_MultiAssetARM_Pause_Test is Unit_MultiAssetARM_Shared_Test { assertEq(arm.paused(), false, "paused post"); } + function test_Unpause_ByAdminMultisig() public { + vm.prank(guardian); + arm.pause(); + assertEq(arm.paused(), true, "paused pre"); + + vm.expectEmit(address(arm)); + emit AbstractARM.Unpaused(adminMultisig); + + vm.prank(adminMultisig); + arm.unpause(); + + assertEq(arm.paused(), false, "paused post"); + } + function test_Unpause_RevertWhen_Operator() public { - // The operator can pause but cannot unpause — that's reserved for the owner. + // The operator is a hot key. It can trip the pause but must never lift it. vm.prank(operator); arm.pause(); vm.prank(operator); - vm.expectRevert(Ownable.OnlyOwner.selector); + vm.expectRevert(AbstractARM.OnlyUnpauser.selector); arm.unpause(); + + assertEq(arm.paused(), true, "must stay paused"); + } + + /// @notice The whole point of the split: the 2/8 guardian can pause but cannot re-open the ARM. + function test_Unpause_RevertWhen_Guardian() public { + vm.prank(guardian); + arm.pause(); + + vm.prank(guardian); + vm.expectRevert(AbstractARM.OnlyUnpauser.selector); + arm.unpause(); + + assertEq(arm.paused(), true, "must stay paused"); } function test_Unpause_RevertWhen_NotAuthorized() public { @@ -80,10 +147,50 @@ contract Unit_MultiAssetARM_Pause_Test is Unit_MultiAssetARM_Shared_Test { arm.pause(); vm.prank(alice); - vm.expectRevert(Ownable.OnlyOwner.selector); + vm.expectRevert(AbstractARM.OnlyUnpauser.selector); arm.unpause(); } + ////////////////////////////////////////////////////// + /// --- setPauseRoles + ////////////////////////////////////////////////////// + function test_SetPauseRoles_ByOwner() public { + address newGuardian = makeAddr("newGuardian"); + address newAdminMultisig = makeAddr("newAdminMultisig"); + + vm.expectEmit(address(arm)); + emit AbstractARM.GuardianChanged(newGuardian); + vm.expectEmit(address(arm)); + emit AbstractARM.AdminMultisigChanged(newAdminMultisig); + + vm.prank(governor); + arm.setPauseRoles(newGuardian, newAdminMultisig); + + assertEq(arm.guardian(), newGuardian, "guardian"); + assertEq(arm.adminMultisig(), newAdminMultisig, "adminMultisig"); + } + + function test_SetPauseRoles_RevertWhen_NotOwner() public { + vm.prank(alice); + vm.expectRevert(Ownable.OnlyOwner.selector); + arm.setPauseRoles(alice, alice); + } + + /// @notice Neither the operator nor the roles themselves can reassign the roles. + function test_SetPauseRoles_RevertWhen_Operator() public { + vm.prank(operator); + vm.expectRevert(Ownable.OnlyOwner.selector); + arm.setPauseRoles(alice, alice); + + vm.prank(guardian); + vm.expectRevert(Ownable.OnlyOwner.selector); + arm.setPauseRoles(alice, alice); + + vm.prank(adminMultisig); + vm.expectRevert(Ownable.OnlyOwner.selector); + arm.setPauseRoles(alice, alice); + } + /// @notice Pausing an already-paused ARM is a no-op state-wise but still emits the event. function test_Pause_WhenAlreadyPaused() public { vm.prank(governor); @@ -171,9 +278,13 @@ contract Unit_MultiAssetARM_Pause_Test is Unit_MultiAssetARM_Shared_Test { /// @dev Give alice liquidity for the deposit/redeem gating tests. Approvals are set by the shared harness. /// Caps are disabled so deposits are only gated by the pause state under test. + /// The guardian and adminMultisig roles are wired here; deployments set them at upgrade time. function setUp() public virtual override { super.setUp(); desactiveCapManager(); deal(address(liquidity), alice, 1_000 * DEFAULT_AMOUNT()); + + vm.prank(governor); + arm.setPauseRoles(guardian, adminMultisig); } }