From f0aabc7814e6c377e4bc4ed44eb381e56c68a676 Mon Sep 17 00:00:00 2001 From: Shahul Hameed <10547529+shahthepro@users.noreply.github.com> Date: Wed, 12 Aug 2026 17:21:54 +0400 Subject: [PATCH 1/5] Add Admin role to the Vault --- contracts/abi/IVault.json | 39 +++++++++++ contracts/contracts/interfaces/IVault.sol | 5 ++ contracts/contracts/vault/VaultAdmin.sol | 41 +++++++++-- contracts/contracts/vault/VaultStorage.sol | 9 ++- .../deploy/base/055_vault_admin_unpause.js | 40 +++++++++++ .../deploy/mainnet/202_vault_admin_unpause.js | 69 +++++++++++++++++++ contracts/test/_fixture-base.js | 12 +++- contracts/test/_fixture.js | 28 +++++++- contracts/test/vault/deposit.js | 23 +++++-- contracts/test/vault/index.js | 20 ++++++ .../test/vault/oethb-vault.base.fork-test.js | 44 ++++++++++++ contracts/test/vault/rebase.js | 20 ++++-- .../test/vault/vault.mainnet.fork-test.js | 52 ++++++++++++++ contracts/utils/addresses.js | 2 + 14 files changed, 386 insertions(+), 18 deletions(-) create mode 100644 contracts/deploy/base/055_vault_admin_unpause.js create mode 100644 contracts/deploy/mainnet/202_vault_admin_unpause.js diff --git a/contracts/abi/IVault.json b/contracts/abi/IVault.json index 509415d560..77e5faab50 100644 --- a/contracts/abi/IVault.json +++ b/contracts/abi/IVault.json @@ -1,4 +1,30 @@ [ + { + "inputs": [], + "name": "adminAddr", + "outputs": [ + { + "internalType": "address", + "name": "", + "type": "address" + } + ], + "stateMutability": "view", + "type": "function" + }, + { + "anonymous": false, + "inputs": [ + { + "indexed": false, + "internalType": "address", + "name": "newAdmin", + "type": "address" + } + ], + "name": "AdminUpdated", + "type": "event" + }, { "anonymous": false, "inputs": [ @@ -921,6 +947,19 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [ + { + "internalType": "address", + "name": "_admin", + "type": "address" + } + ], + "name": "setAdminAddr", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, { "inputs": [ { diff --git a/contracts/contracts/interfaces/IVault.sol b/contracts/contracts/interfaces/IVault.sol index 6b9c3bd3d4..c0c8d3dded 100644 --- a/contracts/contracts/interfaces/IVault.sol +++ b/contracts/contracts/interfaces/IVault.sol @@ -28,6 +28,7 @@ interface IVault { event RebasePerSecondMaxChanged(uint256 rebaseRatePerSecond); event DripDurationChanged(uint256 dripDuration); event OperatorUpdated(address newOperator); + event AdminUpdated(address newAdmin); event WithdrawalRequested( address indexed _withdrawer, uint256 indexed _requestId, @@ -68,6 +69,10 @@ interface IVault { function operatorAddr() external view returns (address); + function setAdminAddr(address _admin) external; + + function adminAddr() external view returns (address); + function setMaxSupplyDiff(uint256 _maxSupplyDiff) external; function maxSupplyDiff() external view returns (uint256); diff --git a/contracts/contracts/vault/VaultAdmin.sol b/contracts/contracts/vault/VaultAdmin.sol index 5b0f892cb9..750ee6c1a8 100644 --- a/contracts/contracts/vault/VaultAdmin.sol +++ b/contracts/contracts/vault/VaultAdmin.sol @@ -31,6 +31,30 @@ abstract contract VaultAdmin is VaultCore { _; } + /** + * @dev Verifies that the caller is the Governor or Admin. + */ + modifier onlyGovernorOrAdmin() { + require( + msg.sender == adminAddr || isGovernor(), + "Caller is not the Admin or Governor" + ); + _; + } + + /** + * @dev Verifies that the caller is the Governor, Strategist or Admin. + */ + modifier onlyGovernorOrStrategistOrAdmin() { + require( + msg.sender == strategistAddr || + msg.sender == adminAddr || + isGovernor(), + "Caller is not the Strategist, Admin or Governor" + ); + _; + } + constructor(address _asset) VaultCore(_asset) {} /*************************************** @@ -82,6 +106,15 @@ abstract contract VaultAdmin is VaultCore { emit OperatorUpdated(_operator); } + /** + * @notice Set the address authorized to pause and unpause the Vault. + * @param _admin New Admin address. + */ + function setAdminAddr(address _admin) external onlyGovernor { + adminAddr = _admin; + emit AdminUpdated(_admin); + } + /** * @notice Set the default Strategy for asset, i.e. the one which * the asset will be automatically allocated to and withdrawn from @@ -391,7 +424,7 @@ abstract contract VaultAdmin is VaultCore { /** * @notice Set the deposit paused flag to true to prevent rebasing. */ - function pauseRebase() external onlyGovernorOrStrategist { + function pauseRebase() external onlyGovernorOrStrategistOrAdmin { rebasePaused = true; emit RebasePaused(); } @@ -399,7 +432,7 @@ abstract contract VaultAdmin is VaultCore { /** * @notice Set the deposit paused flag to true to allow rebasing. */ - function unpauseRebase() external onlyGovernorOrStrategist { + function unpauseRebase() external onlyGovernorOrAdmin { rebasePaused = false; emit RebaseUnpaused(); } @@ -407,7 +440,7 @@ abstract contract VaultAdmin is VaultCore { /** * @notice Set the deposit paused flag to true to prevent capital movement. */ - function pauseCapital() external onlyGovernorOrStrategist { + function pauseCapital() external onlyGovernorOrStrategistOrAdmin { capitalPaused = true; emit CapitalPaused(); } @@ -415,7 +448,7 @@ abstract contract VaultAdmin is VaultCore { /** * @notice Set the deposit paused flag to false to enable capital movement. */ - function unpauseCapital() external onlyGovernorOrStrategist { + function unpauseCapital() external onlyGovernorOrAdmin { capitalPaused = false; emit CapitalUnpaused(); } diff --git a/contracts/contracts/vault/VaultStorage.sol b/contracts/contracts/vault/VaultStorage.sol index 92eb90e03d..24e22a25ff 100644 --- a/contracts/contracts/vault/VaultStorage.sol +++ b/contracts/contracts/vault/VaultStorage.sol @@ -43,6 +43,7 @@ abstract contract VaultStorage is Initializable, Governable { event RebasePerSecondMaxChanged(uint256 rebaseRatePerSecond); event DripDurationChanged(uint256 dripDuration); event OperatorUpdated(address newOperator); + event AdminUpdated(address newAdmin); event WithdrawalRequested( address indexed _withdrawer, uint256 indexed _requestId, @@ -207,8 +208,14 @@ abstract contract VaultStorage is Initializable, Governable { /// and Strategist are always allowed in addition to this address. address public operatorAddr; + /// @notice Address authorized to pause and unpause the Vault. Held by the + /// Admin multisig. Deliberately separate from the Strategist so a + /// single key cannot both pause and immediately unpause: the + /// Strategist trips the pause, only the Admin can lift it. + address public adminAddr; + // For future use - uint256[41] private __gap; + uint256[40] private __gap; /// @notice Index of WETH asset in allAssets array /// Legacy OETHVaultCore code, relocated here for vault consistency. diff --git a/contracts/deploy/base/055_vault_admin_unpause.js b/contracts/deploy/base/055_vault_admin_unpause.js new file mode 100644 index 0000000000..95059e731f --- /dev/null +++ b/contracts/deploy/base/055_vault_admin_unpause.js @@ -0,0 +1,40 @@ +const { deployOnBase } = require("../../utils/deploy-l2"); +const { deployWithConfirmation } = require("../../utils/deploy"); +const addresses = require("../../utils/addresses"); + +module.exports = deployOnBase( + { + deployName: "055_vault_admin_unpause", + }, + async ({ ethers }) => { + // 1. Deploy new OETHBaseVault implementation + const dOETHbVault = await deployWithConfirmation( + "OETHBaseVault", + [addresses.base.WETH], + "OETHBaseVault", + true + ); + + const cOETHbVaultProxy = await ethers.getContract("OETHBaseVaultProxy"); + const cOETHbVault = await ethers.getContractAt( + "IVault", + cOETHbVaultProxy.address + ); + + return { + name: "Upgrade OETHBaseVault: Admin can pause, only Admin can unpause", + actions: [ + { + contract: cOETHbVaultProxy, + signature: "upgradeTo(address)", + args: [dOETHbVault.address], + }, + { + contract: cOETHbVault, + signature: "setAdminAddr(address)", + args: [addresses.base.admin], + }, + ], + }; + } +); diff --git a/contracts/deploy/mainnet/202_vault_admin_unpause.js b/contracts/deploy/mainnet/202_vault_admin_unpause.js new file mode 100644 index 0000000000..e58a6ab6ce --- /dev/null +++ b/contracts/deploy/mainnet/202_vault_admin_unpause.js @@ -0,0 +1,69 @@ +const addresses = require("../../utils/addresses"); +const { deploymentWithGovernanceProposal } = require("../../utils/deploy"); + +module.exports = deploymentWithGovernanceProposal( + { + deployName: "202_vault_admin_unpause", + forceDeploy: false, + reduceQueueTime: true, + deployerIsProposer: false, + }, + async ({ deployWithConfirmation, ethers }) => { + // 1. Deploy new OUSD Vault implementation + const dOUSDVault = await deployWithConfirmation( + "OUSDVault", + [addresses.mainnet.USDC], + undefined, + true + ); + + // 2. Deploy new OETH Vault implementation + const dOETHVault = await deployWithConfirmation( + "OETHVault", + [addresses.mainnet.WETH], + undefined, + true + ); + + const cVaultProxy = await ethers.getContract("VaultProxy"); + const cOUSDVault = await ethers.getContractAt( + "IVault", + cVaultProxy.address + ); + + const cOETHVaultProxy = await ethers.getContract("OETHVaultProxy"); + const cOETHVault = await ethers.getContractAt( + "IVault", + cOETHVaultProxy.address + ); + + // The Admin (5/8) multisig. Stored in addresses.js as `Guardian`. + const adminAddr = addresses.mainnet.Guardian; + + return { + name: "Upgrade OUSD and OETH vaults: Admin can pause, only Admin can unpause", + actions: [ + { + contract: cVaultProxy, + signature: "upgradeTo(address)", + args: [dOUSDVault.address], + }, + { + contract: cOUSDVault, + signature: "setAdminAddr(address)", + args: [adminAddr], + }, + { + contract: cOETHVaultProxy, + signature: "upgradeTo(address)", + args: [dOETHVault.address], + }, + { + contract: cOETHVault, + signature: "setAdminAddr(address)", + args: [adminAddr], + }, + ], + }; + } +); diff --git a/contracts/test/_fixture-base.js b/contracts/test/_fixture-base.js index fea85a728d..a6f6bdd6ac 100644 --- a/contracts/test/_fixture-base.js +++ b/contracts/test/_fixture-base.js @@ -153,11 +153,16 @@ const defaultFixture = async () => { const oethVaultSigner = await impersonateAccount(oethbVault.address); let strategist; + let admin; if (isFork) { // Impersonate strategist on Fork strategist = await impersonateAndFund(multichainStrategistAddr); strategist.address = multichainStrategistAddr; + // The Admin (5/8) multisig + admin = await impersonateAndFund(addresses.base.admin); + admin.address = addresses.base.admin; + await impersonateAndFund(governor.address); await impersonateAndFund(timelock.address); @@ -167,7 +172,11 @@ const defaultFixture = async () => { // Production vault sits paused-for-rebase between strategist runs. // Lift the pause once per fork fixture so tests can exercise rebase // without each call site having to unpause/rebase/pause itself. - await oethbVault.connect(strategist).unpauseRebase(); + // Only the Admin can unpause. + await oethbVault.connect(admin).unpauseRebase(); + } else { + admin = signers[2]; + await oethbVault.connect(governor).setAdminAddr(admin.address); } // Make sure we can print bridged WOETH for tests @@ -260,6 +269,7 @@ const defaultFixture = async () => { guardian, timelock, strategist, + admin, minter, burner, oethVaultSigner, diff --git a/contracts/test/_fixture.js b/contracts/test/_fixture.js index 8a703d06a5..c80b2bffdc 100644 --- a/contracts/test/_fixture.js +++ b/contracts/test/_fixture.js @@ -133,17 +133,22 @@ const simpleOETHFixture = deployments.createFixture(async () => { const signers = await hre.ethers.getSigners(); let governor = signers[1]; let strategist = signers[0]; + let admin = signers[2]; const [matt, josh, anna, domen, daniel, franck] = signers.slice(4); if (isFork) { governor = await impersonateAndFund(governorAddr); strategist = await impersonateAndFund(multichainStrategistAddr); + // The Admin (5/8) multisig, stored in addresses.js as `Guardian`. + admin = await impersonateAndFund(addresses.mainnet.Guardian); + admin.address = addresses.mainnet.Guardian; // Production vault sits paused-for-rebase between strategist runs. // Lift the pause once per fork fixture so tests can exercise rebase // without each call site having to unpause/rebase/pause itself. - await oethVault.connect(strategist).unpauseRebase(); + // Only the Admin can unpause. + await oethVault.connect(admin).unpauseRebase(); for (const user of [matt, josh, anna, domen, daniel, franck]) { // Everyone gets free weth @@ -152,6 +157,8 @@ const simpleOETHFixture = deployments.createFixture(async () => { await resetAllowance(weth, user, oethVault.address); } } else { + await oethVault.connect(sGovernor).setAdminAddr(admin.address); + // Fund WETH contract await hardhatSetBalance(weth.address, "999999999999999"); @@ -171,6 +178,7 @@ const simpleOETHFixture = deployments.createFixture(async () => { anna, governor, strategist, + admin, domen, daniel, franck, @@ -694,6 +702,7 @@ const defaultFixture = deployments.createFixture(async () => { const signers = await hre.ethers.getSigners(); let governor = signers[1]; let strategist = signers[0]; + let admin = signers[2]; let timelock; let oldTimelock; @@ -702,22 +711,34 @@ const defaultFixture = deployments.createFixture(async () => { if (isFork) { governor = await impersonateAndFund(governorAddr); strategist = await impersonateAndFund(multichainStrategistAddr); + // The Admin (5/8) multisig, stored in addresses.js as `Guardian`. + admin = await impersonateAndFund(addresses.mainnet.Guardian); timelock = await impersonateAndFund(timelockAddr); oldTimelock = await impersonateAndFund(addresses.mainnet.OldTimelock); // Just a hack to get around using `.getAddress()` on the signer governor.address = governorAddr; strategist.address = multichainStrategistAddr; + admin.address = addresses.mainnet.Guardian; timelock.address = timelockAddr; oldTimelock.address = addresses.mainnet.OldTimelock; // Production vaults sit paused-for-rebase between strategist runs. // Lift the pause once per fork fixture so tests can exercise rebase // without each call site having to unpause/rebase/pause itself. - await vaultAndTokenContracts.vault.connect(strategist).unpauseRebase(); - await vaultAndTokenContracts.oethVault.connect(strategist).unpauseRebase(); + // Only the Admin can unpause. + await vaultAndTokenContracts.vault.connect(admin).unpauseRebase(); + await vaultAndTokenContracts.oethVault.connect(admin).unpauseRebase(); } else { timelock = governor; + + const sGovernor = await ethers.provider.getSigner(governorAddr); + await vaultAndTokenContracts.vault + .connect(sGovernor) + .setAdminAddr(admin.address); + await vaultAndTokenContracts.oethVault + .connect(sGovernor) + .setAdminAddr(admin.address); } if (!isFork) { @@ -747,6 +768,7 @@ const defaultFixture = deployments.createFixture(async () => { anna, governor, strategist, + admin, domen, daniel, franck, diff --git a/contracts/test/vault/deposit.js b/contracts/test/vault/deposit.js index 2225336819..10482b903a 100644 --- a/contracts/test/vault/deposit.js +++ b/contracts/test/vault/deposit.js @@ -21,21 +21,34 @@ describe("Vault deposit pausing", function () { expect(await vault.connect(anna).capitalPaused()).to.be.false; }); - it("Strategist can pause and unpause", async () => { - const { anna, strategist, vault } = fixture; + it("Strategist can pause but not unpause", async () => { + const { anna, admin, strategist, vault } = fixture; await vault.connect(strategist).pauseCapital(); expect(await vault.connect(anna).capitalPaused()).to.be.true; - await vault.connect(strategist).unpauseCapital(); + await expect(vault.connect(strategist).unpauseCapital()).to.be.revertedWith( + "Caller is not the Admin or Governor" + ); + expect(await vault.connect(anna).capitalPaused()).to.be.true; + // Only the Admin can lift the pause the Strategist tripped + await vault.connect(admin).unpauseCapital(); + expect(await vault.connect(anna).capitalPaused()).to.be.false; + }); + + it("Admin can pause and unpause", async () => { + const { anna, admin, vault } = fixture; + await vault.connect(admin).pauseCapital(); + expect(await vault.connect(anna).capitalPaused()).to.be.true; + await vault.connect(admin).unpauseCapital(); expect(await vault.connect(anna).capitalPaused()).to.be.false; }); it("Other can not pause and unpause", async () => { const { anna, vault } = fixture; await expect(vault.connect(anna).pauseCapital()).to.be.revertedWith( - "Caller is not the Strategist or Governor" + "Caller is not the Strategist, Admin or Governor" ); await expect(vault.connect(anna).unpauseCapital()).to.be.revertedWith( - "Caller is not the Strategist or Governor" + "Caller is not the Admin or Governor" ); }); diff --git a/contracts/test/vault/index.js b/contracts/test/vault/index.js index 513348cfd2..0a62e6de53 100644 --- a/contracts/test/vault/index.js +++ b/contracts/test/vault/index.js @@ -162,6 +162,26 @@ describe("Vault", function () { ).to.be.revertedWith("Caller is not the Governor"); }); + it("Should allow governor to change Admin address", async () => { + const { vault, governor, josh } = fixture; + + await expect(vault.connect(governor).setAdminAddr(josh.address)) + .to.emit(vault, "AdminUpdated") + .withArgs(josh.address); + + expect(await vault.adminAddr()).to.equal(josh.address); + }); + + it("Should not allow non-governor to change Admin address", async () => { + const { vault, admin, josh, matt, strategist } = fixture; + + for (const signer of [matt, strategist, admin]) { + await expect( + vault.connect(signer).setAdminAddr(josh.address) + ).to.be.revertedWith("Caller is not the Governor"); + } + }); + it("Should allow the Governor to call withdraw and then deposit", async () => { const { vault, governor, usdc, josh, mockStrategy } = fixture; diff --git a/contracts/test/vault/oethb-vault.base.fork-test.js b/contracts/test/vault/oethb-vault.base.fork-test.js index 583f37e6d1..940c91f1f4 100644 --- a/contracts/test/vault/oethb-vault.base.fork-test.js +++ b/contracts/test/vault/oethb-vault.base.fork-test.js @@ -21,6 +21,50 @@ describe("ForkTest: OETHb Vault", function () { await oethbVault.connect(signer).mint(oethUnits("1")); } + describe("Admin", function () { + it("Should have the correct admin address set", async () => { + const { oethbVault } = fixture; + expect(await oethbVault.adminAddr()).to.equal(addresses.base.admin); + }); + + it("Should let the strategist pause but not unpause capital", async () => { + const { admin, oethbVault, strategist } = fixture; + + await oethbVault.connect(strategist).pauseCapital(); + expect(await oethbVault.capitalPaused()).to.be.true; + + await expect( + oethbVault.connect(strategist).unpauseCapital() + ).to.be.revertedWith("Caller is not the Admin or Governor"); + expect(await oethbVault.capitalPaused()).to.be.true; + + // Only the Admin can lift the pause the Strategist tripped + await oethbVault.connect(admin).unpauseCapital(); + expect(await oethbVault.capitalPaused()).to.be.false; + }); + + it("Should let the admin pause and unpause rebase", async () => { + const { admin, oethbVault } = fixture; + + await oethbVault.connect(admin).pauseRebase(); + expect(await oethbVault.rebasePaused()).to.be.true; + await oethbVault.connect(admin).unpauseRebase(); + expect(await oethbVault.rebasePaused()).to.be.false; + }); + + it("Should still read pre-existing storage correctly after the upgrade", async () => { + const { oethbVault } = fixture; + + // The admin slot was taken from the storage gap, so a layout shift + // would show up in the slots around it first. + expect(await oethbVault.strategistAddr()).to.equal( + addresses.multichainStrategist + ); + expect(await oethbVault.governor()).to.equal(addresses.base.timelock); + expect(await oethbVault.totalValue()).to.be.gt(0); + }); + }); + describe("Mint & Permissioned redeems", function () { it("Should allow anyone to mint", async () => { const { nick, weth, oethb, oethbVault, strategist } = fixture; diff --git a/contracts/test/vault/rebase.js b/contracts/test/vault/rebase.js index 79e53a14e7..9f57eb90b5 100644 --- a/contracts/test/vault/rebase.js +++ b/contracts/test/vault/rebase.js @@ -24,10 +24,10 @@ describe("Vault rebase", () => { const { vault, anna } = fixture; await expect(vault.connect(anna).pauseRebase()).to.be.revertedWith( - "Caller is not the Strategist or Governor" + "Caller is not the Strategist, Admin or Governor" ); await expect(vault.connect(anna).unpauseRebase()).to.be.revertedWith( - "Caller is not the Strategist or Governor" + "Caller is not the Admin or Governor" ); }); @@ -37,10 +37,22 @@ describe("Vault rebase", () => { await vault.connect(josh).pauseRebase(); }); - it("Should allow strategist to unpause rebasing", async () => { + it("Should not allow strategist to unpause rebasing", async () => { const { vault, governor, josh } = fixture; await vault.connect(governor).setStrategistAddr(josh.address); - await vault.connect(josh).unpauseRebase(); + await expect(vault.connect(josh).unpauseRebase()).to.be.revertedWith( + "Caller is not the Admin or Governor" + ); + }); + + it("Should allow admin to pause rebasing", async () => { + const { vault, admin } = fixture; + await vault.connect(admin).pauseRebase(); + }); + + it("Should allow admin to unpause rebasing", async () => { + const { vault, admin } = fixture; + await vault.connect(admin).unpauseRebase(); }); it("Should allow governor to pause rebasing", async () => { diff --git a/contracts/test/vault/vault.mainnet.fork-test.js b/contracts/test/vault/vault.mainnet.fork-test.js index 2a04129eef..ccd2423f93 100644 --- a/contracts/test/vault/vault.mainnet.fork-test.js +++ b/contracts/test/vault/vault.mainnet.fork-test.js @@ -71,6 +71,58 @@ describe("ForkTest: Vault", function () { ); }); + it("Should have the correct admin address set", async () => { + const { vault } = fixture; + expect(await vault.adminAddr()).to.equal(addresses.mainnet.Guardian); + }); + + it("Should let the strategist pause but not unpause capital", async () => { + const { admin, strategist, vault } = fixture; + + await vault.connect(strategist).pauseCapital(); + expect(await vault.capitalPaused()).to.be.true; + + await expect( + vault.connect(strategist).unpauseCapital() + ).to.be.revertedWith("Caller is not the Admin or Governor"); + expect(await vault.capitalPaused()).to.be.true; + + // Only the Admin can lift the pause the Strategist tripped + await vault.connect(admin).unpauseCapital(); + expect(await vault.capitalPaused()).to.be.false; + }); + + it("Should let the admin pause and unpause capital and rebase", async () => { + const { admin, vault } = fixture; + + await vault.connect(admin).pauseCapital(); + expect(await vault.capitalPaused()).to.be.true; + await vault.connect(admin).unpauseCapital(); + expect(await vault.capitalPaused()).to.be.false; + + await vault.connect(admin).pauseRebase(); + expect(await vault.rebasePaused()).to.be.true; + await vault.connect(admin).unpauseRebase(); + expect(await vault.rebasePaused()).to.be.false; + }); + + it("Should still read pre-existing storage correctly after the upgrade", async () => { + const { vault, ousd } = fixture; + + // The admin slot was taken from the storage gap. `defaultStrategy` and + // `operatorAddr` are the two slots immediately before it, so a layout + // shift would show up here first. + expect(await vault.defaultStrategy()).to.not.equal(addresses.zero); + expect(await vault.operatorAddr()).to.not.equal(addresses.zero); + expect(await vault.strategistAddr()).to.equal( + addresses.multichainStrategist + ); + expect(await vault.trusteeAddress()).to.not.equal(addresses.zero); + expect(await vault.governor()).to.equal(addresses.mainnet.Timelock); + expect(await vault.totalValue()).to.be.gt(0); + expect(await ousd.totalSupply()).to.be.gt(0); + }); + it("Should have the OUSD/USDC AMO mint whitelist", async () => { const { vault } = fixture; expect( diff --git a/contracts/utils/addresses.js b/contracts/utils/addresses.js index c5120cef30..f7013c7ec3 100644 --- a/contracts/utils/addresses.js +++ b/contracts/utils/addresses.js @@ -424,6 +424,8 @@ addresses.base.WETH = "0x4200000000000000000000000000000000000006"; addresses.base.wethAeroPoolAddress = "0x80aBe24A3ef1fc593aC5Da960F232ca23B2069d0"; addresses.base.governor = "0x92A19381444A001d62cE67BaFF066fA1111d7202"; +// 5/8 Multisig. Same Safe. +addresses.base.admin = "0x92A19381444A001d62cE67BaFF066fA1111d7202"; // 2/8 Multisig addresses.base.strategist = "0x28bce2eE5775B652D92bB7c2891A89F036619703"; addresses.base.timelock = "0xf817cb3092179083c48c014688D98B72fB61464f"; From b228677e887b8e9878b9edc98ea954aeb145ebcb Mon Sep 17 00:00:00 2001 From: Shahul Hameed <10547529+shahthepro@users.noreply.github.com> Date: Thu, 13 Aug 2026 16:53:42 +0400 Subject: [PATCH 2/5] Add Safe Module --- .../contracts/automation/PauseSafeModule.sol | 122 +++++++++++++++ .../automation/PermissionedRebaseModule.sol | 94 ------------ .../automation/IPauseSafeModule.sol | 21 +++ .../automation/IPermissionedRebaseModule.sol | 20 --- .../deploy/base/056_pause_safe_module.js | 63 ++++++++ .../deploy/mainnet/203_pause_safe_module.js | 72 +++++++++ contracts/docs/ACTIONS.md | 3 - contracts/docs/talos-actions-inventory.md | 1 - contracts/migrations/seed_schedules.sql | 3 - contracts/tasks/actions/permissionedRebase.ts | 62 -------- .../pause-module.base.fork-test.js | 70 +++++++++ .../pause-module.mainnet.fork-test.js | 93 ++++++++++++ .../concrete/Constructor.t.sol | 57 +++++++ .../concrete/PauseActions.t.sol | 142 ++++++++++++++++++ .../concrete/TargetManagement.t.sol | 86 +++++++++++ .../shared/Shared.t.sol | 89 ++++------- .../concrete/PermissionedRebase.t.sol | 123 --------------- .../concrete/VaultManagement.t.sol | 117 --------------- .../unit/vault/OETHVault/concrete/Admin.t.sol | 20 ++- .../unit/vault/OETHVault/shared/Shared.t.sol | 1 + .../unit/vault/OUSDVault/concrete/Admin.t.sol | 51 ++++++- .../unit/vault/OUSDVault/shared/Shared.t.sol | 1 + .../tests/utils/artifacts/Automation.sol | 4 +- 23 files changed, 818 insertions(+), 497 deletions(-) create mode 100644 contracts/contracts/automation/PauseSafeModule.sol delete mode 100644 contracts/contracts/automation/PermissionedRebaseModule.sol create mode 100644 contracts/contracts/interfaces/automation/IPauseSafeModule.sol delete mode 100644 contracts/contracts/interfaces/automation/IPermissionedRebaseModule.sol create mode 100644 contracts/deploy/base/056_pause_safe_module.js create mode 100644 contracts/deploy/mainnet/203_pause_safe_module.js delete mode 100644 contracts/tasks/actions/permissionedRebase.ts create mode 100644 contracts/test/safe-modules/pause-module.base.fork-test.js create mode 100644 contracts/test/safe-modules/pause-module.mainnet.fork-test.js create mode 100644 contracts/tests/unit/automation/PauseSafeModule/concrete/Constructor.t.sol create mode 100644 contracts/tests/unit/automation/PauseSafeModule/concrete/PauseActions.t.sol create mode 100644 contracts/tests/unit/automation/PauseSafeModule/concrete/TargetManagement.t.sol rename contracts/tests/unit/automation/{PermissionedRebaseModule => PauseSafeModule}/shared/Shared.t.sol (52%) delete mode 100644 contracts/tests/unit/automation/PermissionedRebaseModule/concrete/PermissionedRebase.t.sol delete mode 100644 contracts/tests/unit/automation/PermissionedRebaseModule/concrete/VaultManagement.t.sol diff --git a/contracts/contracts/automation/PauseSafeModule.sol b/contracts/contracts/automation/PauseSafeModule.sol new file mode 100644 index 0000000000..401c0a2ae7 --- /dev/null +++ b/contracts/contracts/automation/PauseSafeModule.sol @@ -0,0 +1,122 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +import { AbstractSafeModule } from "./AbstractSafeModule.sol"; + +import { IVault } from "../interfaces/IVault.sol"; + +/** + * @title PauseSafeModule + * @notice Gnosis Safe module that lets a threat-detection operator pause OToken + * vaults without waiting for multisig signatures to be gathered. + * + * @dev The Safe hosting this module is the Guardian multisig, which is the + * vaults' `strategistAddr` and therefore already authorized to pause. This + * module does not widen that authority — it only lets a keyed operator + * exercise it without collecting signatures. + * + * Safety properties, in order of importance: + * + * 1. This module can never unpause. The only two selectors it can encode + * are `pauseCapital()` and `pauseRebase()`, both compiled in below. + * That is a property of the bytecode, not of configuration — there is + * no allow-list entry or role that could turn an unpause into a legal + * call. Unpausing requires the Admin multisig acting directly on the + * vault. + * 2. Targets are allow-listed by the Safe, so a compromised operator key + * cannot aim a pause at an arbitrary contract. + * 3. Pause failures revert. A pause that silently did not land is worse + * than a loud failure, because the detection service would treat the + * protocol as contained when it is not. + * + * The Safe must call `enableModule(address(this))` before this module can + * do anything at all. + */ +contract PauseSafeModule is AbstractSafeModule { + /// @notice Contracts this module is permitted to pause. + mapping(address => bool) public isPausableTarget; + + event TargetAllowed(address indexed target); + event TargetRevoked(address indexed target); + event CapitalPauseExecuted(address indexed target); + event RebasePauseExecuted(address indexed target); + + /** + * @param _safeContract Address of the Gnosis Safe (Guardian multisig). + * @param _operators Addresses allowed to trigger a pause. Typically the + * threat-detection keeper plus an in-house backup. + * @param _targets Contracts this module may pause. + */ + constructor( + address _safeContract, + address[] memory _operators, + address[] memory _targets + ) AbstractSafeModule(_safeContract) { + for (uint256 i = 0; i < _operators.length; i++) { + require(_operators[i] != address(0), "Invalid operator"); + _grantRole(OPERATOR_ROLE, _operators[i]); + } + + for (uint256 i = 0; i < _targets.length; i++) { + _allowTarget(_targets[i]); + } + } + + /** + * @notice Halt mint and redeem on an allow-listed vault. + * @param _target Vault to pause. + */ + function pauseCapital(address _target) external onlyOperator { + _execPause(_target, IVault.pauseCapital.selector); + emit CapitalPauseExecuted(_target); + } + + /** + * @notice Halt rebasing on an allow-listed vault. + * @param _target Vault to pause. + */ + function pauseRebase(address _target) external onlyOperator { + _execPause(_target, IVault.pauseRebase.selector); + emit RebasePauseExecuted(_target); + } + + /// @dev Execute a pause selector on `_target` through the Safe. + function _execPause(address _target, bytes4 _selector) internal { + require(isPausableTarget[_target], "Target not allowed"); + + bool success = safeContract.execTransactionFromModule( + _target, + 0, // Value + abi.encodeWithSelector(_selector), + 0 // Call + ); + + require(success, "Pause failed"); + } + + /** + * @notice Allow this module to pause a contract. Only the Safe can call. + * @param _target Contract to add to the allow-list. + */ + function allowTarget(address _target) external onlySafe { + _allowTarget(_target); + } + + function _allowTarget(address _target) internal { + require(_target != address(0), "Invalid target"); + require(!isPausableTarget[_target], "Target already allowed"); + isPausableTarget[_target] = true; + emit TargetAllowed(_target); + } + + /** + * @notice Stop this module being able to pause a contract. Only the Safe + * can call. + * @param _target Contract to remove from the allow-list. + */ + function revokeTarget(address _target) external onlySafe { + require(isPausableTarget[_target], "Target not allowed"); + isPausableTarget[_target] = false; + emit TargetRevoked(_target); + } +} diff --git a/contracts/contracts/automation/PermissionedRebaseModule.sol b/contracts/contracts/automation/PermissionedRebaseModule.sol deleted file mode 100644 index 780eb9bce8..0000000000 --- a/contracts/contracts/automation/PermissionedRebaseModule.sol +++ /dev/null @@ -1,94 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 -pragma solidity ^0.8.0; - -import { AbstractSafeModule } from "./AbstractSafeModule.sol"; -import { IVault } from "../interfaces/IVault.sol"; - -/** - * @title PermissionedRebaseModule - * @notice Safe module that lets a permissioned operator drive a `rebase()` - * on vaults that are kept in the rebase-paused state. For each - * configured vault, the module calls (in order, via the Safe): - * `unpauseRebase()` -> `rebase()` -> `pauseRebase()`. - * If any sub-call fails, the whole transaction reverts so a vault - * can never be left unpaused by a partial run. - */ -contract PermissionedRebaseModule is AbstractSafeModule { - mapping(address => bool) public isVaultWhitelisted; - address[] public vaults; - - event VaultAdded(address vault); - event VaultRemoved(address vault); - event PermissionedRebaseExecuted(address vault); - - constructor( - address _safeAddress, - address _operator, - address[] memory _vaults - ) AbstractSafeModule(_safeAddress) { - _grantRole(OPERATOR_ROLE, _operator); - - for (uint256 i = 0; i < _vaults.length; i++) { - _addVault(_vaults[i]); - } - } - - /** - * @notice For every whitelisted vault, sequentially call - * `unpauseRebase()`, `rebase()`, then `pauseRebase()` via the - * Safe. Reverts atomically on any sub-call failure. - */ - function permissionedRebase() external onlyRole(OPERATOR_ROLE) { - uint256 vaultsLength = vaults.length; - for (uint256 i = 0; i < vaultsLength; i++) { - address vault = vaults[i]; - _execOnVault(vault, IVault.unpauseRebase.selector); - _execOnVault(vault, IVault.rebase.selector); - _execOnVault(vault, IVault.pauseRebase.selector); - emit PermissionedRebaseExecuted(vault); - } - } - - function _execOnVault(address vault, bytes4 selector) internal { - bool success = safeContract.execTransactionFromModule( - vault, - 0, // Value - abi.encodeWithSelector(selector), - 0 // Call - ); - require(success, "Vault call failed"); - } - - /** - * @notice Add a vault to the whitelist. Only the Safe can call. - */ - function addVault(address _vault) external onlyRole(DEFAULT_ADMIN_ROLE) { - _addVault(_vault); - } - - function _addVault(address _vault) internal { - require(_vault != address(0), "Vault is zero address"); - require(!isVaultWhitelisted[_vault], "Vault already whitelisted"); - isVaultWhitelisted[_vault] = true; - vaults.push(_vault); - emit VaultAdded(_vault); - } - - /** - * @notice Remove a vault from the whitelist. Only the Safe can call. - */ - function removeVault(address _vault) external onlyRole(DEFAULT_ADMIN_ROLE) { - require(isVaultWhitelisted[_vault], "Vault not whitelisted"); - isVaultWhitelisted[_vault] = false; - - for (uint256 i = 0; i < vaults.length; i++) { - if (vaults[i] == _vault) { - vaults[i] = vaults[vaults.length - 1]; - vaults.pop(); - break; - } - } - - emit VaultRemoved(_vault); - } -} diff --git a/contracts/contracts/interfaces/automation/IPauseSafeModule.sol b/contracts/contracts/interfaces/automation/IPauseSafeModule.sol new file mode 100644 index 0000000000..7ae0c635db --- /dev/null +++ b/contracts/contracts/interfaces/automation/IPauseSafeModule.sol @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +import { IAbstractSafeModule } from "contracts/interfaces/automation/IAbstractSafeModule.sol"; + +interface IPauseSafeModule is IAbstractSafeModule { + event TargetAllowed(address indexed target); + event TargetRevoked(address indexed target); + event CapitalPauseExecuted(address indexed target); + event RebasePauseExecuted(address indexed target); + + function isPausableTarget(address target) external view returns (bool); + + function pauseCapital(address _target) external; + + function pauseRebase(address _target) external; + + function allowTarget(address _target) external; + + function revokeTarget(address _target) external; +} diff --git a/contracts/contracts/interfaces/automation/IPermissionedRebaseModule.sol b/contracts/contracts/interfaces/automation/IPermissionedRebaseModule.sol deleted file mode 100644 index 94e4c1d55e..0000000000 --- a/contracts/contracts/interfaces/automation/IPermissionedRebaseModule.sol +++ /dev/null @@ -1,20 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 -pragma solidity ^0.8.0; - -import { IAbstractSafeModule } from "contracts/interfaces/automation/IAbstractSafeModule.sol"; - -interface IPermissionedRebaseModule is IAbstractSafeModule { - event VaultAdded(address vault); - event VaultRemoved(address vault); - event PermissionedRebaseExecuted(address vault); - - function isVaultWhitelisted(address vault) external view returns (bool); - - function vaults(uint256 index) external view returns (address); - - function permissionedRebase() external; - - function addVault(address _vault) external; - - function removeVault(address _vault) external; -} diff --git a/contracts/deploy/base/056_pause_safe_module.js b/contracts/deploy/base/056_pause_safe_module.js new file mode 100644 index 0000000000..e518a4d366 --- /dev/null +++ b/contracts/deploy/base/056_pause_safe_module.js @@ -0,0 +1,63 @@ +const { deployOnBase } = require("../../utils/deploy-l2"); +const addresses = require("../../utils/addresses"); +const { isFork } = require("../../utils/hardhat-helpers"); +const { impersonateAndFund } = require("../../utils/signers"); + +module.exports = deployOnBase( + { + deployName: "056_pause_safe_module", + }, + async ({ deployWithConfirmation, withConfirmation }) => { + const safeAddress = addresses.multichainStrategist; + + const cOETHbVaultProxy = await ethers.getContract("OETHBaseVaultProxy"); + + // The Hypernative keeper address is not known yet. Deploy with the Talos + // relayer as the initial operator; the Safe grants OPERATOR_ROLE to the + // keeper once the vendor supplies its address (see checklist below). + const operators = [addresses.talosRelayer]; + + await deployWithConfirmation("PauseSafeModule", [ + safeAddress, + operators, + [cOETHbVaultProxy.address], + ]); + const cPauseSafeModule = await ethers.getContract("PauseSafeModule"); + + console.log( + `PauseSafeModule (for ${safeAddress}) deployed to`, + cPauseSafeModule.address + ); + console.log(` +======================================================================= +REQUIRED POST-DEPLOY STEPS — the module does nothing until these are done +======================================================================= + 1. Guardian Safe (${safeAddress}) calls: + enableModule(${cPauseSafeModule.address}) + 2. Verify on-chain: + safe.isModuleEnabled(${cPauseSafeModule.address}) == true + 3. Guardian Safe grants the Hypernative keeper the operator role: + pauseSafeModule.grantRole(OPERATOR_ROLE, ) + 4. Smoke-test one pauseCapital() on a fork before arming detection. +======================================================================= +`); + + if (isFork) { + const safeSigner = await impersonateAndFund(safeAddress); + const cSafe = await ethers.getContractAt( + ["function enableModule(address module) external"], + safeAddress + ); + + await withConfirmation( + cSafe.connect(safeSigner).enableModule(cPauseSafeModule.address) + ); + + console.log("Enabled PauseSafeModule on fork"); + } + + return { + actions: [], + }; + } +); diff --git a/contracts/deploy/mainnet/203_pause_safe_module.js b/contracts/deploy/mainnet/203_pause_safe_module.js new file mode 100644 index 0000000000..23c4175db4 --- /dev/null +++ b/contracts/deploy/mainnet/203_pause_safe_module.js @@ -0,0 +1,72 @@ +const addresses = require("../../utils/addresses"); +const { + deploymentWithGovernanceProposal, + deployWithConfirmation, + withConfirmation, +} = require("../../utils/deploy"); +const { isFork } = require("../../utils/hardhat-helpers"); +const { impersonateAndFund } = require("../../utils/signers"); + +module.exports = deploymentWithGovernanceProposal( + { + deployName: "203_pause_safe_module", + forceDeploy: false, + reduceQueueTime: true, + deployerIsProposer: false, + proposalId: "", + }, + async () => { + const safeAddress = addresses.multichainStrategist; + + const cVaultProxy = await ethers.getContract("VaultProxy"); + const cOETHVaultProxy = await ethers.getContract("OETHVaultProxy"); + + // The Hypernative keeper address is not known yet. Deploy with the Talos + // relayer as the initial operator; the Safe grants OPERATOR_ROLE to the + // keeper once the vendor supplies its address (see checklist below). + const operators = [addresses.talosRelayer]; + + await deployWithConfirmation("PauseSafeModule", [ + safeAddress, + operators, + [cVaultProxy.address, cOETHVaultProxy.address], + ]); + const cPauseSafeModule = await ethers.getContract("PauseSafeModule"); + + console.log(`PauseSafeModule deployed to ${cPauseSafeModule.address}`); + console.log(` +======================================================================= +REQUIRED POST-DEPLOY STEPS — the module does nothing until these are done +======================================================================= + 1. Guardian Safe (${safeAddress}) calls: + enableModule(${cPauseSafeModule.address}) + 2. Verify on-chain: + safe.isModuleEnabled(${cPauseSafeModule.address}) == true + 3. Guardian Safe grants the Hypernative keeper the operator role: + pauseSafeModule.grantRole(OPERATOR_ROLE, ) + 4. Smoke-test one pauseCapital() on a fork before arming detection. + + Step 1 is the one that was silently skipped for PermissionedRebaseModule, + leaving it dead on-chain for months. Do not close this PR until it is done. +======================================================================= +`); + + if (isFork) { + const safeSigner = await impersonateAndFund(safeAddress); + const cSafe = await ethers.getContractAt( + ["function enableModule(address module) external"], + safeAddress + ); + + await withConfirmation( + cSafe.connect(safeSigner).enableModule(cPauseSafeModule.address) + ); + + console.log("Enabled PauseSafeModule on fork"); + } + + return { + actions: [], + }; + } +); diff --git a/contracts/docs/ACTIONS.md b/contracts/docs/ACTIONS.md index abcc7aee4f..fffa6e8575 100644 --- a/contracts/docs/ACTIONS.md +++ b/contracts/docs/ACTIONS.md @@ -23,9 +23,6 @@ Cron times are UTC. Enable state and operational caveats (e.g. "do not enable", | `otokenOusdOethRebase` | mainnet | `45 11,23 * * *` | Collect OETH and rebase OUSD on mainnet | | `otokenOsRebase` | sonic | `45 11,23 * * *` | Collect the OS dripper and rebase OS on Sonic | | `otokenOethbRebase` | base | `25 9,21 * * *` | Rebase the OETHb vault on Base | -| `permissionedRebase` | mainnet | `15 10,22 * * *` | Collect fixed-rate drippers, then `permissionedRebase()` every managed vault via the Safe module (unpause → rebase → re-pause atomically) | -| `permissionedRebase` | base | `15 10,22 * * *` | As above, on Base | -| `permissionedRebase` | sonic | `15 10,22 * * *` | As above, on Sonic | ## OToken operations diff --git a/contracts/docs/talos-actions-inventory.md b/contracts/docs/talos-actions-inventory.md index d6f49b81f0..df53989458 100644 --- a/contracts/docs/talos-actions-inventory.md +++ b/contracts/docs/talos-actions-inventory.md @@ -14,7 +14,6 @@ | arb | updateVotemarketEpochs | | eth, base | crossChainRelay, manageMerklBribes, proposeVaultStrategyMoves, relayCCTPMessage | | eth, hoodi | doAccounting, registerValidators, stakeValidators | -| eth, sonic, base | permissionedRebase | | eth, sonic, base, plume | otokenAddWithdrawalQueueLiquidity | | eth, sonic, hyper, base, holesky, arb, plume, hoodi | healthcheck | diff --git a/contracts/migrations/seed_schedules.sql b/contracts/migrations/seed_schedules.sql index 1e09ad7651..8d44cf7619 100644 --- a/contracts/migrations/seed_schedules.sql +++ b/contracts/migrations/seed_schedules.sql @@ -51,9 +51,6 @@ INSERT INTO schedules (product, name, command, cron_expr, timezone, enabled, not ('origin-dollar', 'otoken_os_rebase', 'cd /app && pnpm exec tsx tasks/run.ts otokenOsRebase --network sonic', '45 11,23 * * *', 'UTC', false, NULL), ('origin-dollar', 'ogn_claimAndForwardRewards', 'cd /app && pnpm exec tsx tasks/run.ts ognClaimAndForwardRewards --network mainnet', '50 0 * * 2', 'UTC', false, NULL), ('origin-dollar', 'otoken_oethb_harvest', 'cd /app && pnpm exec tsx tasks/run.ts otokenOethbHarvest --network base', '55 11 * * *', 'UTC', false, NULL), -('origin-dollar', 'module_rebase_mainnet', 'cd /app && pnpm exec tsx tasks/run.ts permissionedRebase --network mainnet', '15 10,22 * * *', 'UTC', false, NULL), -('origin-dollar', 'module_rebase_base', 'cd /app && pnpm exec tsx tasks/run.ts permissionedRebase --network base', '15 10,22 * * *', 'UTC', false, NULL), -('origin-dollar', 'module_rebase_sonic', 'cd /app && pnpm exec tsx tasks/run.ts permissionedRebase --network sonic', '15 10,22 * * *', 'UTC', false, NULL), ('origin-dollar', 'ousd_rebalancer', 'cd /app && pnpm exec tsx tasks/run.ts ousdRebalancer --network mainnet', '0 0 1 1 *', 'UTC', false, 'Manual: Run now to rebalance OUSD Morpho strategies'), ('origin-dollar', 'propose_vault_strategy_moves_mainnet', 'cd /app && pnpm exec tsx tasks/run.ts proposeVaultStrategyMoves --network mainnet --vault OUSD --moves "withdrawAll:REPLACE_WITH_STRATEGY"', '0 0 1 1 *', 'UTC', false, 'Manual Ethereum launcher: replace vault and moves before Run now. Proposes a Strategist Safe transaction; never enable this schedule.'), ('origin-dollar', 'propose_vault_strategy_moves_base', 'cd /app && pnpm exec tsx tasks/run.ts proposeVaultStrategyMoves --network base --vault SuperOETH --moves "withdrawAll:REPLACE_WITH_STRATEGY"', '0 0 1 1 *', 'UTC', false, 'Manual Base launcher: replace moves before Run now. Proposes a Strategist Safe transaction; never enable this schedule.'), diff --git a/contracts/tasks/actions/permissionedRebase.ts b/contracts/tasks/actions/permissionedRebase.ts deleted file mode 100644 index 6d296da5fc..0000000000 --- a/contracts/tasks/actions/permissionedRebase.ts +++ /dev/null @@ -1,62 +0,0 @@ -import { ethers as ethersLib } from "ethers"; -import { action } from "../lib/action"; -import { getContract } from "../lib/contracts"; -import { logTxDetails } from "../../utils/txLogger"; - -// PermissionedRebase Safe module addresses, keyed by chain id. The contract -// source lives in a PR not yet merged into this branch — until that lands and -// the module gets a Hardhat deployment artifact, addresses are pinned here. -const MODULES_BY_CHAIN_ID: Record = { - 1: "0xB3bCfA33C54fa4D18146196eDfB404BD036a52a6", // Ethereum - 8453: "0xf633980A61E9F90a41d030676059Dc201D9d4A37", // Base - 146: "0x77121911A387c9e4Eae46345E0f831A6da8a1364", // Sonic -}; - -const MAINNET_OETH_DRIPPER_DEPLOYMENT = "OETHFixedRateDripperProxy"; -const DRIPPER_ABI = ["function collect() external"]; -const PERMISSIONED_REBASE_ABI = ["function permissionedRebase() external"]; - -action({ - name: "permissionedRebase", - description: - "Collect fixed-rate drippers, then call permissionedRebase() on the PermissionedRebase Safe module on the current chain (Ethereum / Base / Sonic). The module unpauses, rebases, and re-pauses every vault it manages atomically.", - chains: [1, 8453, 146], - run: async ({ signer, chainId, networkName, log }) => { - const moduleAddress = MODULES_BY_CHAIN_ID[chainId]; - if (!moduleAddress) { - throw new Error( - `No PermissionedRebase module address configured for ${networkName} (${chainId})` - ); - } - - if (chainId === 1) { - const dripperProxy = await getContract(MAINNET_OETH_DRIPPER_DEPLOYMENT); - log.info( - `Calling collect on ${networkName} fixed-rate dripper ${MAINNET_OETH_DRIPPER_DEPLOYMENT} at ${dripperProxy.address}` - ); - - const dripper = new ethersLib.Contract( - dripperProxy.address, - DRIPPER_ABI, - signer - ); - const collectTx = await dripper.collect(); - await logTxDetails( - collectTx, - `${MAINNET_OETH_DRIPPER_DEPLOYMENT}.collect` - ); - } - - log.info( - `Calling permissionedRebase on ${networkName} module at ${moduleAddress}` - ); - - const module = new ethersLib.Contract( - moduleAddress, - PERMISSIONED_REBASE_ABI, - signer - ); - const tx = await module.permissionedRebase(); - await logTxDetails(tx, "permissionedRebase"); - }, -}); diff --git a/contracts/test/safe-modules/pause-module.base.fork-test.js b/contracts/test/safe-modules/pause-module.base.fork-test.js new file mode 100644 index 0000000000..f30b0f7ceb --- /dev/null +++ b/contracts/test/safe-modules/pause-module.base.fork-test.js @@ -0,0 +1,70 @@ +const { expect } = require("chai"); + +const addresses = require("../../utils/addresses"); +const { createFixtureLoader } = require("../_fixture"); +const { defaultBaseFixture } = require("../_fixture-base"); +const { impersonateAndFund } = require("../../utils/signers"); + +const baseFixture = createFixtureLoader(defaultBaseFixture); + +describe("ForkTest: Pause Safe Module (Base)", function () { + this.timeout(0); + + let fixture; + let pauseModule; + let operator; + + beforeEach(async () => { + fixture = await baseFixture(); + pauseModule = await ethers.getContract("PauseSafeModule"); + operator = await impersonateAndFund(addresses.talosRelayer); + }); + + it("Should be enabled on the Guardian Safe", async () => { + const safe = await ethers.getContractAt( + ["function isModuleEnabled(address module) external view returns (bool)"], + addresses.multichainStrategist + ); + expect(await safe.isModuleEnabled(pauseModule.address)).to.be.true; + }); + + it("Should have the OETHb vault allow-listed", async () => { + const { oethbVault } = fixture; + expect(await pauseModule.isPausableTarget(oethbVault.address)).to.be.true; + }); + + it("Should let the operator pause capital, and only the admin lift it", async () => { + const { admin, oethbVault, strategist } = fixture; + + expect(await oethbVault.capitalPaused()).to.be.false; + + await pauseModule.connect(operator).pauseCapital(oethbVault.address); + expect(await oethbVault.capitalPaused()).to.be.true; + + await expect( + oethbVault.connect(strategist).unpauseCapital() + ).to.be.revertedWith("Caller is not the Admin or Governor"); + expect(await oethbVault.capitalPaused()).to.be.true; + + await oethbVault.connect(admin).unpauseCapital(); + expect(await oethbVault.capitalPaused()).to.be.false; + }); + + it("Should let the operator pause rebase", async () => { + const { admin, oethbVault } = fixture; + + await pauseModule.connect(operator).pauseRebase(oethbVault.address); + expect(await oethbVault.rebasePaused()).to.be.true; + + await oethbVault.connect(admin).unpauseRebase(); + expect(await oethbVault.rebasePaused()).to.be.false; + }); + + it("Should revert for a non-operator", async () => { + const { nick, oethbVault } = fixture; + + await expect( + pauseModule.connect(nick).pauseCapital(oethbVault.address) + ).to.be.revertedWith("Caller is not an operator"); + }); +}); diff --git a/contracts/test/safe-modules/pause-module.mainnet.fork-test.js b/contracts/test/safe-modules/pause-module.mainnet.fork-test.js new file mode 100644 index 0000000000..e544448195 --- /dev/null +++ b/contracts/test/safe-modules/pause-module.mainnet.fork-test.js @@ -0,0 +1,93 @@ +const { expect } = require("chai"); + +const addresses = require("../../utils/addresses"); +const { loadDefaultFixture } = require("../_fixture"); +const { isCI } = require("../helpers"); +const { impersonateAndFund } = require("../../utils/signers"); + +describe("ForkTest: Pause Safe Module", function () { + this.timeout(0); + this.retries(isCI ? 3 : 0); + + let fixture; + let pauseModule; + let operator; + + beforeEach(async () => { + fixture = await loadDefaultFixture(); + pauseModule = await ethers.getContract("PauseSafeModule"); + operator = await impersonateAndFund(addresses.talosRelayer); + }); + + it("Should have the expected operator", async () => { + const operatorRole = await pauseModule.OPERATOR_ROLE(); + expect(await pauseModule.hasRole(operatorRole, addresses.talosRelayer)).to + .be.true; + }); + + it("Should be enabled on the Guardian Safe", async () => { + const safe = await ethers.getContractAt( + ["function isModuleEnabled(address module) external view returns (bool)"], + addresses.multichainStrategist + ); + expect(await safe.isModuleEnabled(pauseModule.address)).to.be.true; + }); + + it("Should have both mainnet vaults allow-listed", async () => { + const vaultProxy = await ethers.getContract("VaultProxy"); + const oethVaultProxy = await ethers.getContract("OETHVaultProxy"); + + expect(await pauseModule.isPausableTarget(vaultProxy.address)).to.be.true; + expect(await pauseModule.isPausableTarget(oethVaultProxy.address)).to.be + .true; + }); + + for (const [vaultName, vaultProxyName] of [ + ["OUSD", "VaultProxy"], + ["OETH", "OETHVaultProxy"], + ]) { + describe(`${vaultName} vault`, () => { + it("Should let the operator pause capital, and only the admin lift it", async () => { + const { admin, strategist } = fixture; + const proxy = await ethers.getContract(vaultProxyName); + const vault = await ethers.getContractAt("IVault", proxy.address); + + expect(await vault.capitalPaused()).to.be.false; + + await pauseModule.connect(operator).pauseCapital(vault.address); + expect(await vault.capitalPaused()).to.be.true; + + // The Safe hosting the module is the Strategist. It could trip the + // pause, but it cannot lift it — that is the whole point. + await expect( + vault.connect(strategist).unpauseCapital() + ).to.be.revertedWith("Caller is not the Admin or Governor"); + expect(await vault.capitalPaused()).to.be.true; + + await vault.connect(admin).unpauseCapital(); + expect(await vault.capitalPaused()).to.be.false; + }); + + it("Should let the operator pause rebase", async () => { + const { admin } = fixture; + const proxy = await ethers.getContract(vaultProxyName); + const vault = await ethers.getContractAt("IVault", proxy.address); + + await pauseModule.connect(operator).pauseRebase(vault.address); + expect(await vault.rebasePaused()).to.be.true; + + await vault.connect(admin).unpauseRebase(); + expect(await vault.rebasePaused()).to.be.false; + }); + + it("Should revert for a non-operator", async () => { + const { anna } = fixture; + const proxy = await ethers.getContract(vaultProxyName); + + await expect( + pauseModule.connect(anna).pauseCapital(proxy.address) + ).to.be.revertedWith("Caller is not an operator"); + }); + }); + } +}); diff --git a/contracts/tests/unit/automation/PauseSafeModule/concrete/Constructor.t.sol b/contracts/tests/unit/automation/PauseSafeModule/concrete/Constructor.t.sol new file mode 100644 index 0000000000..ab8617db3f --- /dev/null +++ b/contracts/tests/unit/automation/PauseSafeModule/concrete/Constructor.t.sol @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +// --- Test base +import {Unit_PauseSafeModule_Shared_Test} from "tests/unit/automation/PauseSafeModule/shared/Shared.t.sol"; + +// --- Test utilities +import {Automation} from "tests/utils/artifacts/Automation.sol"; + +contract Unit_Concrete_PauseSafeModule_Constructor_Test is Unit_PauseSafeModule_Shared_Test { + ////////////////////////////////////////////////////// + /// --- CONSTRUCTOR + ////////////////////////////////////////////////////// + + function test_constructor_safeContractSet() public view { + assertEq(pauseSafeModule.safeContract(), address(mockSafe)); + } + + function test_constructor_initialTargetAllowed() public view { + assertTrue(pauseSafeModule.isPausableTarget(address(oethVault))); + } + + function test_constructor_unlistedVaultNotAllowed() public view { + assertFalse(pauseSafeModule.isPausableTarget(address(unlistedVault))); + } + + function test_constructor_operatorRoleGranted() public view { + assertTrue(pauseSafeModule.hasRole(pauseSafeModule.OPERATOR_ROLE(), operator)); + } + + function test_constructor_safeHasAdminRole() public view { + assertTrue(pauseSafeModule.hasRole(pauseSafeModule.DEFAULT_ADMIN_ROLE(), address(mockSafe))); + } + + function test_constructor_safeHasOperatorRole() public view { + assertTrue(pauseSafeModule.hasRole(pauseSafeModule.OPERATOR_ROLE(), address(mockSafe))); + } + + function test_constructor_revertsOnZeroOperator() public { + address[] memory operators = new address[](1); + operators[0] = address(0); + address[] memory targets = new address[](0); + + vm.expectRevert("Invalid operator"); + vm.deployCode(Automation.PAUSE_SAFE_MODULE, abi.encode(address(mockSafe), operators, targets)); + } + + function test_constructor_revertsOnZeroTarget() public { + address[] memory operators = new address[](1); + operators[0] = operator; + address[] memory targets = new address[](1); + targets[0] = address(0); + + vm.expectRevert("Invalid target"); + vm.deployCode(Automation.PAUSE_SAFE_MODULE, abi.encode(address(mockSafe), operators, targets)); + } +} diff --git a/contracts/tests/unit/automation/PauseSafeModule/concrete/PauseActions.t.sol b/contracts/tests/unit/automation/PauseSafeModule/concrete/PauseActions.t.sol new file mode 100644 index 0000000000..aefca9a704 --- /dev/null +++ b/contracts/tests/unit/automation/PauseSafeModule/concrete/PauseActions.t.sol @@ -0,0 +1,142 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +// --- Test base +import {Unit_PauseSafeModule_Shared_Test} from "tests/unit/automation/PauseSafeModule/shared/Shared.t.sol"; + +contract Unit_Concrete_PauseSafeModule_PauseActions_Test is Unit_PauseSafeModule_Shared_Test { + event CapitalPauseExecuted(address indexed target); + event RebasePauseExecuted(address indexed target); + + ////////////////////////////////////////////////////// + /// --- PAUSE CAPITAL + ////////////////////////////////////////////////////// + + function test_pauseCapital_operatorTripsTheVaultFlag() public { + assertFalse(oethVault.capitalPaused()); + + vm.expectEmit(address(pauseSafeModule)); + emit CapitalPauseExecuted(address(oethVault)); + + vm.prank(operator); + pauseSafeModule.pauseCapital(address(oethVault)); + + assertTrue(oethVault.capitalPaused()); + } + + function test_pauseCapital_revertsForNonOperator() public { + vm.prank(alice); + vm.expectRevert("Caller is not an operator"); + pauseSafeModule.pauseCapital(address(oethVault)); + + assertFalse(oethVault.capitalPaused()); + } + + function test_pauseCapital_revertsForUnlistedTarget() public { + vm.prank(operator); + vm.expectRevert("Target not allowed"); + pauseSafeModule.pauseCapital(address(unlistedVault)); + + assertFalse(unlistedVault.capitalPaused()); + } + + function test_pauseCapital_revertsAfterTargetRevoked() public { + vm.prank(address(mockSafe)); + pauseSafeModule.revokeTarget(address(oethVault)); + + vm.prank(operator); + vm.expectRevert("Target not allowed"); + pauseSafeModule.pauseCapital(address(oethVault)); + + assertFalse(oethVault.capitalPaused()); + } + + /// @dev The Safe must be the vault's Strategist for the forwarded call to + /// authorize. Strip that role and the vault rejects the Safe, which the + /// module surfaces as a hard revert rather than a silent no-op. + function test_pauseCapital_revertsWhenSafeLosesStrategistRole() public { + vm.prank(governor); + oethVault.setStrategistAddr(alice); + + vm.prank(operator); + vm.expectRevert("Pause failed"); + pauseSafeModule.pauseCapital(address(oethVault)); + + assertFalse(oethVault.capitalPaused()); + } + + ////////////////////////////////////////////////////// + /// --- PAUSE REBASE + ////////////////////////////////////////////////////// + + function test_pauseRebase_operatorTripsTheVaultFlag() public { + assertFalse(oethVault.rebasePaused()); + + vm.expectEmit(address(pauseSafeModule)); + emit RebasePauseExecuted(address(oethVault)); + + vm.prank(operator); + pauseSafeModule.pauseRebase(address(oethVault)); + + assertTrue(oethVault.rebasePaused()); + } + + function test_pauseRebase_revertsForNonOperator() public { + vm.prank(alice); + vm.expectRevert("Caller is not an operator"); + pauseSafeModule.pauseRebase(address(oethVault)); + + assertFalse(oethVault.rebasePaused()); + } + + function test_pauseRebase_revertsForUnlistedTarget() public { + vm.prank(operator); + vm.expectRevert("Target not allowed"); + pauseSafeModule.pauseRebase(address(unlistedVault)); + + assertFalse(unlistedVault.rebasePaused()); + } + + ////////////////////////////////////////////////////// + /// --- SEPARATION OF PAUSE AND UNPAUSE + ////////////////////////////////////////////////////// + + /// @dev The core safety property. The module can only ever encode + /// `pauseCapital()` and `pauseRebase()`, so there is no call it can make + /// that lifts a pause. If someone later adds an unpause entry point these + /// calls stop reverting and this test fails. + function test_module_hasNoUnpauseEntryPoint() public { + string[4] memory signatures = [ + "unpauseCapital(address)", + "unpauseRebase(address)", + "unpauseCapital()", + "unpauseRebase()" + ]; + + for (uint256 i = 0; i < signatures.length; i++) { + (bool success,) = address(pauseSafeModule).call( + abi.encodeWithSelector(bytes4(keccak256(bytes(signatures[i]))), address(oethVault)) + ); + assertFalse(success, signatures[i]); + } + } + + /// @dev End to end: the module pauses, and only the Admin can lift it. The + /// Safe that hosts the module cannot undo its own pause. + function test_onlyAdminCanLiftAModulePause() public { + vm.prank(operator); + pauseSafeModule.pauseCapital(address(oethVault)); + assertTrue(oethVault.capitalPaused()); + + // The Safe hosting the module is the Strategist — it paused, but cannot unpause. + vm.prank(address(mockSafe)); + vm.expectRevert("Caller is not the Admin or Governor"); + oethVault.unpauseCapital(); + assertTrue(oethVault.capitalPaused()); + + // The Admin can. + vm.prank(guardian); + oethVault.unpauseCapital(); + assertFalse(oethVault.capitalPaused()); + } +} diff --git a/contracts/tests/unit/automation/PauseSafeModule/concrete/TargetManagement.t.sol b/contracts/tests/unit/automation/PauseSafeModule/concrete/TargetManagement.t.sol new file mode 100644 index 0000000000..2dff258b44 --- /dev/null +++ b/contracts/tests/unit/automation/PauseSafeModule/concrete/TargetManagement.t.sol @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +// --- Test base +import {Unit_PauseSafeModule_Shared_Test} from "tests/unit/automation/PauseSafeModule/shared/Shared.t.sol"; + +contract Unit_Concrete_PauseSafeModule_TargetManagement_Test is Unit_PauseSafeModule_Shared_Test { + event TargetAllowed(address indexed target); + event TargetRevoked(address indexed target); + + ////////////////////////////////////////////////////// + /// --- ALLOW TARGET + ////////////////////////////////////////////////////// + + function test_allowTarget_safeCanAllow() public { + vm.expectEmit(address(pauseSafeModule)); + emit TargetAllowed(address(unlistedVault)); + + vm.prank(address(mockSafe)); + pauseSafeModule.allowTarget(address(unlistedVault)); + + assertTrue(pauseSafeModule.isPausableTarget(address(unlistedVault))); + + // Newly allowed target is immediately pausable + vm.prank(operator); + pauseSafeModule.pauseCapital(address(unlistedVault)); + assertTrue(unlistedVault.capitalPaused()); + } + + function test_allowTarget_revertsForNonSafe() public { + vm.prank(operator); + vm.expectRevert("Caller is not the safe contract"); + pauseSafeModule.allowTarget(address(unlistedVault)); + + assertFalse(pauseSafeModule.isPausableTarget(address(unlistedVault))); + } + + function test_allowTarget_revertsForZeroAddress() public { + vm.prank(address(mockSafe)); + vm.expectRevert("Invalid target"); + pauseSafeModule.allowTarget(address(0)); + } + + function test_allowTarget_revertsWhenAlreadyAllowed() public { + vm.prank(address(mockSafe)); + vm.expectRevert("Target already allowed"); + pauseSafeModule.allowTarget(address(oethVault)); + } + + ////////////////////////////////////////////////////// + /// --- REVOKE TARGET + ////////////////////////////////////////////////////// + + function test_revokeTarget_safeCanRevoke() public { + vm.expectEmit(address(pauseSafeModule)); + emit TargetRevoked(address(oethVault)); + + vm.prank(address(mockSafe)); + pauseSafeModule.revokeTarget(address(oethVault)); + + assertFalse(pauseSafeModule.isPausableTarget(address(oethVault))); + } + + function test_revokeTarget_revertsForNonSafe() public { + vm.prank(operator); + vm.expectRevert("Caller is not the safe contract"); + pauseSafeModule.revokeTarget(address(oethVault)); + + assertTrue(pauseSafeModule.isPausableTarget(address(oethVault))); + } + + function test_revokeTarget_revertsWhenNotAllowed() public { + vm.prank(address(mockSafe)); + vm.expectRevert("Target not allowed"); + pauseSafeModule.revokeTarget(address(unlistedVault)); + } + + function test_revokeTarget_canBeReAllowed() public { + vm.startPrank(address(mockSafe)); + pauseSafeModule.revokeTarget(address(oethVault)); + pauseSafeModule.allowTarget(address(oethVault)); + vm.stopPrank(); + + assertTrue(pauseSafeModule.isPausableTarget(address(oethVault))); + } +} diff --git a/contracts/tests/unit/automation/PermissionedRebaseModule/shared/Shared.t.sol b/contracts/tests/unit/automation/PauseSafeModule/shared/Shared.t.sol similarity index 52% rename from contracts/tests/unit/automation/PermissionedRebaseModule/shared/Shared.t.sol rename to contracts/tests/unit/automation/PauseSafeModule/shared/Shared.t.sol index 4892779d5f..7cddcbcfc3 100644 --- a/contracts/tests/unit/automation/PermissionedRebaseModule/shared/Shared.t.sol +++ b/contracts/tests/unit/automation/PauseSafeModule/shared/Shared.t.sol @@ -14,29 +14,27 @@ import {Vaults} from "tests/utils/artifacts/Vaults.sol"; import {IVault} from "contracts/interfaces/IVault.sol"; import {IProxy} from "contracts/interfaces/IProxy.sol"; import {IOToken} from "contracts/interfaces/IOToken.sol"; -import {IPermissionedRebaseModule} from "contracts/interfaces/automation/IPermissionedRebaseModule.sol"; +import {IPauseSafeModule} from "contracts/interfaces/automation/IPauseSafeModule.sol"; import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; // --- Mocks import {MockERC20} from "@solmate/test/utils/mocks/MockERC20.sol"; import {MockSafeContract} from "tests/mocks/MockSafeContract.sol"; -abstract contract Unit_PermissionedRebaseModule_Shared_Test is Base { +abstract contract Unit_PauseSafeModule_Shared_Test is Base { ////////////////////////////////////////////////////// /// --- CONTRACTS & MOCKS ////////////////////////////////////////////////////// MockSafeContract internal mockSafe; - IPermissionedRebaseModule internal permissionedRebaseModule; + IPauseSafeModule internal pauseSafeModule; IOToken internal oeth; IVault internal oethVault; - ////////////////////////////////////////////////////// - /// --- CONSTANTS - ////////////////////////////////////////////////////// - - uint256 internal constant REBASE_RATE_MAX = 200e18; // 200% APR + /// @dev A second vault, deliberately left off the module's allow-list. + IOToken internal otherOeth; + IVault internal unlistedVault; ////////////////////////////////////////////////////// /// --- SETUP @@ -45,12 +43,8 @@ abstract contract Unit_PermissionedRebaseModule_Shared_Test is Base { function setUp() public virtual override { super.setUp(); - // Set a reasonable starting timestamp so rebase per-second caps work - vm.warp(7 days); - _deployContracts(); _configureContracts(); - _fundInitialUsers(); label(); } @@ -59,17 +53,22 @@ abstract contract Unit_PermissionedRebaseModule_Shared_Test is Base { weth = IERC20(address(new MockERC20("Wrapped Ether", "WETH", 18))); (oeth, oethVault) = _deployOethVault(); + (otherOeth, unlistedVault) = _deployOethVault(); + + // Only `oethVault` is allow-listed. `unlistedVault` exists so tests can + // prove the module refuses targets the Safe never approved. + address[] memory initialTargets = new address[](1); + initialTargets[0] = address(oethVault); - address[] memory initialVaults = new address[](1); - initialVaults[0] = address(oethVault); + address[] memory operators = new address[](1); + operators[0] = operator; - permissionedRebaseModule = IPermissionedRebaseModule( - vm.deployCode(Automation.PERMISSIONED_REBASE_MODULE, abi.encode(address(mockSafe), operator, initialVaults)) + pauseSafeModule = IPauseSafeModule( + vm.deployCode(Automation.PAUSE_SAFE_MODULE, abi.encode(address(mockSafe), operators, initialTargets)) ); } - /// @dev Deploy an OETH token + vault pair behind fresh proxies. Exposed so - /// tests can stand up a second vault and exercise the module's loop. + /// @dev Deploy an OETH token + vault pair behind fresh proxies. function _deployOethVault() internal returns (IOToken token, IVault vault) { vm.startPrank(deployer); @@ -96,68 +95,32 @@ abstract contract Unit_PermissionedRebaseModule_Shared_Test is Base { function _configureContracts() internal { _configureVault(oethVault); + _configureVault(unlistedVault); } - /// @dev Wire a vault the way production wires it for this module: the Safe is - /// the Strategist. `pauseRebase`/`unpauseRebase` are onlyGovernorOrStrategist - /// and `rebase` accepts the Strategist, so that single role lets the module - /// drive the whole unpause->rebase->pause sequence. The vault is then left - /// rebase-paused, which is the module's premise. + /// @dev Wire a vault the way production wires it: the Safe hosting the module + /// is the Strategist, which is what authorizes `pauseCapital`/`pauseRebase`. + /// A separate Admin holds unpause, so the module's Safe can pause but can + /// never lift what it paused — the property this module exists to preserve. + /// Both flags start unpaused so tests can observe them being tripped. function _configureVault(IVault vault) internal { vm.startPrank(governor); vault.unpauseCapital(); vault.setStrategistAddr(address(mockSafe)); - vault.setDripDuration(0); // Disable drip smoothing for instant rebase in tests - vault.setRebaseRateMax(REBASE_RATE_MAX); // Without this the per-second cap clamps yield to 0 - vault.pauseRebase(); + vault.setAdminAddr(guardian); vm.stopPrank(); } - /// @dev Give the vault a non-zero rebasing supply so a rebase can distribute yield - function _fundInitialUsers() internal { - _fundVault(oethVault); - } - - function _fundVault(IVault vault) internal { - _mintOETH(vault, matt, 100e18); - _mintOETH(vault, josh, 100e18); - } - - ////////////////////////////////////////////////////// - /// --- HELPERS - ////////////////////////////////////////////////////// - - function _dealWETH(address to, uint256 amount) internal { - MockERC20(address(weth)).mint(to, amount); - } - - function _mintOETH(IVault vault, address user, uint256 wethAmount) internal { - _dealWETH(user, wethAmount); - vm.startPrank(user); - weth.approve(address(vault), wethAmount); - vault.mint(wethAmount); - vm.stopPrank(); - } - - /// @dev Send WETH straight to the vault so `rebase()` has yield to distribute - function _injectYield(uint256 amount) internal { - _injectYield(oethVault, amount); - } - - function _injectYield(IVault vault, uint256 amount) internal { - _dealWETH(address(vault), amount); - vm.warp(block.timestamp + 1); - } - ////////////////////////////////////////////////////// /// --- LABELS ////////////////////////////////////////////////////// function label() public { vm.label(address(mockSafe), "MockSafe"); - vm.label(address(permissionedRebaseModule), "PermissionedRebaseModule"); + vm.label(address(pauseSafeModule), "PauseSafeModule"); vm.label(address(weth), "WETH"); vm.label(address(oeth), "OETH"); vm.label(address(oethVault), "OETHVault"); + vm.label(address(unlistedVault), "UnlistedVault"); } } diff --git a/contracts/tests/unit/automation/PermissionedRebaseModule/concrete/PermissionedRebase.t.sol b/contracts/tests/unit/automation/PermissionedRebaseModule/concrete/PermissionedRebase.t.sol deleted file mode 100644 index 2ac405de65..0000000000 --- a/contracts/tests/unit/automation/PermissionedRebaseModule/concrete/PermissionedRebase.t.sol +++ /dev/null @@ -1,123 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 -pragma solidity ^0.8.0; - -// --- Test base -import { - Unit_PermissionedRebaseModule_Shared_Test -} from "tests/unit/automation/PermissionedRebaseModule/shared/Shared.t.sol"; - -// --- Project imports -import {IPermissionedRebaseModule} from "contracts/interfaces/automation/IPermissionedRebaseModule.sol"; -import {IVault} from "contracts/interfaces/IVault.sol"; -import {IOToken} from "contracts/interfaces/IOToken.sol"; - -contract Unit_Concrete_PermissionedRebaseModule_PermissionedRebase_Test is Unit_PermissionedRebaseModule_Shared_Test { - ////////////////////////////////////////////////////// - /// --- PERMISSIONEDREBASE - ////////////////////////////////////////////////////// - - function test_permissionedRebase_distributesYield() public { - _injectYield(2e18); - - uint256 supplyBefore = oeth.totalSupply(); - - vm.prank(operator); - permissionedRebaseModule.permissionedRebase(); - - assertGt(oeth.totalSupply(), supplyBefore, "Rebase should have distributed yield"); - } - - /// @dev The whole point of the module: the vault must be left paused again, - /// so a partial run can never leave rebasing open. - function test_permissionedRebase_leavesVaultPaused() public { - assertTrue(oethVault.rebasePaused(), "Vault should start paused"); - - _injectYield(2e18); - - vm.prank(operator); - permissionedRebaseModule.permissionedRebase(); - - assertTrue(oethVault.rebasePaused(), "Vault must be re-paused after the rebase"); - } - - function test_permissionedRebase_emitsEvent() public { - _injectYield(2e18); - - vm.expectEmit(true, true, true, true); - emit IPermissionedRebaseModule.PermissionedRebaseExecuted(address(oethVault)); - - vm.prank(operator); - permissionedRebaseModule.permissionedRebase(); - } - - function test_permissionedRebase_withNoYield() public { - uint256 supplyBefore = oeth.totalSupply(); - - vm.prank(operator); - permissionedRebaseModule.permissionedRebase(); - - assertEq(oeth.totalSupply(), supplyBefore, "No yield means no supply change"); - assertTrue(oethVault.rebasePaused(), "Vault must still be re-paused"); - } - - function test_permissionedRebase_withNoVaults() public { - // Remove the only vault; the loop body should never run. - vm.prank(address(mockSafe)); - permissionedRebaseModule.removeVault(address(oethVault)); - - vm.prank(operator); - permissionedRebaseModule.permissionedRebase(); // Should not revert - } - - /// @dev The module loops over every registered vault. Both must be rebased - /// and both must be left paused. - function test_permissionedRebase_acrossMultipleVaults() public { - (IOToken oeth2, IVault oethVault2) = _deployOethVault(); - _configureVault(oethVault2); - _fundVault(oethVault2); - - vm.prank(address(mockSafe)); - permissionedRebaseModule.addVault(address(oethVault2)); - - _injectYield(oethVault, 2e18); - _injectYield(oethVault2, 3e18); - - uint256 supply1Before = oeth.totalSupply(); - uint256 supply2Before = oeth2.totalSupply(); - - vm.prank(operator); - permissionedRebaseModule.permissionedRebase(); - - assertGt(oeth.totalSupply(), supply1Before, "First vault should have rebased"); - assertGt(oeth2.totalSupply(), supply2Before, "Second vault should have rebased"); - - assertTrue(oethVault.rebasePaused(), "First vault must be re-paused"); - assertTrue(oethVault2.rebasePaused(), "Second vault must be re-paused"); - } - - ////////////////////////////////////////////////////// - /// --- AUTHORIZATION - ////////////////////////////////////////////////////// - - function test_permissionedRebase_RevertWhen_notOperator() public { - vm.prank(alice); - vm.expectRevert(); - permissionedRebaseModule.permissionedRebase(); - } - - ////////////////////////////////////////////////////// - /// --- ATOMICITY - ////////////////////////////////////////////////////// - - /// @dev If any sub-call fails the whole run must revert, so the vault can - /// never be left unpaused by a partial execution. - function test_permissionedRebase_RevertWhen_safeCallFails() public { - mockSafe.setShouldFail(true); - - vm.prank(operator); - vm.expectRevert("Vault call failed"); - permissionedRebaseModule.permissionedRebase(); - - assertTrue(oethVault.rebasePaused(), "Vault must remain paused after a failed run"); - } -} diff --git a/contracts/tests/unit/automation/PermissionedRebaseModule/concrete/VaultManagement.t.sol b/contracts/tests/unit/automation/PermissionedRebaseModule/concrete/VaultManagement.t.sol deleted file mode 100644 index f6d41c41b5..0000000000 --- a/contracts/tests/unit/automation/PermissionedRebaseModule/concrete/VaultManagement.t.sol +++ /dev/null @@ -1,117 +0,0 @@ -// SPDX-License-Identifier: BUSL-1.1 -pragma solidity ^0.8.0; - -// --- Test base -import { - Unit_PermissionedRebaseModule_Shared_Test -} from "tests/unit/automation/PermissionedRebaseModule/shared/Shared.t.sol"; - -// --- Project imports -import {IPermissionedRebaseModule} from "contracts/interfaces/automation/IPermissionedRebaseModule.sol"; - -contract Unit_Concrete_PermissionedRebaseModule_VaultManagement_Test is Unit_PermissionedRebaseModule_Shared_Test { - ////////////////////////////////////////////////////// - /// --- CONSTRUCTOR - ////////////////////////////////////////////////////// - - function test_constructor_registersInitialVaults() public view { - assertTrue(permissionedRebaseModule.isVaultWhitelisted(address(oethVault))); - assertEq(permissionedRebaseModule.vaults(0), address(oethVault)); - } - - ////////////////////////////////////////////////////// - /// --- ADDVAULT - ////////////////////////////////////////////////////// - - function test_addVault() public { - vm.prank(address(mockSafe)); - permissionedRebaseModule.addVault(alice); - - assertTrue(permissionedRebaseModule.isVaultWhitelisted(alice)); - assertEq(permissionedRebaseModule.vaults(1), alice); - } - - function test_addVault_emitsEvent() public { - vm.expectEmit(true, true, true, true); - emit IPermissionedRebaseModule.VaultAdded(alice); - - vm.prank(address(mockSafe)); - permissionedRebaseModule.addVault(alice); - } - - function test_addVault_RevertWhen_zeroAddress() public { - vm.prank(address(mockSafe)); - vm.expectRevert("Vault is zero address"); - permissionedRebaseModule.addVault(address(0)); - } - - function test_addVault_RevertWhen_alreadyWhitelisted() public { - vm.prank(address(mockSafe)); - vm.expectRevert("Vault already whitelisted"); - permissionedRebaseModule.addVault(address(oethVault)); - } - - function test_addVault_RevertWhen_notAdmin() public { - vm.prank(operator); - vm.expectRevert(); - permissionedRebaseModule.addVault(alice); - } - - ////////////////////////////////////////////////////// - /// --- REMOVEVAULT - ////////////////////////////////////////////////////// - - function test_removeVault() public { - vm.prank(address(mockSafe)); - permissionedRebaseModule.removeVault(address(oethVault)); - - assertFalse(permissionedRebaseModule.isVaultWhitelisted(address(oethVault))); - } - - function test_removeVault_emitsEvent() public { - vm.expectEmit(true, true, true, true); - emit IPermissionedRebaseModule.VaultRemoved(address(oethVault)); - - vm.prank(address(mockSafe)); - permissionedRebaseModule.removeVault(address(oethVault)); - } - - /// @dev removeVault swaps the last element into the removed slot, so the - /// surviving vault must still be reachable at index 0. - function test_removeVault_swapsLastIntoGap() public { - vm.startPrank(address(mockSafe)); - permissionedRebaseModule.addVault(alice); - permissionedRebaseModule.removeVault(address(oethVault)); - vm.stopPrank(); - - assertEq(permissionedRebaseModule.vaults(0), alice); - assertTrue(permissionedRebaseModule.isVaultWhitelisted(alice)); - assertFalse(permissionedRebaseModule.isVaultWhitelisted(address(oethVault))); - } - - function test_removeVault_RevertWhen_notWhitelisted() public { - vm.prank(address(mockSafe)); - vm.expectRevert("Vault not whitelisted"); - permissionedRebaseModule.removeVault(alice); - } - - function test_removeVault_RevertWhen_notAdmin() public { - vm.prank(operator); - vm.expectRevert(); - permissionedRebaseModule.removeVault(address(oethVault)); - } - - /// @dev A removed vault must no longer be driven by permissionedRebase. - function test_removeVault_stopsRebasing() public { - vm.prank(address(mockSafe)); - permissionedRebaseModule.removeVault(address(oethVault)); - - _injectYield(2e18); - uint256 supplyBefore = oeth.totalSupply(); - - vm.prank(operator); - permissionedRebaseModule.permissionedRebase(); - - assertEq(oeth.totalSupply(), supplyBefore, "Removed vault must not be rebased"); - } -} diff --git a/contracts/tests/unit/vault/OETHVault/concrete/Admin.t.sol b/contracts/tests/unit/vault/OETHVault/concrete/Admin.t.sol index c35e95fabf..cb2aa59ad9 100644 --- a/contracts/tests/unit/vault/OETHVault/concrete/Admin.t.sol +++ b/contracts/tests/unit/vault/OETHVault/concrete/Admin.t.sol @@ -383,12 +383,30 @@ contract Unit_Concrete_OETHVault_Admin_Test is Unit_OETHVault_Shared_Test { assertTrue(oethVault.rebasePaused()); } + function test_pauseRebase_byAdmin() public { + vm.prank(guardian); + oethVault.pauseRebase(); + assertTrue(oethVault.rebasePaused()); + } + function test_pauseRebase_RevertWhen_unauthorized() public { vm.prank(alice); - vm.expectRevert("Caller is not the Strategist or Governor"); + vm.expectRevert("Caller is not the Strategist, Admin or Governor"); oethVault.pauseRebase(); } + /// @dev The Strategist can pause but not unpause — a compromised Strategist + /// key cannot re-open what it closed. + function test_unpauseRebase_RevertWhen_strategist() public { + vm.prank(governor); + oethVault.pauseRebase(); + + vm.prank(strategist); + vm.expectRevert("Caller is not the Admin or Governor"); + oethVault.unpauseRebase(); + assertTrue(oethVault.rebasePaused()); + } + function test_unpauseRebase_works() public { vm.prank(governor); oethVault.pauseRebase(); diff --git a/contracts/tests/unit/vault/OETHVault/shared/Shared.t.sol b/contracts/tests/unit/vault/OETHVault/shared/Shared.t.sol index 78023968fd..84af34be71 100644 --- a/contracts/tests/unit/vault/OETHVault/shared/Shared.t.sol +++ b/contracts/tests/unit/vault/OETHVault/shared/Shared.t.sol @@ -98,6 +98,7 @@ abstract contract Unit_OETHVault_Shared_Test is Base { vm.startPrank(governor); oethVault.unpauseCapital(); oethVault.setStrategistAddr(strategist); + oethVault.setAdminAddr(guardian); // Admin multisig: the only unpauser oethVault.setMaxSupplyDiff(5e16); // 5% oethVault.setWithdrawalClaimDelay(DELAY_PERIOD); oethVault.setDripDuration(0); // Disable drip smoothing for instant rebase in tests diff --git a/contracts/tests/unit/vault/OUSDVault/concrete/Admin.t.sol b/contracts/tests/unit/vault/OUSDVault/concrete/Admin.t.sol index e760d00d4d..653a49ae98 100644 --- a/contracts/tests/unit/vault/OUSDVault/concrete/Admin.t.sol +++ b/contracts/tests/unit/vault/OUSDVault/concrete/Admin.t.sol @@ -39,9 +39,15 @@ contract Unit_Concrete_OUSDVault_Admin_Test is Unit_Shared_Test { ousdVault.pauseCapital(); } + function test_pauseCapital_admin() public { + vm.prank(guardian); + ousdVault.pauseCapital(); + assertTrue(ousdVault.capitalPaused()); + } + function test_pauseCapital_RevertWhen_unauthorized() public { vm.prank(alice); - vm.expectRevert("Caller is not the Strategist or Governor"); + vm.expectRevert("Caller is not the Strategist, Admin or Governor"); ousdVault.pauseCapital(); } @@ -54,15 +60,28 @@ contract Unit_Concrete_OUSDVault_Admin_Test is Unit_Shared_Test { assertFalse(ousdVault.capitalPaused()); } - function test_unpauseCapital_strategist() public { + function test_unpauseCapital_admin() public { vm.prank(governor); ousdVault.pauseCapital(); - vm.prank(strategist); + vm.prank(guardian); ousdVault.unpauseCapital(); assertFalse(ousdVault.capitalPaused()); } + /// @dev The Strategist can trip the capital pause but cannot lift it. That + /// asymmetry is deliberate: it stops a single compromised Strategist key + /// from re-opening the vault after an emergency pause. + function test_unpauseCapital_RevertWhen_strategist() public { + vm.prank(governor); + ousdVault.pauseCapital(); + + vm.prank(strategist); + vm.expectRevert("Caller is not the Admin or Governor"); + ousdVault.unpauseCapital(); + assertTrue(ousdVault.capitalPaused()); + } + function test_unpauseCapital_emitsEvent() public { vm.prank(governor); ousdVault.pauseCapital(); @@ -75,7 +94,7 @@ contract Unit_Concrete_OUSDVault_Admin_Test is Unit_Shared_Test { function test_unpauseCapital_RevertWhen_unauthorized() public { vm.prank(alice); - vm.expectRevert("Caller is not the Strategist or Governor"); + vm.expectRevert("Caller is not the Admin or Governor"); ousdVault.unpauseCapital(); } @@ -133,9 +152,15 @@ contract Unit_Concrete_OUSDVault_Admin_Test is Unit_Shared_Test { ousdVault.pauseRebase(); } + function test_pauseRebase_admin() public { + vm.prank(guardian); + ousdVault.pauseRebase(); + assertTrue(ousdVault.rebasePaused()); + } + function test_pauseRebase_RevertWhen_unauthorized() public { vm.prank(alice); - vm.expectRevert("Caller is not the Strategist or Governor"); + vm.expectRevert("Caller is not the Strategist, Admin or Governor"); ousdVault.pauseRebase(); } @@ -148,15 +173,25 @@ contract Unit_Concrete_OUSDVault_Admin_Test is Unit_Shared_Test { assertFalse(ousdVault.rebasePaused()); } - function test_unpauseRebase_strategist() public { + function test_unpauseRebase_admin() public { vm.prank(governor); ousdVault.pauseRebase(); - vm.prank(strategist); + vm.prank(guardian); ousdVault.unpauseRebase(); assertFalse(ousdVault.rebasePaused()); } + function test_unpauseRebase_RevertWhen_strategist() public { + vm.prank(governor); + ousdVault.pauseRebase(); + + vm.prank(strategist); + vm.expectRevert("Caller is not the Admin or Governor"); + ousdVault.unpauseRebase(); + assertTrue(ousdVault.rebasePaused()); + } + function test_unpauseRebase_emitsEvent() public { vm.prank(governor); ousdVault.pauseRebase(); @@ -169,7 +204,7 @@ contract Unit_Concrete_OUSDVault_Admin_Test is Unit_Shared_Test { function test_unpauseRebase_RevertWhen_unauthorized() public { vm.prank(alice); - vm.expectRevert("Caller is not the Strategist or Governor"); + vm.expectRevert("Caller is not the Admin or Governor"); ousdVault.unpauseRebase(); } diff --git a/contracts/tests/unit/vault/OUSDVault/shared/Shared.t.sol b/contracts/tests/unit/vault/OUSDVault/shared/Shared.t.sol index bc11dee5d2..801d679d78 100644 --- a/contracts/tests/unit/vault/OUSDVault/shared/Shared.t.sol +++ b/contracts/tests/unit/vault/OUSDVault/shared/Shared.t.sol @@ -100,6 +100,7 @@ abstract contract Unit_Shared_Test is Base { vm.startPrank(governor); ousdVault.unpauseCapital(); ousdVault.setStrategistAddr(strategist); + ousdVault.setAdminAddr(guardian); // Admin multisig: the only unpauser ousdVault.setMaxSupplyDiff(5e16); // 5% ousdVault.setWithdrawalClaimDelay(DELAY_PERIOD); ousdVault.setDripDuration(0); // Disable drip smoothing for instant rebase in tests diff --git a/contracts/tests/utils/artifacts/Automation.sol b/contracts/tests/utils/artifacts/Automation.sol index 43263423cc..431562386c 100644 --- a/contracts/tests/utils/artifacts/Automation.sol +++ b/contracts/tests/utils/artifacts/Automation.sol @@ -18,6 +18,6 @@ library Automation { "contracts/automation/EthereumBridgeHelperModule.sol:EthereumBridgeHelperModule"; string internal constant MERKL_POOL_BOOSTER_BRIBES_MODULE = "contracts/automation/MerklPoolBoosterBribesModule.sol:MerklPoolBoosterBribesModule"; - string internal constant PERMISSIONED_REBASE_MODULE = - "contracts/automation/PermissionedRebaseModule.sol:PermissionedRebaseModule"; + string internal constant PAUSE_SAFE_MODULE = + "contracts/automation/PauseSafeModule.sol:PauseSafeModule"; } From 225934d7dcd43e92d544fab4214a23908d67ca8e Mon Sep 17 00:00:00 2001 From: Shahul Hameed <10547529+shahthepro@users.noreply.github.com> Date: Thu, 13 Aug 2026 17:06:51 +0400 Subject: [PATCH 3/5] Add ARM pausing to the module --- .../contracts/automation/PauseSafeModule.sol | 40 +++++++++--- .../automation/IPauseSafeModule.sol | 3 + .../deploy/mainnet/203_pause_safe_module.js | 8 +++ contracts/tests/mocks/MockPausableARM.sol | 33 ++++++++++ .../concrete/PauseActions.t.sol | 64 +++++++++++++++++++ .../PauseSafeModule/shared/Shared.t.sol | 17 ++++- 6 files changed, 152 insertions(+), 13 deletions(-) create mode 100644 contracts/tests/mocks/MockPausableARM.sol diff --git a/contracts/contracts/automation/PauseSafeModule.sol b/contracts/contracts/automation/PauseSafeModule.sol index 401c0a2ae7..06cd711e91 100644 --- a/contracts/contracts/automation/PauseSafeModule.sol +++ b/contracts/contracts/automation/PauseSafeModule.sol @@ -10,19 +10,20 @@ import { IVault } from "../interfaces/IVault.sol"; * @notice Gnosis Safe module that lets a threat-detection operator pause OToken * vaults without waiting for multisig signatures to be gathered. * - * @dev The Safe hosting this module is the Guardian multisig, which is the - * vaults' `strategistAddr` and therefore already authorized to pause. This - * module does not widen that authority — it only lets a keyed operator - * exercise it without collecting signatures. + * @dev The Safe hosting this module is the Guardian multisig, which is already + * authorized to pause every supported target: it is the vaults' + * `strategistAddr`, and the ARMs' `guardian`. This module does not widen + * that authority — it only lets a keyed operator exercise it without + * collecting signatures. * * Safety properties, in order of importance: * - * 1. This module can never unpause. The only two selectors it can encode - * are `pauseCapital()` and `pauseRebase()`, both compiled in below. - * That is a property of the bytecode, not of configuration — there is - * no allow-list entry or role that could turn an unpause into a legal - * call. Unpausing requires the Admin multisig acting directly on the - * vault. + * 1. This module can never unpause. The only three selectors it can encode + * are `pauseCapital()`, `pauseRebase()` and `pause()`, all compiled in + * below. That is a property of the bytecode, not of configuration — + * there is no allow-list entry or role that could turn an unpause into + * a legal call. Unpausing requires the Admin multisig acting directly + * on the target. * 2. Targets are allow-listed by the Safe, so a compromised operator key * cannot aim a pause at an arbitrary contract. * 3. Pause failures revert. A pause that silently did not land is worse @@ -33,6 +34,14 @@ import { IVault } from "../interfaces/IVault.sol"; * do anything at all. */ contract PauseSafeModule is AbstractSafeModule { + /// @dev `bytes4(keccak256("pause()"))`. Not taken from a project interface + /// because it is shared by contracts that have none in common: the ARMs + /// (`AbstractARM.pause()`, guarded by `onlyPauser`, which includes the + /// Guardian Safe hosting this module) and the native staking strategies. + /// Any target exposing a no-argument `pause()` is reachable through + /// `pause(address)` below. + bytes4 internal constant PAUSE_SELECTOR = 0x8456cb59; + /// @notice Contracts this module is permitted to pause. mapping(address => bool) public isPausableTarget; @@ -40,6 +49,7 @@ contract PauseSafeModule is AbstractSafeModule { event TargetRevoked(address indexed target); event CapitalPauseExecuted(address indexed target); event RebasePauseExecuted(address indexed target); + event PauseExecuted(address indexed target); /** * @param _safeContract Address of the Gnosis Safe (Guardian multisig). @@ -80,6 +90,16 @@ contract PauseSafeModule is AbstractSafeModule { emit RebasePauseExecuted(_target); } + /** + * @notice Halt an allow-listed contract that exposes a no-argument `pause()` + * — the ARMs, and the native staking strategies. + * @param _target Contract to pause. + */ + function pause(address _target) external onlyOperator { + _execPause(_target, PAUSE_SELECTOR); + emit PauseExecuted(_target); + } + /// @dev Execute a pause selector on `_target` through the Safe. function _execPause(address _target, bytes4 _selector) internal { require(isPausableTarget[_target], "Target not allowed"); diff --git a/contracts/contracts/interfaces/automation/IPauseSafeModule.sol b/contracts/contracts/interfaces/automation/IPauseSafeModule.sol index 7ae0c635db..7a35a368f5 100644 --- a/contracts/contracts/interfaces/automation/IPauseSafeModule.sol +++ b/contracts/contracts/interfaces/automation/IPauseSafeModule.sol @@ -8,6 +8,7 @@ interface IPauseSafeModule is IAbstractSafeModule { event TargetRevoked(address indexed target); event CapitalPauseExecuted(address indexed target); event RebasePauseExecuted(address indexed target); + event PauseExecuted(address indexed target); function isPausableTarget(address target) external view returns (bool); @@ -15,6 +16,8 @@ interface IPauseSafeModule is IAbstractSafeModule { function pauseRebase(address _target) external; + function pause(address _target) external; + function allowTarget(address _target) external; function revokeTarget(address _target) external; diff --git a/contracts/deploy/mainnet/203_pause_safe_module.js b/contracts/deploy/mainnet/203_pause_safe_module.js index 23c4175db4..ce8013dc53 100644 --- a/contracts/deploy/mainnet/203_pause_safe_module.js +++ b/contracts/deploy/mainnet/203_pause_safe_module.js @@ -48,6 +48,14 @@ REQUIRED POST-DEPLOY STEPS — the module does nothing until these are done Step 1 is the one that was silently skipped for PermissionedRebaseModule, leaving it dead on-chain for months. Do not close this PR until it is done. + + LATER — ARM targets. The module can already drive AbstractARM.pause() via + pause(address), but the ARMs are deliberately NOT allow-listed here. They + only become pausable by this Safe once arm-oeth PR #337 ships and each ARM + has been upgraded and had setPauseRoles(2/8, 5/8) called. After that, the + Safe adds each one with: + pauseSafeModule.allowTarget() + Allow-listing them before then would look configured but revert on use. ======================================================================= `); diff --git a/contracts/tests/mocks/MockPausableARM.sol b/contracts/tests/mocks/MockPausableARM.sol new file mode 100644 index 0000000000..d09fc7974e --- /dev/null +++ b/contracts/tests/mocks/MockPausableARM.sol @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +/// @notice Minimal stand-in for `AbstractARM`'s pause surface, mirroring the split +/// introduced in arm-oeth PR #337: `pause()` accepts the guardian (the Safe +/// that hosts the pause module), `unpause()` accepts only the admin multisig. +/// Enough to prove the module can trip an ARM-shaped target and cannot lift it. +contract MockPausableARM { + bool public paused; + + address public guardian; + address public adminMultisig; + + error OnlyPauser(); + error OnlyUnpauser(); + + constructor(address _guardian, address _adminMultisig) { + guardian = _guardian; + adminMultisig = _adminMultisig; + } + + function pause() external { + if (msg.sender != guardian && msg.sender != adminMultisig) { + revert OnlyPauser(); + } + paused = true; + } + + function unpause() external { + if (msg.sender != adminMultisig) revert OnlyUnpauser(); + paused = false; + } +} diff --git a/contracts/tests/unit/automation/PauseSafeModule/concrete/PauseActions.t.sol b/contracts/tests/unit/automation/PauseSafeModule/concrete/PauseActions.t.sol index aefca9a704..7700028459 100644 --- a/contracts/tests/unit/automation/PauseSafeModule/concrete/PauseActions.t.sol +++ b/contracts/tests/unit/automation/PauseSafeModule/concrete/PauseActions.t.sol @@ -4,9 +4,13 @@ pragma solidity ^0.8.0; // --- Test base import {Unit_PauseSafeModule_Shared_Test} from "tests/unit/automation/PauseSafeModule/shared/Shared.t.sol"; +// --- Mocks +import {MockPausableARM} from "tests/mocks/MockPausableARM.sol"; + contract Unit_Concrete_PauseSafeModule_PauseActions_Test is Unit_PauseSafeModule_Shared_Test { event CapitalPauseExecuted(address indexed target); event RebasePauseExecuted(address indexed target); + event PauseExecuted(address indexed target); ////////////////////////////////////////////////////// /// --- PAUSE CAPITAL @@ -97,6 +101,66 @@ contract Unit_Concrete_PauseSafeModule_PauseActions_Test is Unit_PauseSafeModule assertFalse(unlistedVault.rebasePaused()); } + ////////////////////////////////////////////////////// + /// --- PAUSE (ARM-shaped targets) + ////////////////////////////////////////////////////// + + function test_pause_operatorTripsAnArm() public { + assertFalse(arm.paused()); + + vm.expectEmit(address(pauseSafeModule)); + emit PauseExecuted(address(arm)); + + vm.prank(operator); + pauseSafeModule.pause(address(arm)); + + assertTrue(arm.paused()); + } + + function test_pause_revertsForNonOperator() public { + vm.prank(alice); + vm.expectRevert("Caller is not an operator"); + pauseSafeModule.pause(address(arm)); + + assertFalse(arm.paused()); + } + + function test_pause_revertsForUnlistedTarget() public { + vm.prank(operator); + vm.expectRevert("Target not allowed"); + pauseSafeModule.pause(address(unlistedVault)); + } + + /// @dev The vault selectors do not exist on an ARM and vice versa. Aiming the + /// wrong entry point at a target fails loudly rather than doing something + /// unexpected. + function test_pause_mismatchedSelectorReverts() public { + vm.prank(operator); + vm.expectRevert("Pause failed"); + pauseSafeModule.pauseCapital(address(arm)); + + vm.prank(operator); + vm.expectRevert("Pause failed"); + pauseSafeModule.pause(address(oethVault)); + } + + /// @dev Same separation as the vaults, on an ARM: the Safe hosting the module + /// is the ARM guardian, so it can pause but the ARM rejects its unpause. + function test_onlyAdminCanLiftAnArmPause() public { + vm.prank(operator); + pauseSafeModule.pause(address(arm)); + assertTrue(arm.paused()); + + vm.prank(address(mockSafe)); + vm.expectRevert(MockPausableARM.OnlyUnpauser.selector); + arm.unpause(); + assertTrue(arm.paused()); + + vm.prank(guardian); + arm.unpause(); + assertFalse(arm.paused()); + } + ////////////////////////////////////////////////////// /// --- SEPARATION OF PAUSE AND UNPAUSE ////////////////////////////////////////////////////// diff --git a/contracts/tests/unit/automation/PauseSafeModule/shared/Shared.t.sol b/contracts/tests/unit/automation/PauseSafeModule/shared/Shared.t.sol index 7cddcbcfc3..14d37e9af7 100644 --- a/contracts/tests/unit/automation/PauseSafeModule/shared/Shared.t.sol +++ b/contracts/tests/unit/automation/PauseSafeModule/shared/Shared.t.sol @@ -20,6 +20,7 @@ import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; // --- Mocks import {MockERC20} from "@solmate/test/utils/mocks/MockERC20.sol"; import {MockSafeContract} from "tests/mocks/MockSafeContract.sol"; +import {MockPausableARM} from "tests/mocks/MockPausableARM.sol"; abstract contract Unit_PauseSafeModule_Shared_Test is Base { ////////////////////////////////////////////////////// @@ -36,6 +37,10 @@ abstract contract Unit_PauseSafeModule_Shared_Test is Base { IOToken internal otherOeth; IVault internal unlistedVault; + /// @dev An ARM-shaped target: no-argument `pause()`, guardian can pause, + /// only the admin multisig can unpause. + MockPausableARM internal arm; + ////////////////////////////////////////////////////// /// --- SETUP ////////////////////////////////////////////////////// @@ -55,10 +60,15 @@ abstract contract Unit_PauseSafeModule_Shared_Test is Base { (oeth, oethVault) = _deployOethVault(); (otherOeth, unlistedVault) = _deployOethVault(); - // Only `oethVault` is allow-listed. `unlistedVault` exists so tests can - // prove the module refuses targets the Safe never approved. - address[] memory initialTargets = new address[](1); + // The Safe hosting the module is the ARM's guardian, exactly as + // arm-oeth deploy script 043 wires it on mainnet. + arm = new MockPausableARM(address(mockSafe), guardian); + + // `unlistedVault` is left off so tests can prove the module refuses + // targets the Safe never approved. + address[] memory initialTargets = new address[](2); initialTargets[0] = address(oethVault); + initialTargets[1] = address(arm); address[] memory operators = new address[](1); operators[0] = operator; @@ -122,5 +132,6 @@ abstract contract Unit_PauseSafeModule_Shared_Test is Base { vm.label(address(oeth), "OETH"); vm.label(address(oethVault), "OETHVault"); vm.label(address(unlistedVault), "UnlistedVault"); + vm.label(address(arm), "MockPausableARM"); } } From 7eb146b1c20a7c0bf6c2ac3420ba1f9623423f07 Mon Sep 17 00:00:00 2001 From: Shahul Hameed <10547529+shahthepro@users.noreply.github.com> Date: Fri, 14 Aug 2026 14:37:02 +0400 Subject: [PATCH 4/5] Switch to Foundry and address CR comments --- .../contracts/automation/PauseSafeModule.sol | 26 ++-- contracts/contracts/vault/VaultAdmin.sol | 4 + .../deploy/base/055_vault_admin_unpause.js | 40 ------ .../deploy/base/056_pause_safe_module.js | 63 --------- .../deploy/mainnet/202_vault_admin_unpause.js | 69 ---------- .../deploy/mainnet/203_pause_safe_module.js | 80 ----------- .../deploy/base/001_VaultAdminRole.s.sol | 82 +++++++++++ .../deploy/base/002_PauseSafeModule.s.sol | 77 +++++++++++ .../deploy/mainnet/005_VaultAdminRole.s.sol | 97 +++++++++++++ .../deploy/mainnet/006_PauseSafeModule.s.sol | 82 +++++++++++ contracts/test/_fixture-base.js | 5 +- contracts/test/_fixture.js | 12 +- .../pause-module.base.fork-test.js | 70 ---------- .../pause-module.mainnet.fork-test.js | 93 ------------- contracts/test/vault/index.js | 11 ++ .../test/vault/oethb-vault.base.fork-test.js | 44 ------ contracts/test/vault/rebase.js | 5 + .../test/vault/vault.mainnet.fork-test.js | 52 ------- contracts/tests/smoke/BaseSmoke.t.sol | 12 ++ .../concrete/PauseSafeModule.t.sol | 93 +++++++++++++ .../PauseSafeModule/shared/Shared.t.sol | 40 ++++++ .../vault/OETHBaseVault/concrete/Admin.t.sol | 63 +++++++++ .../concrete/PauseSafeModule.t.sol | 127 ++++++++++++++++++ .../PauseSafeModule/shared/Shared.t.sol | 43 ++++++ .../vault/OETHVault/concrete/Admin.t.sol | 63 +++++++++ .../vault/OUSDVault/concrete/Admin.t.sol | 63 +++++++++ .../concrete/Constructor.t.sol | 12 +- .../concrete/PauseActions.t.sol | 13 +- .../concrete/TargetManagement.t.sol | 14 +- .../unit/vault/OETHVault/concrete/Admin.t.sol | 34 +++++ .../unit/vault/OUSDVault/concrete/Admin.t.sol | 34 +++++ contracts/tests/utils/Addresses.sol | 3 + contracts/utils/addresses.js | 5 +- 33 files changed, 989 insertions(+), 542 deletions(-) delete mode 100644 contracts/deploy/base/055_vault_admin_unpause.js delete mode 100644 contracts/deploy/base/056_pause_safe_module.js delete mode 100644 contracts/deploy/mainnet/202_vault_admin_unpause.js delete mode 100644 contracts/deploy/mainnet/203_pause_safe_module.js create mode 100644 contracts/scripts/deploy/base/001_VaultAdminRole.s.sol create mode 100644 contracts/scripts/deploy/base/002_PauseSafeModule.s.sol create mode 100644 contracts/scripts/deploy/mainnet/005_VaultAdminRole.s.sol create mode 100644 contracts/scripts/deploy/mainnet/006_PauseSafeModule.s.sol delete mode 100644 contracts/test/safe-modules/pause-module.base.fork-test.js delete mode 100644 contracts/test/safe-modules/pause-module.mainnet.fork-test.js create mode 100644 contracts/tests/smoke/base/automation/PauseSafeModule/concrete/PauseSafeModule.t.sol create mode 100644 contracts/tests/smoke/base/automation/PauseSafeModule/shared/Shared.t.sol create mode 100644 contracts/tests/smoke/base/vault/OETHBaseVault/concrete/Admin.t.sol create mode 100644 contracts/tests/smoke/mainnet/automation/PauseSafeModule/concrete/PauseSafeModule.t.sol create mode 100644 contracts/tests/smoke/mainnet/automation/PauseSafeModule/shared/Shared.t.sol create mode 100644 contracts/tests/smoke/mainnet/vault/OETHVault/concrete/Admin.t.sol create mode 100644 contracts/tests/smoke/mainnet/vault/OUSDVault/concrete/Admin.t.sol diff --git a/contracts/contracts/automation/PauseSafeModule.sol b/contracts/contracts/automation/PauseSafeModule.sol index 06cd711e91..5c7ba6e83d 100644 --- a/contracts/contracts/automation/PauseSafeModule.sol +++ b/contracts/contracts/automation/PauseSafeModule.sol @@ -28,18 +28,19 @@ import { IVault } from "../interfaces/IVault.sol"; * cannot aim a pause at an arbitrary contract. * 3. Pause failures revert. A pause that silently did not land is worse * than a loud failure, because the detection service would treat the - * protocol as contained when it is not. + * protocol as contained when it is not. Targets are checked for code at + * allow-list time for the same reason: a Safe module call to a codeless + * address reports success, so an EOA target would pause nothing while + * looking like it had. * * The Safe must call `enableModule(address(this))` before this module can * do anything at all. */ contract PauseSafeModule is AbstractSafeModule { - /// @dev `bytes4(keccak256("pause()"))`. Not taken from a project interface - /// because it is shared by contracts that have none in common: the ARMs - /// (`AbstractARM.pause()`, guarded by `onlyPauser`, which includes the - /// Guardian Safe hosting this module) and the native staking strategies. - /// Any target exposing a no-argument `pause()` is reachable through - /// `pause(address)` below. + /// @dev `bytes4(keccak256("pause()"))`. Hardcoded rather than taken from a + /// project interface because the target lives in another repo: this is + /// `AbstractARM.pause()`, guarded by `onlyPauser`, which includes the + /// Guardian Safe hosting this module. bytes4 internal constant PAUSE_SELECTOR = 0x8456cb59; /// @notice Contracts this module is permitted to pause. @@ -91,9 +92,8 @@ contract PauseSafeModule is AbstractSafeModule { } /** - * @notice Halt an allow-listed contract that exposes a no-argument `pause()` - * — the ARMs, and the native staking strategies. - * @param _target Contract to pause. + * @notice Halt an allow-listed ARM, which exposes a no-argument `pause()`. + * @param _target ARM to pause. */ function pause(address _target) external onlyOperator { _execPause(_target, PAUSE_SELECTOR); @@ -123,7 +123,11 @@ contract PauseSafeModule is AbstractSafeModule { } function _allowTarget(address _target) internal { - require(_target != address(0), "Invalid target"); + // A Safe module call to a codeless address succeeds, so an EOA or a + // mistyped address here would make `_execPause` report a pause that + // never happened. Checked at allow-list time rather than on the + // latency-critical pause path. Also covers address(0). + require(_target.code.length > 0, "Target has no code"); require(!isPausableTarget[_target], "Target already allowed"); isPausableTarget[_target] = true; emit TargetAllowed(_target); diff --git a/contracts/contracts/vault/VaultAdmin.sol b/contracts/contracts/vault/VaultAdmin.sol index 750ee6c1a8..0213b4736c 100644 --- a/contracts/contracts/vault/VaultAdmin.sol +++ b/contracts/contracts/vault/VaultAdmin.sol @@ -111,6 +111,10 @@ abstract contract VaultAdmin is VaultCore { * @param _admin New Admin address. */ function setAdminAddr(address _admin) external onlyGovernor { + // Unlike the Strategist and Operator, the zero address is not a useful + // way to disable this role: it would silently leave the Governor as the + // only account able to lift a pause. + require(_admin != address(0), "Invalid admin"); adminAddr = _admin; emit AdminUpdated(_admin); } diff --git a/contracts/deploy/base/055_vault_admin_unpause.js b/contracts/deploy/base/055_vault_admin_unpause.js deleted file mode 100644 index 95059e731f..0000000000 --- a/contracts/deploy/base/055_vault_admin_unpause.js +++ /dev/null @@ -1,40 +0,0 @@ -const { deployOnBase } = require("../../utils/deploy-l2"); -const { deployWithConfirmation } = require("../../utils/deploy"); -const addresses = require("../../utils/addresses"); - -module.exports = deployOnBase( - { - deployName: "055_vault_admin_unpause", - }, - async ({ ethers }) => { - // 1. Deploy new OETHBaseVault implementation - const dOETHbVault = await deployWithConfirmation( - "OETHBaseVault", - [addresses.base.WETH], - "OETHBaseVault", - true - ); - - const cOETHbVaultProxy = await ethers.getContract("OETHBaseVaultProxy"); - const cOETHbVault = await ethers.getContractAt( - "IVault", - cOETHbVaultProxy.address - ); - - return { - name: "Upgrade OETHBaseVault: Admin can pause, only Admin can unpause", - actions: [ - { - contract: cOETHbVaultProxy, - signature: "upgradeTo(address)", - args: [dOETHbVault.address], - }, - { - contract: cOETHbVault, - signature: "setAdminAddr(address)", - args: [addresses.base.admin], - }, - ], - }; - } -); diff --git a/contracts/deploy/base/056_pause_safe_module.js b/contracts/deploy/base/056_pause_safe_module.js deleted file mode 100644 index e518a4d366..0000000000 --- a/contracts/deploy/base/056_pause_safe_module.js +++ /dev/null @@ -1,63 +0,0 @@ -const { deployOnBase } = require("../../utils/deploy-l2"); -const addresses = require("../../utils/addresses"); -const { isFork } = require("../../utils/hardhat-helpers"); -const { impersonateAndFund } = require("../../utils/signers"); - -module.exports = deployOnBase( - { - deployName: "056_pause_safe_module", - }, - async ({ deployWithConfirmation, withConfirmation }) => { - const safeAddress = addresses.multichainStrategist; - - const cOETHbVaultProxy = await ethers.getContract("OETHBaseVaultProxy"); - - // The Hypernative keeper address is not known yet. Deploy with the Talos - // relayer as the initial operator; the Safe grants OPERATOR_ROLE to the - // keeper once the vendor supplies its address (see checklist below). - const operators = [addresses.talosRelayer]; - - await deployWithConfirmation("PauseSafeModule", [ - safeAddress, - operators, - [cOETHbVaultProxy.address], - ]); - const cPauseSafeModule = await ethers.getContract("PauseSafeModule"); - - console.log( - `PauseSafeModule (for ${safeAddress}) deployed to`, - cPauseSafeModule.address - ); - console.log(` -======================================================================= -REQUIRED POST-DEPLOY STEPS — the module does nothing until these are done -======================================================================= - 1. Guardian Safe (${safeAddress}) calls: - enableModule(${cPauseSafeModule.address}) - 2. Verify on-chain: - safe.isModuleEnabled(${cPauseSafeModule.address}) == true - 3. Guardian Safe grants the Hypernative keeper the operator role: - pauseSafeModule.grantRole(OPERATOR_ROLE, ) - 4. Smoke-test one pauseCapital() on a fork before arming detection. -======================================================================= -`); - - if (isFork) { - const safeSigner = await impersonateAndFund(safeAddress); - const cSafe = await ethers.getContractAt( - ["function enableModule(address module) external"], - safeAddress - ); - - await withConfirmation( - cSafe.connect(safeSigner).enableModule(cPauseSafeModule.address) - ); - - console.log("Enabled PauseSafeModule on fork"); - } - - return { - actions: [], - }; - } -); diff --git a/contracts/deploy/mainnet/202_vault_admin_unpause.js b/contracts/deploy/mainnet/202_vault_admin_unpause.js deleted file mode 100644 index e58a6ab6ce..0000000000 --- a/contracts/deploy/mainnet/202_vault_admin_unpause.js +++ /dev/null @@ -1,69 +0,0 @@ -const addresses = require("../../utils/addresses"); -const { deploymentWithGovernanceProposal } = require("../../utils/deploy"); - -module.exports = deploymentWithGovernanceProposal( - { - deployName: "202_vault_admin_unpause", - forceDeploy: false, - reduceQueueTime: true, - deployerIsProposer: false, - }, - async ({ deployWithConfirmation, ethers }) => { - // 1. Deploy new OUSD Vault implementation - const dOUSDVault = await deployWithConfirmation( - "OUSDVault", - [addresses.mainnet.USDC], - undefined, - true - ); - - // 2. Deploy new OETH Vault implementation - const dOETHVault = await deployWithConfirmation( - "OETHVault", - [addresses.mainnet.WETH], - undefined, - true - ); - - const cVaultProxy = await ethers.getContract("VaultProxy"); - const cOUSDVault = await ethers.getContractAt( - "IVault", - cVaultProxy.address - ); - - const cOETHVaultProxy = await ethers.getContract("OETHVaultProxy"); - const cOETHVault = await ethers.getContractAt( - "IVault", - cOETHVaultProxy.address - ); - - // The Admin (5/8) multisig. Stored in addresses.js as `Guardian`. - const adminAddr = addresses.mainnet.Guardian; - - return { - name: "Upgrade OUSD and OETH vaults: Admin can pause, only Admin can unpause", - actions: [ - { - contract: cVaultProxy, - signature: "upgradeTo(address)", - args: [dOUSDVault.address], - }, - { - contract: cOUSDVault, - signature: "setAdminAddr(address)", - args: [adminAddr], - }, - { - contract: cOETHVaultProxy, - signature: "upgradeTo(address)", - args: [dOETHVault.address], - }, - { - contract: cOETHVault, - signature: "setAdminAddr(address)", - args: [adminAddr], - }, - ], - }; - } -); diff --git a/contracts/deploy/mainnet/203_pause_safe_module.js b/contracts/deploy/mainnet/203_pause_safe_module.js deleted file mode 100644 index ce8013dc53..0000000000 --- a/contracts/deploy/mainnet/203_pause_safe_module.js +++ /dev/null @@ -1,80 +0,0 @@ -const addresses = require("../../utils/addresses"); -const { - deploymentWithGovernanceProposal, - deployWithConfirmation, - withConfirmation, -} = require("../../utils/deploy"); -const { isFork } = require("../../utils/hardhat-helpers"); -const { impersonateAndFund } = require("../../utils/signers"); - -module.exports = deploymentWithGovernanceProposal( - { - deployName: "203_pause_safe_module", - forceDeploy: false, - reduceQueueTime: true, - deployerIsProposer: false, - proposalId: "", - }, - async () => { - const safeAddress = addresses.multichainStrategist; - - const cVaultProxy = await ethers.getContract("VaultProxy"); - const cOETHVaultProxy = await ethers.getContract("OETHVaultProxy"); - - // The Hypernative keeper address is not known yet. Deploy with the Talos - // relayer as the initial operator; the Safe grants OPERATOR_ROLE to the - // keeper once the vendor supplies its address (see checklist below). - const operators = [addresses.talosRelayer]; - - await deployWithConfirmation("PauseSafeModule", [ - safeAddress, - operators, - [cVaultProxy.address, cOETHVaultProxy.address], - ]); - const cPauseSafeModule = await ethers.getContract("PauseSafeModule"); - - console.log(`PauseSafeModule deployed to ${cPauseSafeModule.address}`); - console.log(` -======================================================================= -REQUIRED POST-DEPLOY STEPS — the module does nothing until these are done -======================================================================= - 1. Guardian Safe (${safeAddress}) calls: - enableModule(${cPauseSafeModule.address}) - 2. Verify on-chain: - safe.isModuleEnabled(${cPauseSafeModule.address}) == true - 3. Guardian Safe grants the Hypernative keeper the operator role: - pauseSafeModule.grantRole(OPERATOR_ROLE, ) - 4. Smoke-test one pauseCapital() on a fork before arming detection. - - Step 1 is the one that was silently skipped for PermissionedRebaseModule, - leaving it dead on-chain for months. Do not close this PR until it is done. - - LATER — ARM targets. The module can already drive AbstractARM.pause() via - pause(address), but the ARMs are deliberately NOT allow-listed here. They - only become pausable by this Safe once arm-oeth PR #337 ships and each ARM - has been upgraded and had setPauseRoles(2/8, 5/8) called. After that, the - Safe adds each one with: - pauseSafeModule.allowTarget() - Allow-listing them before then would look configured but revert on use. -======================================================================= -`); - - if (isFork) { - const safeSigner = await impersonateAndFund(safeAddress); - const cSafe = await ethers.getContractAt( - ["function enableModule(address module) external"], - safeAddress - ); - - await withConfirmation( - cSafe.connect(safeSigner).enableModule(cPauseSafeModule.address) - ); - - console.log("Enabled PauseSafeModule on fork"); - } - - return { - actions: [], - }; - } -); diff --git a/contracts/scripts/deploy/base/001_VaultAdminRole.s.sol b/contracts/scripts/deploy/base/001_VaultAdminRole.s.sol new file mode 100644 index 0000000000..ad48b79d50 --- /dev/null +++ b/contracts/scripts/deploy/base/001_VaultAdminRole.s.sol @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +// Deployment framework +import {AbstractDeployScript} from "scripts/deploy/helpers/AbstractDeployScript.s.sol"; +import {GovHelper} from "scripts/deploy/helpers/GovHelper.sol"; +import {GovProposal} from "scripts/deploy/helpers/DeploymentTypes.sol"; + +// Contracts +import {IVault} from "contracts/interfaces/IVault.sol"; +import {OETHBaseVault} from "contracts/vault/OETHBaseVault.sol"; +import {InitializeGovernedUpgradeabilityProxy} from "contracts/proxies/InitializeGovernedUpgradeabilityProxy.sol"; + +// Addresses +import {Base as BaseAddresses, CrossChain} from "tests/utils/Addresses.sol"; + +/// @title 001_VaultAdminRole +/// @notice Gives the Super OETH vault an Admin role that holds the sole right to unpause. +/// @dev Base counterpart of mainnet 005_VaultAdminRole. Before this upgrade the Strategist could +/// both pause and unpause, so a single compromised key could trip a pause and immediately lift +/// it. The new `adminAddr` slot splits the two: `pauseCapital`/`pauseRebase` stay open to the +/// Strategist (the 2/8 Guardian Safe), while `unpauseCapital`/`unpauseRebase` become +/// Admin-or-Governor only. +/// +/// Governance on Base runs through the TimelockController, with the 5/8 as its proposer and +/// executor — GovHelper handles that. The timelock operation id is salted with +/// keccak256(description), so the description below must not change between scheduling and +/// executing or the scheduled operation is orphaned. +contract $001_VaultAdminRole is AbstractDeployScript("001_VaultAdminRole") { + using GovHelper for GovProposal; + + // ==================== Deployment Logic ==================== // + + function _execute() internal override { + OETHBaseVault oethbVaultImpl = new OETHBaseVault(BaseAddresses.WETH); + _recordDeployment("OETHBASE_VAULT_IMPL", address(oethbVaultImpl)); + } + + // ==================== Governance Proposal ==================== // + + function _buildGovernanceProposal() internal override { + address oethbVaultProxy = resolver.resolve("OETHBASE_VAULT_PROXY"); + + govProposal.setDescription( + "Upgrade OETHBaseVault: Admin can pause, only Admin can unpause\n\n" + "Adds an Admin role to the Super OETH vault and points it at the 5/8 multisig. Pausing " + "capital and rebasing stays available to the Strategist, the Admin and the Governor; " + "unpausing is narrowed to the Admin and the Governor, so the Strategist can no longer " + "lift a pause it triggered. The vault is upgraded and has its Admin set in this same " + "proposal, so it is never left with an unset Admin." + ); + + govProposal.action(oethbVaultProxy, "upgradeTo(address)", abi.encode(resolver.resolve("OETHBASE_VAULT_IMPL"))); + govProposal.action(oethbVaultProxy, "setAdminAddr(address)", abi.encode(BaseAddresses.admin)); + } + + // ==================== Fork Verification ==================== // + + /// @dev Read-only. Behaviour of the new role split is covered by the smoke tests under + /// tests/smoke/base/vault; this only proves the deploy landed. + function _fork() internal override { + address vaultProxy = resolver.resolve("OETHBASE_VAULT_PROXY"); + address expectedImpl = resolver.resolve("OETHBASE_VAULT_IMPL"); + + require( + InitializeGovernedUpgradeabilityProxy(payable(vaultProxy)).implementation() == expectedImpl, + "Vault implementation not updated" + ); + + IVault vault = IVault(vaultProxy); + require(vault.adminAddr() == BaseAddresses.admin, "Vault admin not set"); + + // `adminAddr` was carved out of the storage gap, immediately after `defaultStrategy` and + // `operatorAddr`. Those two neighbours are where a layout shift would surface first. + require(vault.defaultStrategy() != address(0), "Default strategy cleared"); + require(vault.operatorAddr() == CrossChain.talosRelayer, "Operator changed"); + + // The pause module in 002 drives this vault through the Strategist Safe, so the pause path + // is only wired if the Safe still holds that role. + require(vault.strategistAddr() == CrossChain.multichainStrategist, "Strategist changed"); + } +} diff --git a/contracts/scripts/deploy/base/002_PauseSafeModule.s.sol b/contracts/scripts/deploy/base/002_PauseSafeModule.s.sol new file mode 100644 index 0000000000..87672165e5 --- /dev/null +++ b/contracts/scripts/deploy/base/002_PauseSafeModule.s.sol @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +// Deployment framework +import {AbstractDeployScript} from "scripts/deploy/helpers/AbstractDeployScript.s.sol"; + +// Contracts +import {PauseSafeModule} from "contracts/automation/PauseSafeModule.sol"; + +// Addresses +import {CrossChain} from "tests/utils/Addresses.sol"; + +/// @title 002_PauseSafeModule +/// @notice Deploys the Safe module that lets a threat-detection operator pause the Super OETH vault +/// without gathering multisig signatures. +/// @dev Base counterpart of mainnet 006_PauseSafeModule, hosted on the same 2/8 Guardian Safe (the +/// multichain Strategist has the same address on both chains). No governance proposal: that +/// Safe is already the vault's Strategist, and the Strategist may pause. The module widens no +/// authority and cannot unpause — the only selectors compiled into its bytecode are +/// `pauseCapital()`, `pauseRebase()` and `pause()`. +/// +/// Depends on 001_VaultAdminRole having landed: without the Admin role the Strategist could +/// still unpause, and handing an automated operator a pause trigger would be handing it an +/// unpause trigger too. +/// +/// REQUIRED POST-DEPLOY STEP — the module does nothing until the Safe calls +/// `enableModule()` and `isModuleEnabled()` reads true. +contract $002_PauseSafeModule is AbstractDeployScript("002_PauseSafeModule") { + // ==================== Deployment Logic ==================== // + + function _execute() internal override { + address[] memory operators = new address[](1); + // The Hypernative keeper address is not known yet. Ship with the Talos relayer as the + // initial operator; the Safe grants OPERATOR_ROLE to the keeper once the vendor supplies + // its address. Both are pause-only, so neither can undo what it triggers. + operators[0] = CrossChain.talosRelayer; + + address[] memory targets = new address[](1); + targets[0] = resolver.resolve("OETHBASE_VAULT_PROXY"); + + PauseSafeModule pauseSafeModule = new PauseSafeModule(CrossChain.multichainStrategist, operators, targets); + _recordDeployment("PAUSE_SAFE_MODULE", address(pauseSafeModule)); + } + + // ==================== Fork Verification ==================== // + + function _fork() internal override { + PauseSafeModule pauseSafeModule = PauseSafeModule(payable(resolver.resolve("PAUSE_SAFE_MODULE"))); + + require(address(pauseSafeModule.safeContract()) == CrossChain.multichainStrategist, "Wrong Safe"); + require( + pauseSafeModule.isPausableTarget(resolver.resolve("OETHBASE_VAULT_PROXY")), "OETHb vault not allow-listed" + ); + require( + pauseSafeModule.hasRole(pauseSafeModule.OPERATOR_ROLE(), CrossChain.talosRelayer), "Operator role not set" + ); + + // Enabling the module is a Safe transaction, not part of any proposal, so it has to be + // simulated here for the smoke tests to exercise the module at all. Guarded because + // `_fork()` is re-run by every later smoke suite and Safe reverts on a double enable. + ISafeModuleManager safe = ISafeModuleManager(CrossChain.multichainStrategist); + if (!safe.isModuleEnabled(address(pauseSafeModule))) { + vm.prank(CrossChain.multichainStrategist); + safe.enableModule(address(pauseSafeModule)); + } + require(safe.isModuleEnabled(address(pauseSafeModule)), "Module not enabled on the Safe"); + } +} + +// ==================== External Interface ==================== // + +/// @notice Module management surface on a Gnosis Safe. Not part of ISafe, which only carries the +/// `execTransactionFromModule` call the modules themselves make. +interface ISafeModuleManager { + function enableModule(address module) external; + function isModuleEnabled(address module) external view returns (bool); +} diff --git a/contracts/scripts/deploy/mainnet/005_VaultAdminRole.s.sol b/contracts/scripts/deploy/mainnet/005_VaultAdminRole.s.sol new file mode 100644 index 0000000000..35d4402e35 --- /dev/null +++ b/contracts/scripts/deploy/mainnet/005_VaultAdminRole.s.sol @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +// Deployment framework +import {AbstractDeployScript} from "scripts/deploy/helpers/AbstractDeployScript.s.sol"; +import {GovHelper} from "scripts/deploy/helpers/GovHelper.sol"; +import {GovProposal} from "scripts/deploy/helpers/DeploymentTypes.sol"; + +// Contracts +import {IVault} from "contracts/interfaces/IVault.sol"; +import {OUSDVault} from "contracts/vault/OUSDVault.sol"; +import {OETHVault} from "contracts/vault/OETHVault.sol"; +import {InitializeGovernedUpgradeabilityProxy} from "contracts/proxies/InitializeGovernedUpgradeabilityProxy.sol"; + +// Addresses +import {CrossChain, Mainnet} from "tests/utils/Addresses.sol"; + +/// @title 005_VaultAdminRole +/// @notice Gives the OUSD and OETH vaults an Admin role that holds the sole right to unpause. +/// @dev Before this upgrade the Strategist could both pause and unpause, so a single compromised +/// key could trip a pause and immediately lift it. The new `adminAddr` slot splits the two: +/// `pauseCapital`/`pauseRebase` stay open to the Strategist (the 2/8 Guardian Safe), while +/// `unpauseCapital`/`unpauseRebase` become Admin-or-Governor only. That is what makes it safe +/// to hand a pause trigger to an automated threat-detection service — see 006_PauseSafeModule. +/// +/// `adminAddr` is set in the same proposal as the upgrade so the vaults are never left with a +/// zero Admin, which would leave the Governor as the only account able to lift a pause. +contract $005_VaultAdminRole is AbstractDeployScript("005_VaultAdminRole") { + using GovHelper for GovProposal; + + /// @notice The Admin (5/8) multisig. Carried in the address books as `Guardian`, a name that + /// predates the pause/unpause split — it is the 5/8, not the 2/8 Guardian Safe. + address internal constant ADMIN = Mainnet.Guardian; + + // ==================== Deployment Logic ==================== // + + function _execute() internal override { + OUSDVault ousdVaultImpl = new OUSDVault(Mainnet.USDC); + _recordDeployment("OUSD_VAULT_IMPL", address(ousdVaultImpl)); + + OETHVault oethVaultImpl = new OETHVault(Mainnet.WETH); + _recordDeployment("OETH_VAULT_IMPL", address(oethVaultImpl)); + } + + // ==================== Governance Proposal ==================== // + + function _buildGovernanceProposal() internal override { + address ousdVaultProxy = resolver.resolve("OUSD_VAULT_PROXY"); + address oethVaultProxy = resolver.resolve("OETH_VAULT_PROXY"); + + govProposal.setDescription( + "Upgrade OUSD and OETH vaults: Admin can pause, only Admin can unpause\n\n" + "Adds an Admin role to both vaults and points it at the 5/8 multisig. Pausing capital " + "and rebasing stays available to the Strategist, the Admin and the Governor; unpausing " + "is narrowed to the Admin and the Governor, so the Strategist can no longer lift a " + "pause it triggered. Each vault is upgraded and has its Admin set in this same " + "proposal, so neither is left with an unset Admin." + ); + + govProposal.action(ousdVaultProxy, "upgradeTo(address)", abi.encode(resolver.resolve("OUSD_VAULT_IMPL"))); + govProposal.action(ousdVaultProxy, "setAdminAddr(address)", abi.encode(ADMIN)); + + govProposal.action(oethVaultProxy, "upgradeTo(address)", abi.encode(resolver.resolve("OETH_VAULT_IMPL"))); + govProposal.action(oethVaultProxy, "setAdminAddr(address)", abi.encode(ADMIN)); + } + + // ==================== Fork Verification ==================== // + + function _fork() internal override { + _verifyVault("OUSD_VAULT_PROXY", "OUSD_VAULT_IMPL"); + _verifyVault("OETH_VAULT_PROXY", "OETH_VAULT_IMPL"); + } + + /// @dev Read-only. Behaviour of the new role split is covered by the smoke tests under + /// tests/smoke/mainnet/vault; this only proves the deploy landed. + function _verifyVault(string memory proxyName, string memory implName) internal view { + address vaultProxy = resolver.resolve(proxyName); + address expectedImpl = resolver.resolve(implName); + + require( + InitializeGovernedUpgradeabilityProxy(payable(vaultProxy)).implementation() == expectedImpl, + "Vault implementation not updated" + ); + + IVault vault = IVault(vaultProxy); + require(vault.adminAddr() == ADMIN, "Vault admin not set"); + + // `adminAddr` was carved out of the storage gap, immediately after `defaultStrategy` and + // `operatorAddr`. Those two neighbours are where a layout shift would surface first. + require(vault.defaultStrategy() != address(0), "Default strategy cleared"); + require(vault.operatorAddr() == CrossChain.talosRelayer, "Operator changed"); + + // The pause module in 006 drives these vaults through the Strategist Safe, so the pause + // path is only wired if the Safe still holds that role. + require(vault.strategistAddr() == CrossChain.multichainStrategist, "Strategist changed"); + } +} diff --git a/contracts/scripts/deploy/mainnet/006_PauseSafeModule.s.sol b/contracts/scripts/deploy/mainnet/006_PauseSafeModule.s.sol new file mode 100644 index 0000000000..50c5987e53 --- /dev/null +++ b/contracts/scripts/deploy/mainnet/006_PauseSafeModule.s.sol @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +// Deployment framework +import {AbstractDeployScript} from "scripts/deploy/helpers/AbstractDeployScript.s.sol"; + +// Contracts +import {PauseSafeModule} from "contracts/automation/PauseSafeModule.sol"; + +// Addresses +import {CrossChain} from "tests/utils/Addresses.sol"; + +/// @title 006_PauseSafeModule +/// @notice Deploys the Safe module that lets a threat-detection operator pause the OUSD and OETH +/// vaults without gathering multisig signatures. +/// @dev No governance proposal: the 2/8 Guardian Safe hosting the module is already the vaults' +/// Strategist, and the Strategist may pause. The module widens no authority — it only removes +/// the signature-gathering step. It cannot unpause, because the only selectors compiled into +/// its bytecode are `pauseCapital()`, `pauseRebase()` and `pause()`. +/// +/// Depends on 005_VaultAdminRole having landed: without the Admin role the Strategist could +/// still unpause, and handing an automated operator a pause trigger would be handing it an +/// unpause trigger too. +/// +/// REQUIRED POST-DEPLOY STEP — the module does nothing until the Safe calls +/// `enableModule()` and `isModuleEnabled()` reads true. This is the step that +/// was silently skipped for PermissionedRebaseModule, leaving it dead on-chain for months. +/// +/// The ARMs are deliberately NOT allow-listed here. They only become pausable by this Safe +/// once arm-oeth PR #337 ships and each ARM has had `setPauseRoles(2/8, 5/8)` called; the Safe +/// adds each one afterwards with `allowTarget()`. Allow-listing them now would look +/// configured but revert on use. +contract $006_PauseSafeModule is AbstractDeployScript("006_PauseSafeModule") { + // ==================== Deployment Logic ==================== // + + function _execute() internal override { + address[] memory operators = new address[](1); + // The Hypernative keeper address is not known yet. Ship with the Talos relayer as the + // initial operator; the Safe grants OPERATOR_ROLE to the keeper once the vendor supplies + // its address. Both are pause-only, so neither can undo what it triggers. + operators[0] = CrossChain.talosRelayer; + + address[] memory targets = new address[](2); + targets[0] = resolver.resolve("OUSD_VAULT_PROXY"); + targets[1] = resolver.resolve("OETH_VAULT_PROXY"); + + PauseSafeModule pauseSafeModule = new PauseSafeModule(CrossChain.multichainStrategist, operators, targets); + _recordDeployment("PAUSE_SAFE_MODULE", address(pauseSafeModule)); + } + + // ==================== Fork Verification ==================== // + + function _fork() internal override { + PauseSafeModule pauseSafeModule = PauseSafeModule(payable(resolver.resolve("PAUSE_SAFE_MODULE"))); + + require(address(pauseSafeModule.safeContract()) == CrossChain.multichainStrategist, "Wrong Safe"); + require(pauseSafeModule.isPausableTarget(resolver.resolve("OUSD_VAULT_PROXY")), "OUSD vault not allow-listed"); + require(pauseSafeModule.isPausableTarget(resolver.resolve("OETH_VAULT_PROXY")), "OETH vault not allow-listed"); + require( + pauseSafeModule.hasRole(pauseSafeModule.OPERATOR_ROLE(), CrossChain.talosRelayer), "Operator role not set" + ); + + // Enabling the module is a Safe transaction, not part of any proposal, so it has to be + // simulated here for the smoke tests to exercise the module at all. Guarded because + // `_fork()` is re-run by every later smoke suite and Safe reverts on a double enable. + ISafeModuleManager safe = ISafeModuleManager(CrossChain.multichainStrategist); + if (!safe.isModuleEnabled(address(pauseSafeModule))) { + vm.prank(CrossChain.multichainStrategist); + safe.enableModule(address(pauseSafeModule)); + } + require(safe.isModuleEnabled(address(pauseSafeModule)), "Module not enabled on the Safe"); + } +} + +// ==================== External Interface ==================== // + +/// @notice Module management surface on a Gnosis Safe. Not part of ISafe, which only carries the +/// `execTransactionFromModule` call the modules themselves make. +interface ISafeModuleManager { + function enableModule(address module) external; + function isModuleEnabled(address module) external view returns (bool); +} diff --git a/contracts/test/_fixture-base.js b/contracts/test/_fixture-base.js index a6f6bdd6ac..6c8065bba1 100644 --- a/contracts/test/_fixture-base.js +++ b/contracts/test/_fixture-base.js @@ -172,8 +172,9 @@ const defaultFixture = async () => { // Production vault sits paused-for-rebase between strategist runs. // Lift the pause once per fork fixture so tests can exercise rebase // without each call site having to unpause/rebase/pause itself. - // Only the Admin can unpause. - await oethbVault.connect(admin).unpauseRebase(); + // The Strategist is still the unpauser on the live implementation; this + // becomes `admin` once scripts/deploy/base/001_VaultAdminRole executes. + await oethbVault.connect(strategist).unpauseRebase(); } else { admin = signers[2]; await oethbVault.connect(governor).setAdminAddr(admin.address); diff --git a/contracts/test/_fixture.js b/contracts/test/_fixture.js index c80b2bffdc..74b240d22f 100644 --- a/contracts/test/_fixture.js +++ b/contracts/test/_fixture.js @@ -147,8 +147,9 @@ const simpleOETHFixture = deployments.createFixture(async () => { // Production vault sits paused-for-rebase between strategist runs. // Lift the pause once per fork fixture so tests can exercise rebase // without each call site having to unpause/rebase/pause itself. - // Only the Admin can unpause. - await oethVault.connect(admin).unpauseRebase(); + // The Strategist is still the unpauser on the live implementation; this + // becomes `admin` once scripts/deploy/mainnet/005_VaultAdminRole executes. + await oethVault.connect(strategist).unpauseRebase(); for (const user of [matt, josh, anna, domen, daniel, franck]) { // Everyone gets free weth @@ -726,9 +727,10 @@ const defaultFixture = deployments.createFixture(async () => { // Production vaults sit paused-for-rebase between strategist runs. // Lift the pause once per fork fixture so tests can exercise rebase // without each call site having to unpause/rebase/pause itself. - // Only the Admin can unpause. - await vaultAndTokenContracts.vault.connect(admin).unpauseRebase(); - await vaultAndTokenContracts.oethVault.connect(admin).unpauseRebase(); + // The Strategist is still the unpauser on the live implementations; this + // becomes `admin` once scripts/deploy/mainnet/005_VaultAdminRole executes. + await vaultAndTokenContracts.vault.connect(strategist).unpauseRebase(); + await vaultAndTokenContracts.oethVault.connect(strategist).unpauseRebase(); } else { timelock = governor; diff --git a/contracts/test/safe-modules/pause-module.base.fork-test.js b/contracts/test/safe-modules/pause-module.base.fork-test.js deleted file mode 100644 index f30b0f7ceb..0000000000 --- a/contracts/test/safe-modules/pause-module.base.fork-test.js +++ /dev/null @@ -1,70 +0,0 @@ -const { expect } = require("chai"); - -const addresses = require("../../utils/addresses"); -const { createFixtureLoader } = require("../_fixture"); -const { defaultBaseFixture } = require("../_fixture-base"); -const { impersonateAndFund } = require("../../utils/signers"); - -const baseFixture = createFixtureLoader(defaultBaseFixture); - -describe("ForkTest: Pause Safe Module (Base)", function () { - this.timeout(0); - - let fixture; - let pauseModule; - let operator; - - beforeEach(async () => { - fixture = await baseFixture(); - pauseModule = await ethers.getContract("PauseSafeModule"); - operator = await impersonateAndFund(addresses.talosRelayer); - }); - - it("Should be enabled on the Guardian Safe", async () => { - const safe = await ethers.getContractAt( - ["function isModuleEnabled(address module) external view returns (bool)"], - addresses.multichainStrategist - ); - expect(await safe.isModuleEnabled(pauseModule.address)).to.be.true; - }); - - it("Should have the OETHb vault allow-listed", async () => { - const { oethbVault } = fixture; - expect(await pauseModule.isPausableTarget(oethbVault.address)).to.be.true; - }); - - it("Should let the operator pause capital, and only the admin lift it", async () => { - const { admin, oethbVault, strategist } = fixture; - - expect(await oethbVault.capitalPaused()).to.be.false; - - await pauseModule.connect(operator).pauseCapital(oethbVault.address); - expect(await oethbVault.capitalPaused()).to.be.true; - - await expect( - oethbVault.connect(strategist).unpauseCapital() - ).to.be.revertedWith("Caller is not the Admin or Governor"); - expect(await oethbVault.capitalPaused()).to.be.true; - - await oethbVault.connect(admin).unpauseCapital(); - expect(await oethbVault.capitalPaused()).to.be.false; - }); - - it("Should let the operator pause rebase", async () => { - const { admin, oethbVault } = fixture; - - await pauseModule.connect(operator).pauseRebase(oethbVault.address); - expect(await oethbVault.rebasePaused()).to.be.true; - - await oethbVault.connect(admin).unpauseRebase(); - expect(await oethbVault.rebasePaused()).to.be.false; - }); - - it("Should revert for a non-operator", async () => { - const { nick, oethbVault } = fixture; - - await expect( - pauseModule.connect(nick).pauseCapital(oethbVault.address) - ).to.be.revertedWith("Caller is not an operator"); - }); -}); diff --git a/contracts/test/safe-modules/pause-module.mainnet.fork-test.js b/contracts/test/safe-modules/pause-module.mainnet.fork-test.js deleted file mode 100644 index e544448195..0000000000 --- a/contracts/test/safe-modules/pause-module.mainnet.fork-test.js +++ /dev/null @@ -1,93 +0,0 @@ -const { expect } = require("chai"); - -const addresses = require("../../utils/addresses"); -const { loadDefaultFixture } = require("../_fixture"); -const { isCI } = require("../helpers"); -const { impersonateAndFund } = require("../../utils/signers"); - -describe("ForkTest: Pause Safe Module", function () { - this.timeout(0); - this.retries(isCI ? 3 : 0); - - let fixture; - let pauseModule; - let operator; - - beforeEach(async () => { - fixture = await loadDefaultFixture(); - pauseModule = await ethers.getContract("PauseSafeModule"); - operator = await impersonateAndFund(addresses.talosRelayer); - }); - - it("Should have the expected operator", async () => { - const operatorRole = await pauseModule.OPERATOR_ROLE(); - expect(await pauseModule.hasRole(operatorRole, addresses.talosRelayer)).to - .be.true; - }); - - it("Should be enabled on the Guardian Safe", async () => { - const safe = await ethers.getContractAt( - ["function isModuleEnabled(address module) external view returns (bool)"], - addresses.multichainStrategist - ); - expect(await safe.isModuleEnabled(pauseModule.address)).to.be.true; - }); - - it("Should have both mainnet vaults allow-listed", async () => { - const vaultProxy = await ethers.getContract("VaultProxy"); - const oethVaultProxy = await ethers.getContract("OETHVaultProxy"); - - expect(await pauseModule.isPausableTarget(vaultProxy.address)).to.be.true; - expect(await pauseModule.isPausableTarget(oethVaultProxy.address)).to.be - .true; - }); - - for (const [vaultName, vaultProxyName] of [ - ["OUSD", "VaultProxy"], - ["OETH", "OETHVaultProxy"], - ]) { - describe(`${vaultName} vault`, () => { - it("Should let the operator pause capital, and only the admin lift it", async () => { - const { admin, strategist } = fixture; - const proxy = await ethers.getContract(vaultProxyName); - const vault = await ethers.getContractAt("IVault", proxy.address); - - expect(await vault.capitalPaused()).to.be.false; - - await pauseModule.connect(operator).pauseCapital(vault.address); - expect(await vault.capitalPaused()).to.be.true; - - // The Safe hosting the module is the Strategist. It could trip the - // pause, but it cannot lift it — that is the whole point. - await expect( - vault.connect(strategist).unpauseCapital() - ).to.be.revertedWith("Caller is not the Admin or Governor"); - expect(await vault.capitalPaused()).to.be.true; - - await vault.connect(admin).unpauseCapital(); - expect(await vault.capitalPaused()).to.be.false; - }); - - it("Should let the operator pause rebase", async () => { - const { admin } = fixture; - const proxy = await ethers.getContract(vaultProxyName); - const vault = await ethers.getContractAt("IVault", proxy.address); - - await pauseModule.connect(operator).pauseRebase(vault.address); - expect(await vault.rebasePaused()).to.be.true; - - await vault.connect(admin).unpauseRebase(); - expect(await vault.rebasePaused()).to.be.false; - }); - - it("Should revert for a non-operator", async () => { - const { anna } = fixture; - const proxy = await ethers.getContract(vaultProxyName); - - await expect( - pauseModule.connect(anna).pauseCapital(proxy.address) - ).to.be.revertedWith("Caller is not an operator"); - }); - }); - } -}); diff --git a/contracts/test/vault/index.js b/contracts/test/vault/index.js index 0a62e6de53..356e1063f0 100644 --- a/contracts/test/vault/index.js +++ b/contracts/test/vault/index.js @@ -3,6 +3,7 @@ const { utils } = require("ethers"); const { loadDefaultFixture } = require("../_fixture"); const { ousdUnits, usdsUnits, usdcUnits, isFork } = require("../helpers"); +const addresses = require("../../utils/addresses"); describe("Vault", function () { if (isFork) { @@ -182,6 +183,16 @@ describe("Vault", function () { } }); + it("Should not allow the Admin address to be unset", async () => { + const { vault, governor, admin } = fixture; + + await expect( + vault.connect(governor).setAdminAddr(addresses.zero) + ).to.be.revertedWith("Invalid admin"); + + expect(await vault.adminAddr()).to.equal(admin.address); + }); + it("Should allow the Governor to call withdraw and then deposit", async () => { const { vault, governor, usdc, josh, mockStrategy } = fixture; diff --git a/contracts/test/vault/oethb-vault.base.fork-test.js b/contracts/test/vault/oethb-vault.base.fork-test.js index 940c91f1f4..583f37e6d1 100644 --- a/contracts/test/vault/oethb-vault.base.fork-test.js +++ b/contracts/test/vault/oethb-vault.base.fork-test.js @@ -21,50 +21,6 @@ describe("ForkTest: OETHb Vault", function () { await oethbVault.connect(signer).mint(oethUnits("1")); } - describe("Admin", function () { - it("Should have the correct admin address set", async () => { - const { oethbVault } = fixture; - expect(await oethbVault.adminAddr()).to.equal(addresses.base.admin); - }); - - it("Should let the strategist pause but not unpause capital", async () => { - const { admin, oethbVault, strategist } = fixture; - - await oethbVault.connect(strategist).pauseCapital(); - expect(await oethbVault.capitalPaused()).to.be.true; - - await expect( - oethbVault.connect(strategist).unpauseCapital() - ).to.be.revertedWith("Caller is not the Admin or Governor"); - expect(await oethbVault.capitalPaused()).to.be.true; - - // Only the Admin can lift the pause the Strategist tripped - await oethbVault.connect(admin).unpauseCapital(); - expect(await oethbVault.capitalPaused()).to.be.false; - }); - - it("Should let the admin pause and unpause rebase", async () => { - const { admin, oethbVault } = fixture; - - await oethbVault.connect(admin).pauseRebase(); - expect(await oethbVault.rebasePaused()).to.be.true; - await oethbVault.connect(admin).unpauseRebase(); - expect(await oethbVault.rebasePaused()).to.be.false; - }); - - it("Should still read pre-existing storage correctly after the upgrade", async () => { - const { oethbVault } = fixture; - - // The admin slot was taken from the storage gap, so a layout shift - // would show up in the slots around it first. - expect(await oethbVault.strategistAddr()).to.equal( - addresses.multichainStrategist - ); - expect(await oethbVault.governor()).to.equal(addresses.base.timelock); - expect(await oethbVault.totalValue()).to.be.gt(0); - }); - }); - describe("Mint & Permissioned redeems", function () { it("Should allow anyone to mint", async () => { const { nick, weth, oethb, oethbVault, strategist } = fixture; diff --git a/contracts/test/vault/rebase.js b/contracts/test/vault/rebase.js index 9f57eb90b5..910b4d3f86 100644 --- a/contracts/test/vault/rebase.js +++ b/contracts/test/vault/rebase.js @@ -48,11 +48,16 @@ describe("Vault rebase", () => { it("Should allow admin to pause rebasing", async () => { const { vault, admin } = fixture; await vault.connect(admin).pauseRebase(); + expect(await vault.rebasePaused()).to.be.true; }); it("Should allow admin to unpause rebasing", async () => { const { vault, admin } = fixture; + await vault.connect(admin).pauseRebase(); + expect(await vault.rebasePaused()).to.be.true; + await vault.connect(admin).unpauseRebase(); + expect(await vault.rebasePaused()).to.be.false; }); it("Should allow governor to pause rebasing", async () => { diff --git a/contracts/test/vault/vault.mainnet.fork-test.js b/contracts/test/vault/vault.mainnet.fork-test.js index ccd2423f93..2a04129eef 100644 --- a/contracts/test/vault/vault.mainnet.fork-test.js +++ b/contracts/test/vault/vault.mainnet.fork-test.js @@ -71,58 +71,6 @@ describe("ForkTest: Vault", function () { ); }); - it("Should have the correct admin address set", async () => { - const { vault } = fixture; - expect(await vault.adminAddr()).to.equal(addresses.mainnet.Guardian); - }); - - it("Should let the strategist pause but not unpause capital", async () => { - const { admin, strategist, vault } = fixture; - - await vault.connect(strategist).pauseCapital(); - expect(await vault.capitalPaused()).to.be.true; - - await expect( - vault.connect(strategist).unpauseCapital() - ).to.be.revertedWith("Caller is not the Admin or Governor"); - expect(await vault.capitalPaused()).to.be.true; - - // Only the Admin can lift the pause the Strategist tripped - await vault.connect(admin).unpauseCapital(); - expect(await vault.capitalPaused()).to.be.false; - }); - - it("Should let the admin pause and unpause capital and rebase", async () => { - const { admin, vault } = fixture; - - await vault.connect(admin).pauseCapital(); - expect(await vault.capitalPaused()).to.be.true; - await vault.connect(admin).unpauseCapital(); - expect(await vault.capitalPaused()).to.be.false; - - await vault.connect(admin).pauseRebase(); - expect(await vault.rebasePaused()).to.be.true; - await vault.connect(admin).unpauseRebase(); - expect(await vault.rebasePaused()).to.be.false; - }); - - it("Should still read pre-existing storage correctly after the upgrade", async () => { - const { vault, ousd } = fixture; - - // The admin slot was taken from the storage gap. `defaultStrategy` and - // `operatorAddr` are the two slots immediately before it, so a layout - // shift would show up here first. - expect(await vault.defaultStrategy()).to.not.equal(addresses.zero); - expect(await vault.operatorAddr()).to.not.equal(addresses.zero); - expect(await vault.strategistAddr()).to.equal( - addresses.multichainStrategist - ); - expect(await vault.trusteeAddress()).to.not.equal(addresses.zero); - expect(await vault.governor()).to.equal(addresses.mainnet.Timelock); - expect(await vault.totalValue()).to.be.gt(0); - expect(await ousd.totalSupply()).to.be.gt(0); - }); - it("Should have the OUSD/USDC AMO mint whitelist", async () => { const { vault } = fixture; expect( diff --git a/contracts/tests/smoke/BaseSmoke.t.sol b/contracts/tests/smoke/BaseSmoke.t.sol index a5834e835a..7cfc0d367e 100644 --- a/contracts/tests/smoke/BaseSmoke.t.sol +++ b/contracts/tests/smoke/BaseSmoke.t.sol @@ -12,9 +12,21 @@ abstract contract BaseSmoke is BaseFork { Resolver internal resolver = Resolver(address(uint160(uint256(keccak256("Resolver"))))); DeployManager internal deployManager; + /// @dev Applying the pending deploys means simulating their governance, and that moves the fork + /// clock forward — the Base timelock alone has a 2 day delay, and GovernorSix needs its + /// whole voting window. The jump is an artifact of the simulation, not of the deployment. + /// Smoke tests assert against live chain state and several of them read Chainlink feeds, + /// which go stale and revert once the clock runs past their heartbeat. Put the clock back + /// afterwards so every suite observes the chain at the timestamp it forked from. function _igniteDeployManager() internal { + uint256 forkTimestamp = block.timestamp; + deployManager = new DeployManager(); deployManager.setUp(); deployManager.run(); + + if (block.timestamp != forkTimestamp) { + vm.warp(forkTimestamp); + } } } diff --git a/contracts/tests/smoke/base/automation/PauseSafeModule/concrete/PauseSafeModule.t.sol b/contracts/tests/smoke/base/automation/PauseSafeModule/concrete/PauseSafeModule.t.sol new file mode 100644 index 0000000000..2043e0ed01 --- /dev/null +++ b/contracts/tests/smoke/base/automation/PauseSafeModule/concrete/PauseSafeModule.t.sol @@ -0,0 +1,93 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +// --- Test base +import {Smoke_Base_PauseSafeModule_Shared_Test} from "tests/smoke/base/automation/PauseSafeModule/shared/Shared.t.sol"; + +// --- Test utilities +import {CrossChain} from "tests/utils/Addresses.sol"; + +contract Smoke_Concrete_Base_PauseSafeModule_Test is Smoke_Base_PauseSafeModule_Shared_Test { + ////////////////////////////////////////////////////// + /// --- WIRING + ////////////////////////////////////////////////////// + + function test_operatorRole_isTalosRelayer() public view { + assertTrue(pauseSafeModule.hasRole(pauseSafeModule.OPERATOR_ROLE(), CrossChain.talosRelayer)); + } + + /// @dev The step that was skipped for PermissionedRebaseModule, leaving it dead on-chain for + /// months. A module that is not enabled reverts on every call it makes. + function test_module_isEnabledOnTheGuardianSafe() public view { + assertTrue(ISafeModuleManager(CrossChain.multichainStrategist).isModuleEnabled(address(pauseSafeModule))); + } + + function test_vault_isAllowListed() public view { + assertTrue(pauseSafeModule.isPausableTarget(address(oethBaseVault))); + } + + /// @dev The module forwards through the Safe, which authorizes as the vault's Strategist. The + /// 2/8 Guardian Safe has the same address on Base as on mainnet. + function test_guardianSafe_isTheStrategist() public view { + assertEq(strategist, CrossChain.multichainStrategist); + } + + ////////////////////////////////////////////////////// + /// --- PAUSE + ////////////////////////////////////////////////////// + + /// @dev The property this whole project exists for: the operator trips the pause through the + /// Guardian Safe, and that same Safe — the vault's Strategist — cannot lift it again. + function test_pauseCapital_onlyAdminCanLiftIt() public { + assertFalse(oethBaseVault.capitalPaused()); + + vm.prank(CrossChain.talosRelayer); + pauseSafeModule.pauseCapital(address(oethBaseVault)); + assertTrue(oethBaseVault.capitalPaused()); + + vm.prank(strategist); + vm.expectRevert("Caller is not the Admin or Governor"); + oethBaseVault.unpauseCapital(); + assertTrue(oethBaseVault.capitalPaused()); + + vm.prank(admin); + oethBaseVault.unpauseCapital(); + assertFalse(oethBaseVault.capitalPaused()); + } + + function test_pauseRebase_onlyAdminCanLiftIt() public { + vm.prank(CrossChain.talosRelayer); + pauseSafeModule.pauseRebase(address(oethBaseVault)); + assertTrue(oethBaseVault.rebasePaused()); + + vm.prank(strategist); + vm.expectRevert("Caller is not the Admin or Governor"); + oethBaseVault.unpauseRebase(); + assertTrue(oethBaseVault.rebasePaused()); + + vm.prank(admin); + oethBaseVault.unpauseRebase(); + assertFalse(oethBaseVault.rebasePaused()); + } + + ////////////////////////////////////////////////////// + /// --- ACCESS CONTROL + ////////////////////////////////////////////////////// + + function test_pauseCapital_RevertWhen_notOperator() public { + vm.prank(alice); + vm.expectRevert("Caller is not an operator"); + pauseSafeModule.pauseCapital(address(oethBaseVault)); + } + + function test_allowTarget_RevertWhen_notSafe() public { + vm.prank(CrossChain.talosRelayer); + vm.expectRevert("Caller is not the safe contract"); + pauseSafeModule.allowTarget(alice); + } +} + +/// @notice Module management surface on a Gnosis Safe, not part of ISafe. +interface ISafeModuleManager { + function isModuleEnabled(address module) external view returns (bool); +} diff --git a/contracts/tests/smoke/base/automation/PauseSafeModule/shared/Shared.t.sol b/contracts/tests/smoke/base/automation/PauseSafeModule/shared/Shared.t.sol new file mode 100644 index 0000000000..881772e3cf --- /dev/null +++ b/contracts/tests/smoke/base/automation/PauseSafeModule/shared/Shared.t.sol @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +// --- Test base +import {BaseSmoke} from "tests/smoke/BaseSmoke.t.sol"; + +// --- Test utilities +import {CrossChain} from "tests/utils/Addresses.sol"; + +// --- Project imports +import {IPauseSafeModule} from "contracts/interfaces/automation/IPauseSafeModule.sol"; +import {IVault} from "contracts/interfaces/IVault.sol"; + +abstract contract Smoke_Base_PauseSafeModule_Shared_Test is BaseSmoke { + IPauseSafeModule internal pauseSafeModule; + + IVault internal oethBaseVault; + + /// @dev The 5/8 that holds `adminAddr`, i.e. the only account able to lift what the module trips. + address internal admin; + + function setUp() public virtual override { + super.setUp(); + _createAndSelectForkBase(); + _igniteDeployManager(); + + require(address(resolver).code.length > 0, "Resolver not initialized on fork"); + + pauseSafeModule = IPauseSafeModule(payable(resolver.resolve("PAUSE_SAFE_MODULE"))); + oethBaseVault = IVault(resolver.resolve("OETHBASE_VAULT_PROXY")); + + strategist = oethBaseVault.strategistAddr(); + admin = oethBaseVault.adminAddr(); + + vm.label(address(pauseSafeModule), "PauseSafeModule"); + vm.label(address(oethBaseVault), "OETHBaseVault"); + vm.label(CrossChain.multichainStrategist, "GuardianSafe"); + vm.label(admin, "AdminSafe"); + } +} diff --git a/contracts/tests/smoke/base/vault/OETHBaseVault/concrete/Admin.t.sol b/contracts/tests/smoke/base/vault/OETHBaseVault/concrete/Admin.t.sol new file mode 100644 index 0000000000..c3c923e071 --- /dev/null +++ b/contracts/tests/smoke/base/vault/OETHBaseVault/concrete/Admin.t.sol @@ -0,0 +1,63 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +// --- Test base +import {Smoke_OETHBaseVault_Shared_Test} from "tests/smoke/base/vault/OETHBaseVault/shared/Shared.t.sol"; + +// --- Test utilities +import {Base as BaseAddresses, CrossChain} from "tests/utils/Addresses.sol"; + +contract Smoke_Concrete_OETHBaseVault_Admin_Test is Smoke_OETHBaseVault_Shared_Test { + ////////////////////////////////////////////////////// + /// --- ADMIN ROLE + ////////////////////////////////////////////////////// + + function test_adminAddr_isTheFiveOfEight() public view { + assertEq(oethBaseVault.adminAddr(), BaseAddresses.admin); + } + + /// @dev The point of the Admin role: whoever trips a pause cannot immediately lift it. + function test_strategist_canPauseCapital_butNotUnpause() public { + vm.prank(strategist); + oethBaseVault.pauseCapital(); + assertTrue(oethBaseVault.capitalPaused()); + + vm.prank(strategist); + vm.expectRevert("Caller is not the Admin or Governor"); + oethBaseVault.unpauseCapital(); + assertTrue(oethBaseVault.capitalPaused()); + + vm.prank(oethBaseVault.adminAddr()); + oethBaseVault.unpauseCapital(); + assertFalse(oethBaseVault.capitalPaused()); + } + + function test_admin_canPauseAndUnpauseRebase() public { + vm.startPrank(oethBaseVault.adminAddr()); + + oethBaseVault.pauseRebase(); + assertTrue(oethBaseVault.rebasePaused()); + + oethBaseVault.unpauseRebase(); + assertFalse(oethBaseVault.rebasePaused()); + + vm.stopPrank(); + } + + function test_setAdminAddr_RevertWhen_notGovernor() public { + vm.prank(strategist); + vm.expectRevert("Caller is not the Governor"); + oethBaseVault.setAdminAddr(alice); + } + + /// @dev `adminAddr` was carved out of the storage gap, immediately after `defaultStrategy` and + /// `operatorAddr`. Those neighbours are where a layout shift would surface first. + function test_preExistingStorage_survivedTheUpgrade() public view { + assertTrue(oethBaseVault.defaultStrategy() != address(0)); + assertEq(oethBaseVault.operatorAddr(), CrossChain.talosRelayer); + assertEq(oethBaseVault.strategistAddr(), CrossChain.multichainStrategist); + assertEq(oethBaseVault.governor(), BaseAddresses.timelock); + assertTrue(oethBaseVault.totalValue() > 0); + assertTrue(oethBase.totalSupply() > 0); + } +} diff --git a/contracts/tests/smoke/mainnet/automation/PauseSafeModule/concrete/PauseSafeModule.t.sol b/contracts/tests/smoke/mainnet/automation/PauseSafeModule/concrete/PauseSafeModule.t.sol new file mode 100644 index 0000000000..a1858c8af4 --- /dev/null +++ b/contracts/tests/smoke/mainnet/automation/PauseSafeModule/concrete/PauseSafeModule.t.sol @@ -0,0 +1,127 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +// --- Test base +import {Smoke_PauseSafeModule_Shared_Test} from "tests/smoke/mainnet/automation/PauseSafeModule/shared/Shared.t.sol"; + +// --- Test utilities +import {CrossChain} from "tests/utils/Addresses.sol"; + +// --- Project imports +import {IVault} from "contracts/interfaces/IVault.sol"; + +contract Smoke_Concrete_PauseSafeModule_Test is Smoke_PauseSafeModule_Shared_Test { + ////////////////////////////////////////////////////// + /// --- WIRING + ////////////////////////////////////////////////////// + + function test_operatorRole_isTalosRelayer() public view { + assertTrue(pauseSafeModule.hasRole(pauseSafeModule.OPERATOR_ROLE(), CrossChain.talosRelayer)); + } + + /// @dev The step that was skipped for PermissionedRebaseModule, leaving it dead on-chain for + /// months. A module that is not enabled reverts on every call it makes. + function test_module_isEnabledOnTheGuardianSafe() public view { + assertTrue(ISafeModuleManager(CrossChain.multichainStrategist).isModuleEnabled(address(pauseSafeModule))); + } + + function test_bothVaults_areAllowListed() public view { + assertTrue(pauseSafeModule.isPausableTarget(address(ousdVault))); + assertTrue(pauseSafeModule.isPausableTarget(address(oethVault))); + } + + /// @dev The module forwards through the Safe, which authorizes as the vaults' Strategist. + function test_guardianSafe_isTheStrategist() public view { + assertEq(strategist, CrossChain.multichainStrategist); + assertEq(oethVault.strategistAddr(), CrossChain.multichainStrategist); + } + + ////////////////////////////////////////////////////// + /// --- PAUSE CAPITAL + ////////////////////////////////////////////////////// + + function test_pauseCapital_ousd() public { + _assertOnlyAdminCanLiftACapitalPause(ousdVault); + } + + function test_pauseCapital_oeth() public { + _assertOnlyAdminCanLiftACapitalPause(oethVault); + } + + ////////////////////////////////////////////////////// + /// --- PAUSE REBASE + ////////////////////////////////////////////////////// + + function test_pauseRebase_ousd() public { + _assertOnlyAdminCanLiftARebasePause(ousdVault); + } + + function test_pauseRebase_oeth() public { + _assertOnlyAdminCanLiftARebasePause(oethVault); + } + + ////////////////////////////////////////////////////// + /// --- ACCESS CONTROL + ////////////////////////////////////////////////////// + + function test_pauseCapital_RevertWhen_notOperator() public { + vm.prank(alice); + vm.expectRevert("Caller is not an operator"); + pauseSafeModule.pauseCapital(address(ousdVault)); + } + + function test_pauseRebase_RevertWhen_notOperator() public { + vm.prank(alice); + vm.expectRevert("Caller is not an operator"); + pauseSafeModule.pauseRebase(address(oethVault)); + } + + function test_allowTarget_RevertWhen_notSafe() public { + vm.prank(CrossChain.talosRelayer); + vm.expectRevert("Caller is not the safe contract"); + pauseSafeModule.allowTarget(alice); + } + + ////////////////////////////////////////////////////// + /// --- HELPERS + ////////////////////////////////////////////////////// + + /// @dev The property this whole project exists for: the operator trips the pause through the + /// Guardian Safe, and that same Safe — the vault's Strategist — cannot lift it again. + function _assertOnlyAdminCanLiftACapitalPause(IVault vault) internal { + assertFalse(vault.capitalPaused()); + + vm.prank(CrossChain.talosRelayer); + pauseSafeModule.pauseCapital(address(vault)); + assertTrue(vault.capitalPaused()); + + vm.prank(strategist); + vm.expectRevert("Caller is not the Admin or Governor"); + vault.unpauseCapital(); + assertTrue(vault.capitalPaused()); + + vm.prank(admin); + vault.unpauseCapital(); + assertFalse(vault.capitalPaused()); + } + + function _assertOnlyAdminCanLiftARebasePause(IVault vault) internal { + vm.prank(CrossChain.talosRelayer); + pauseSafeModule.pauseRebase(address(vault)); + assertTrue(vault.rebasePaused()); + + vm.prank(strategist); + vm.expectRevert("Caller is not the Admin or Governor"); + vault.unpauseRebase(); + assertTrue(vault.rebasePaused()); + + vm.prank(admin); + vault.unpauseRebase(); + assertFalse(vault.rebasePaused()); + } +} + +/// @notice Module management surface on a Gnosis Safe, not part of ISafe. +interface ISafeModuleManager { + function isModuleEnabled(address module) external view returns (bool); +} diff --git a/contracts/tests/smoke/mainnet/automation/PauseSafeModule/shared/Shared.t.sol b/contracts/tests/smoke/mainnet/automation/PauseSafeModule/shared/Shared.t.sol new file mode 100644 index 0000000000..10e04bf6e2 --- /dev/null +++ b/contracts/tests/smoke/mainnet/automation/PauseSafeModule/shared/Shared.t.sol @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +// --- Test base +import {BaseSmoke} from "tests/smoke/BaseSmoke.t.sol"; + +// --- Test utilities +import {CrossChain} from "tests/utils/Addresses.sol"; + +// --- Project imports +import {IPauseSafeModule} from "contracts/interfaces/automation/IPauseSafeModule.sol"; +import {IVault} from "contracts/interfaces/IVault.sol"; + +abstract contract Smoke_PauseSafeModule_Shared_Test is BaseSmoke { + IPauseSafeModule internal pauseSafeModule; + + IVault internal ousdVault; + IVault internal oethVault; + + /// @dev The 5/8 that holds `adminAddr`, i.e. the only account able to lift what the module trips. + address internal admin; + + function setUp() public virtual override { + super.setUp(); + _createAndSelectForkMainnet(); + _igniteDeployManager(); + + require(address(resolver).code.length > 0, "Resolver not initialized on fork"); + + pauseSafeModule = IPauseSafeModule(payable(resolver.resolve("PAUSE_SAFE_MODULE"))); + ousdVault = IVault(resolver.resolve("OUSD_VAULT_PROXY")); + oethVault = IVault(resolver.resolve("OETH_VAULT_PROXY")); + + strategist = ousdVault.strategistAddr(); + admin = ousdVault.adminAddr(); + + vm.label(address(pauseSafeModule), "PauseSafeModule"); + vm.label(address(ousdVault), "OUSDVault"); + vm.label(address(oethVault), "OETHVault"); + vm.label(CrossChain.multichainStrategist, "GuardianSafe"); + vm.label(admin, "AdminSafe"); + } +} diff --git a/contracts/tests/smoke/mainnet/vault/OETHVault/concrete/Admin.t.sol b/contracts/tests/smoke/mainnet/vault/OETHVault/concrete/Admin.t.sol new file mode 100644 index 0000000000..8bd105b6ca --- /dev/null +++ b/contracts/tests/smoke/mainnet/vault/OETHVault/concrete/Admin.t.sol @@ -0,0 +1,63 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +// --- Test base +import {Smoke_OETHVault_Shared_Test} from "tests/smoke/mainnet/vault/OETHVault/shared/Shared.t.sol"; + +// --- Test utilities +import {CrossChain, Mainnet} from "tests/utils/Addresses.sol"; + +contract Smoke_Concrete_OETHVault_Admin_Test is Smoke_OETHVault_Shared_Test { + ////////////////////////////////////////////////////// + /// --- ADMIN ROLE + ////////////////////////////////////////////////////// + + function test_adminAddr_isTheFiveOfEight() public view { + assertEq(oethVault.adminAddr(), Mainnet.Guardian); + } + + /// @dev The point of the Admin role: whoever trips a pause cannot immediately lift it. + function test_strategist_canPauseCapital_butNotUnpause() public { + vm.prank(strategist); + oethVault.pauseCapital(); + assertTrue(oethVault.capitalPaused()); + + vm.prank(strategist); + vm.expectRevert("Caller is not the Admin or Governor"); + oethVault.unpauseCapital(); + assertTrue(oethVault.capitalPaused()); + + vm.prank(oethVault.adminAddr()); + oethVault.unpauseCapital(); + assertFalse(oethVault.capitalPaused()); + } + + function test_admin_canPauseAndUnpauseRebase() public { + vm.startPrank(oethVault.adminAddr()); + + oethVault.pauseRebase(); + assertTrue(oethVault.rebasePaused()); + + oethVault.unpauseRebase(); + assertFalse(oethVault.rebasePaused()); + + vm.stopPrank(); + } + + function test_setAdminAddr_RevertWhen_notGovernor() public { + vm.prank(strategist); + vm.expectRevert("Caller is not the Governor"); + oethVault.setAdminAddr(alice); + } + + /// @dev `adminAddr` was carved out of the storage gap, immediately after `defaultStrategy` and + /// `operatorAddr`. Those neighbours are where a layout shift would surface first. + function test_preExistingStorage_survivedTheUpgrade() public view { + assertTrue(oethVault.defaultStrategy() != address(0)); + assertEq(oethVault.operatorAddr(), CrossChain.talosRelayer); + assertEq(oethVault.strategistAddr(), CrossChain.multichainStrategist); + assertEq(oethVault.governor(), Mainnet.Timelock); + assertTrue(oethVault.totalValue() > 0); + assertTrue(oeth.totalSupply() > 0); + } +} diff --git a/contracts/tests/smoke/mainnet/vault/OUSDVault/concrete/Admin.t.sol b/contracts/tests/smoke/mainnet/vault/OUSDVault/concrete/Admin.t.sol new file mode 100644 index 0000000000..40c193aba2 --- /dev/null +++ b/contracts/tests/smoke/mainnet/vault/OUSDVault/concrete/Admin.t.sol @@ -0,0 +1,63 @@ +// SPDX-License-Identifier: BUSL-1.1 +pragma solidity ^0.8.0; + +// --- Test base +import {Smoke_OUSDVault_Shared_Test} from "tests/smoke/mainnet/vault/OUSDVault/shared/Shared.t.sol"; + +// --- Test utilities +import {CrossChain, Mainnet} from "tests/utils/Addresses.sol"; + +contract Smoke_Concrete_OUSDVault_Admin_Test is Smoke_OUSDVault_Shared_Test { + ////////////////////////////////////////////////////// + /// --- ADMIN ROLE + ////////////////////////////////////////////////////// + + function test_adminAddr_isTheFiveOfEight() public view { + assertEq(ousdVault.adminAddr(), Mainnet.Guardian); + } + + /// @dev The point of the Admin role: whoever trips a pause cannot immediately lift it. + function test_strategist_canPauseCapital_butNotUnpause() public { + vm.prank(strategist); + ousdVault.pauseCapital(); + assertTrue(ousdVault.capitalPaused()); + + vm.prank(strategist); + vm.expectRevert("Caller is not the Admin or Governor"); + ousdVault.unpauseCapital(); + assertTrue(ousdVault.capitalPaused()); + + vm.prank(ousdVault.adminAddr()); + ousdVault.unpauseCapital(); + assertFalse(ousdVault.capitalPaused()); + } + + function test_admin_canPauseAndUnpauseRebase() public { + vm.startPrank(ousdVault.adminAddr()); + + ousdVault.pauseRebase(); + assertTrue(ousdVault.rebasePaused()); + + ousdVault.unpauseRebase(); + assertFalse(ousdVault.rebasePaused()); + + vm.stopPrank(); + } + + function test_setAdminAddr_RevertWhen_notGovernor() public { + vm.prank(strategist); + vm.expectRevert("Caller is not the Governor"); + ousdVault.setAdminAddr(alice); + } + + /// @dev `adminAddr` was carved out of the storage gap, immediately after `defaultStrategy` and + /// `operatorAddr`. Those neighbours are where a layout shift would surface first. + function test_preExistingStorage_survivedTheUpgrade() public view { + assertTrue(ousdVault.defaultStrategy() != address(0)); + assertEq(ousdVault.operatorAddr(), CrossChain.talosRelayer); + assertEq(ousdVault.strategistAddr(), CrossChain.multichainStrategist); + assertEq(ousdVault.governor(), Mainnet.Timelock); + assertTrue(ousdVault.totalValue() > 0); + assertTrue(ousd.totalSupply() > 0); + } +} diff --git a/contracts/tests/unit/automation/PauseSafeModule/concrete/Constructor.t.sol b/contracts/tests/unit/automation/PauseSafeModule/concrete/Constructor.t.sol index ab8617db3f..93e9a75563 100644 --- a/contracts/tests/unit/automation/PauseSafeModule/concrete/Constructor.t.sol +++ b/contracts/tests/unit/automation/PauseSafeModule/concrete/Constructor.t.sol @@ -51,7 +51,17 @@ contract Unit_Concrete_PauseSafeModule_Constructor_Test is Unit_PauseSafeModule_ address[] memory targets = new address[](1); targets[0] = address(0); - vm.expectRevert("Invalid target"); + vm.expectRevert("Target has no code"); + vm.deployCode(Automation.PAUSE_SAFE_MODULE, abi.encode(address(mockSafe), operators, targets)); + } + + function test_constructor_revertsOnEOATarget() public { + address[] memory operators = new address[](1); + operators[0] = operator; + address[] memory targets = new address[](1); + targets[0] = alice; + + vm.expectRevert("Target has no code"); vm.deployCode(Automation.PAUSE_SAFE_MODULE, abi.encode(address(mockSafe), operators, targets)); } } diff --git a/contracts/tests/unit/automation/PauseSafeModule/concrete/PauseActions.t.sol b/contracts/tests/unit/automation/PauseSafeModule/concrete/PauseActions.t.sol index 7700028459..0f02611d43 100644 --- a/contracts/tests/unit/automation/PauseSafeModule/concrete/PauseActions.t.sol +++ b/contracts/tests/unit/automation/PauseSafeModule/concrete/PauseActions.t.sol @@ -170,17 +170,12 @@ contract Unit_Concrete_PauseSafeModule_PauseActions_Test is Unit_PauseSafeModule /// that lifts a pause. If someone later adds an unpause entry point these /// calls stop reverting and this test fails. function test_module_hasNoUnpauseEntryPoint() public { - string[4] memory signatures = [ - "unpauseCapital(address)", - "unpauseRebase(address)", - "unpauseCapital()", - "unpauseRebase()" - ]; + string[4] memory signatures = + ["unpauseCapital(address)", "unpauseRebase(address)", "unpauseCapital()", "unpauseRebase()"]; for (uint256 i = 0; i < signatures.length; i++) { - (bool success,) = address(pauseSafeModule).call( - abi.encodeWithSelector(bytes4(keccak256(bytes(signatures[i]))), address(oethVault)) - ); + (bool success,) = address(pauseSafeModule) + .call(abi.encodeWithSelector(bytes4(keccak256(bytes(signatures[i]))), address(oethVault))); assertFalse(success, signatures[i]); } } diff --git a/contracts/tests/unit/automation/PauseSafeModule/concrete/TargetManagement.t.sol b/contracts/tests/unit/automation/PauseSafeModule/concrete/TargetManagement.t.sol index 2dff258b44..4e2c438e68 100644 --- a/contracts/tests/unit/automation/PauseSafeModule/concrete/TargetManagement.t.sol +++ b/contracts/tests/unit/automation/PauseSafeModule/concrete/TargetManagement.t.sol @@ -37,10 +37,22 @@ contract Unit_Concrete_PauseSafeModule_TargetManagement_Test is Unit_PauseSafeMo function test_allowTarget_revertsForZeroAddress() public { vm.prank(address(mockSafe)); - vm.expectRevert("Invalid target"); + vm.expectRevert("Target has no code"); pauseSafeModule.allowTarget(address(0)); } + /// @dev A Safe module call to a codeless address returns success, so an EOA + /// target would let `_execPause` emit its event and revert nothing while + /// pausing nothing. Rejecting it at allow-list time is what keeps the + /// "pause failures revert" property honest. + function test_allowTarget_revertsForEOA() public { + vm.prank(address(mockSafe)); + vm.expectRevert("Target has no code"); + pauseSafeModule.allowTarget(alice); + + assertFalse(pauseSafeModule.isPausableTarget(alice)); + } + function test_allowTarget_revertsWhenAlreadyAllowed() public { vm.prank(address(mockSafe)); vm.expectRevert("Target already allowed"); diff --git a/contracts/tests/unit/vault/OETHVault/concrete/Admin.t.sol b/contracts/tests/unit/vault/OETHVault/concrete/Admin.t.sol index cb2aa59ad9..12877efa92 100644 --- a/contracts/tests/unit/vault/OETHVault/concrete/Admin.t.sol +++ b/contracts/tests/unit/vault/OETHVault/concrete/Admin.t.sol @@ -108,6 +108,40 @@ contract Unit_Concrete_OETHVault_Admin_Test is Unit_OETHVault_Shared_Test { oethVault.setOperatorAddr(operator); } + ////////////////////////////////////////////////////// + /// --- SETADMINADDR + ////////////////////////////////////////////////////// + + function test_setAdminAddr_works() public { + vm.prank(governor); + oethVault.setAdminAddr(alice); + assertEq(oethVault.adminAddr(), alice); + } + + function test_setAdminAddr_emitsEvent() public { + vm.prank(governor); + vm.expectEmit(true, true, true, true); + emit IVault.AdminUpdated(alice); + oethVault.setAdminAddr(alice); + } + + /// @dev Unlike the Operator, the Admin cannot be switched off with the zero + /// address — that would leave the Governor as the only account able to + /// lift a pause. + function test_setAdminAddr_RevertWhen_zeroAddress() public { + vm.prank(governor); + vm.expectRevert("Invalid admin"); + oethVault.setAdminAddr(address(0)); + + assertEq(oethVault.adminAddr(), guardian); + } + + function test_setAdminAddr_RevertWhen_notGovernor() public { + vm.prank(strategist); + vm.expectRevert("Caller is not the Governor"); + oethVault.setAdminAddr(alice); + } + ////////////////////////////////////////////////////// /// --- SETDEFAULTSTRATEGY ////////////////////////////////////////////////////// diff --git a/contracts/tests/unit/vault/OUSDVault/concrete/Admin.t.sol b/contracts/tests/unit/vault/OUSDVault/concrete/Admin.t.sol index 653a49ae98..7cf0ed796d 100644 --- a/contracts/tests/unit/vault/OUSDVault/concrete/Admin.t.sol +++ b/contracts/tests/unit/vault/OUSDVault/concrete/Admin.t.sol @@ -316,6 +316,40 @@ contract Unit_Concrete_OUSDVault_Admin_Test is Unit_Shared_Test { ousdVault.setOperatorAddr(operator); } + ////////////////////////////////////////////////////// + /// --- SETADMINADDR + ////////////////////////////////////////////////////// + + function test_setAdminAddr_governor() public { + vm.prank(governor); + ousdVault.setAdminAddr(alice); + assertEq(ousdVault.adminAddr(), alice); + } + + function test_setAdminAddr_emitsEvent() public { + vm.prank(governor); + vm.expectEmit(true, true, true, true); + emit IVault.AdminUpdated(alice); + ousdVault.setAdminAddr(alice); + } + + /// @dev Unlike the Operator, the Admin cannot be switched off with the zero + /// address — that would leave the Governor as the only account able to + /// lift a pause. + function test_setAdminAddr_RevertWhen_zeroAddress() public { + vm.prank(governor); + vm.expectRevert("Invalid admin"); + ousdVault.setAdminAddr(address(0)); + + assertEq(ousdVault.adminAddr(), guardian); + } + + function test_setAdminAddr_RevertWhen_unauthorized() public { + vm.prank(alice); + vm.expectRevert("Caller is not the Governor"); + ousdVault.setAdminAddr(alice); + } + ////////////////////////////////////////////////////// /// --- SETSTRATEGISTADDR ////////////////////////////////////////////////////// diff --git a/contracts/tests/utils/Addresses.sol b/contracts/tests/utils/Addresses.sol index 46594026fb..417ca718c3 100644 --- a/contracts/tests/utils/Addresses.sol +++ b/contracts/tests/utils/Addresses.sol @@ -258,6 +258,9 @@ library Base { address internal constant WETH = 0x4200000000000000000000000000000000000006; address internal constant wethAeroPoolAddress = 0x80aBe24A3ef1fc593aC5Da960F232ca23B2069d0; address internal constant governor = 0x92A19381444A001d62cE67BaFF066fA1111d7202; + /// @dev 5/8 Multisig, holder of the vaults' `adminAddr`. The same Safe as the + /// governor above; aliased rather than repeated so the two cannot drift. + address internal constant admin = governor; address internal constant strategist = 0x28bce2eE5775B652D92bB7c2891A89F036619703; address internal constant timelock = 0xf817cb3092179083c48c014688D98B72fB61464f; address internal constant multichainStrategist = 0x4FF1b9D9ba8558F5EAfCec096318eA0d8b541971; diff --git a/contracts/utils/addresses.js b/contracts/utils/addresses.js index f7013c7ec3..7246568016 100644 --- a/contracts/utils/addresses.js +++ b/contracts/utils/addresses.js @@ -424,8 +424,9 @@ addresses.base.WETH = "0x4200000000000000000000000000000000000006"; addresses.base.wethAeroPoolAddress = "0x80aBe24A3ef1fc593aC5Da960F232ca23B2069d0"; addresses.base.governor = "0x92A19381444A001d62cE67BaFF066fA1111d7202"; -// 5/8 Multisig. Same Safe. -addresses.base.admin = "0x92A19381444A001d62cE67BaFF066fA1111d7202"; +// 5/8 Multisig. The same Safe as the governor above; aliased rather than +// repeated so the two can never drift apart. +addresses.base.admin = addresses.base.governor; // 2/8 Multisig addresses.base.strategist = "0x28bce2eE5775B652D92bB7c2891A89F036619703"; addresses.base.timelock = "0xf817cb3092179083c48c014688D98B72fB61464f"; From a96133fa41fdf92d1d81231cb63c13057476a51a Mon Sep 17 00:00:00 2001 From: Shahul Hameed <10547529+shahthepro@users.noreply.github.com> Date: Fri, 14 Aug 2026 16:57:36 +0400 Subject: [PATCH 5/5] Fix CI --- .../deploy/base/001_VaultAdminRole.s.sol | 2 +- .../deploy/base/002_PauseSafeModule.s.sol | 2 +- .../deploy/mainnet/005_VaultAdminRole.s.sol | 4 +- .../deploy/mainnet/006_PauseSafeModule.s.sol | 2 +- contracts/scripts/test/layout-pinned.test.js | 3 +- .../tests/utils/artifacts/Automation.sol | 3 +- vault-loss-scenarios-and-freeze.md | 207 ++++++++++++++++++ 7 files changed, 215 insertions(+), 8 deletions(-) create mode 100644 vault-loss-scenarios-and-freeze.md diff --git a/contracts/scripts/deploy/base/001_VaultAdminRole.s.sol b/contracts/scripts/deploy/base/001_VaultAdminRole.s.sol index ad48b79d50..e55898d452 100644 --- a/contracts/scripts/deploy/base/001_VaultAdminRole.s.sol +++ b/contracts/scripts/deploy/base/001_VaultAdminRole.s.sol @@ -33,7 +33,7 @@ contract $001_VaultAdminRole is AbstractDeployScript("001_VaultAdminRole") { function _execute() internal override { OETHBaseVault oethbVaultImpl = new OETHBaseVault(BaseAddresses.WETH); - _recordDeployment("OETHBASE_VAULT_IMPL", address(oethbVaultImpl)); + _recordDeployment("OETHBASE_VAULT_IMPL", address(oethbVaultImpl), type(OETHBaseVault).name); } // ==================== Governance Proposal ==================== // diff --git a/contracts/scripts/deploy/base/002_PauseSafeModule.s.sol b/contracts/scripts/deploy/base/002_PauseSafeModule.s.sol index 87672165e5..97d16dd196 100644 --- a/contracts/scripts/deploy/base/002_PauseSafeModule.s.sol +++ b/contracts/scripts/deploy/base/002_PauseSafeModule.s.sol @@ -39,7 +39,7 @@ contract $002_PauseSafeModule is AbstractDeployScript("002_PauseSafeModule") { targets[0] = resolver.resolve("OETHBASE_VAULT_PROXY"); PauseSafeModule pauseSafeModule = new PauseSafeModule(CrossChain.multichainStrategist, operators, targets); - _recordDeployment("PAUSE_SAFE_MODULE", address(pauseSafeModule)); + _recordDeployment("PAUSE_SAFE_MODULE", address(pauseSafeModule), type(PauseSafeModule).name); } // ==================== Fork Verification ==================== // diff --git a/contracts/scripts/deploy/mainnet/005_VaultAdminRole.s.sol b/contracts/scripts/deploy/mainnet/005_VaultAdminRole.s.sol index 35d4402e35..765811f3bb 100644 --- a/contracts/scripts/deploy/mainnet/005_VaultAdminRole.s.sol +++ b/contracts/scripts/deploy/mainnet/005_VaultAdminRole.s.sol @@ -36,10 +36,10 @@ contract $005_VaultAdminRole is AbstractDeployScript("005_VaultAdminRole") { function _execute() internal override { OUSDVault ousdVaultImpl = new OUSDVault(Mainnet.USDC); - _recordDeployment("OUSD_VAULT_IMPL", address(ousdVaultImpl)); + _recordDeployment("OUSD_VAULT_IMPL", address(ousdVaultImpl), type(OUSDVault).name); OETHVault oethVaultImpl = new OETHVault(Mainnet.WETH); - _recordDeployment("OETH_VAULT_IMPL", address(oethVaultImpl)); + _recordDeployment("OETH_VAULT_IMPL", address(oethVaultImpl), type(OETHVault).name); } // ==================== Governance Proposal ==================== // diff --git a/contracts/scripts/deploy/mainnet/006_PauseSafeModule.s.sol b/contracts/scripts/deploy/mainnet/006_PauseSafeModule.s.sol index 50c5987e53..fa4eb04859 100644 --- a/contracts/scripts/deploy/mainnet/006_PauseSafeModule.s.sol +++ b/contracts/scripts/deploy/mainnet/006_PauseSafeModule.s.sol @@ -45,7 +45,7 @@ contract $006_PauseSafeModule is AbstractDeployScript("006_PauseSafeModule") { targets[1] = resolver.resolve("OETH_VAULT_PROXY"); PauseSafeModule pauseSafeModule = new PauseSafeModule(CrossChain.multichainStrategist, operators, targets); - _recordDeployment("PAUSE_SAFE_MODULE", address(pauseSafeModule)); + _recordDeployment("PAUSE_SAFE_MODULE", address(pauseSafeModule), type(PauseSafeModule).name); } // ==================== Fork Verification ==================== // diff --git a/contracts/scripts/test/layout-pinned.test.js b/contracts/scripts/test/layout-pinned.test.js index 177d5257b1..73946e0a48 100644 --- a/contracts/scripts/test/layout-pinned.test.js +++ b/contracts/scripts/test/layout-pinned.test.js @@ -76,7 +76,8 @@ const PINS = { "79|24|rebasePerSecondTarget|uint64|8", "80|0|defaultStrategy|address|20", "81|0|operatorAddr|address|20", - "82|0|__gap|uint256[41]|1312", + "82|0|adminAddr|address|20", + "83|0|__gap|uint256[40]|1280", "123|0|_deprecated_wethAssetIndex|uint256|32", ], diff --git a/contracts/tests/utils/artifacts/Automation.sol b/contracts/tests/utils/artifacts/Automation.sol index 431562386c..242935e83b 100644 --- a/contracts/tests/utils/artifacts/Automation.sol +++ b/contracts/tests/utils/artifacts/Automation.sol @@ -18,6 +18,5 @@ library Automation { "contracts/automation/EthereumBridgeHelperModule.sol:EthereumBridgeHelperModule"; string internal constant MERKL_POOL_BOOSTER_BRIBES_MODULE = "contracts/automation/MerklPoolBoosterBribesModule.sol:MerklPoolBoosterBribesModule"; - string internal constant PAUSE_SAFE_MODULE = - "contracts/automation/PauseSafeModule.sol:PauseSafeModule"; + string internal constant PAUSE_SAFE_MODULE = "contracts/automation/PauseSafeModule.sol:PauseSafeModule"; } diff --git a/vault-loss-scenarios-and-freeze.md b/vault-loss-scenarios-and-freeze.md new file mode 100644 index 0000000000..212e2a2b26 --- /dev/null +++ b/vault-loss-scenarios-and-freeze.md @@ -0,0 +1,207 @@ +# OToken Vault: loss scenarios, freeze behavior, and options + +Scope: what could trigger a loss, what the vault does during a loss, how the withdrawal circuit breaker (`maxSupplyDiff`) freezes and unfreezes, the cases at each loss size, and the options on the table with their tradeoffs. + +## What could trigger a capital loss + +A capital loss is anything that makes gross assets `A` fall below supply `S`. It shows up as a strategy's `checkBalance` dropping, or the vault's own asset balance falling short. From there it flows through the mechanics below. The triggers depend on the strategies each vault runs. + +**OETH (backed by WETH).** Mainly native ETH staking through SSV validators, plus a Curve AMO. + +- Validator slashing. A validator commits a slashable offense and loses part of its 32 ETH stake. A correlated event (a client bug, or one operator slashed across many validators) is the tail case. This drops the native staking strategy's ETH directly. +- Validator penalties and downtime. Missed attestations or an inactivity leak during non-finality. A slow bleed, not a spike. +- AMO pool imbalance. The Curve AMO can lose value if the OETH/ETH pool depegs while it holds the heavy side, similar to impermanent loss. Its own moves are guarded: liquidity operations are slippage-capped (max 5%) and a solvency check blocks any deposit, withdrawal, or rebalance that would push the vault below ~99.8% backed. Those guards don't stop an external depeg or a pool exploit, which is the residual risk. + +**OUSD (backed by USDC).** Mainly Morpho lending markets across Ethereum, Base, and HyperEVM, plus cross-chain movement of USDC. + +- Bad debt in a Morpho market. Collateral crashes faster than liquidators can act, or liquidation is blocked by illiquidity, leaving bad debt. As a supplier, the vault can't withdraw full principal. +- Oracle failure in a market. A wrong or manipulated price lets borrowers over-borrow against bad collateral, creating bad debt that hits suppliers. +- Market misconfiguration. A risky market (bad LLTV, thin collateral) or a bad curator allocation loses supplied funds. +- Cross-chain risk. USDC moved across chains (CCTP or the cross-chain strategy) could be stuck or lost if a bridge or a remote market fails. + +**Applies to every vault.** + +- Smart contract exploit. A bug in a strategy, an underlying protocol (Morpho, Curve, SSV, Aerodrome), or the vault drains funds. Usually large enough to land in the catastrophic band. +- Accounting or oracle error. A strategy over-reports `checkBalance` (stale price, a reward token marked too high), so the vault looks solvent until the correction realizes the loss. This is the "strategy was off on its asset total" case the code comments already call out. +Severity tends to split by speed: + +- Slow bleeds (penalties, small bad debt, minor pool imbalance) tend to stay in the mild band, where the socialization and freeze mechanics below apply. +- Sudden large losses (an exploit, a mass slashing) tend to blow past the threshold into the catastrophic band, where the guardian pause and strategy-level recovery matter more than the withdrawal accounting. + +## How the safeguard works + +The check lives in `_postRedeem()` and runs on `requestWithdrawal`, `claimWithdrawal`, and `claimWithdrawals`. It does **not** run on `mint` or `mintForStrategy`. + +Terms: + +``` +A grossAssets = asset in the vault + all strategies (not reduced by the queue) +q outstandingQueue = queued − claimed (promised to the queue, not yet paid) +S liveSupply = oToken.totalSupply() (queued requests are already burned, so not counted) +V netValue = A − q (what the contract calls totalValue, floored at 0) +d maxSupplyDiff = 3% OETH, 5% OUSD +``` + +The check is: + +``` +diff = S / V +require( |diff − 1| <= d ) // else revert "Backing supply liquidity error" +require( V > 0 ) // else revert "Too many outstanding requests" +``` + +One subtlety: the percentage is measured against value `V`, not supply `S`. So a 3% `maxSupplyDiff` trips when backing falls to `S / 1.03 = 97.09%` of supply, which is a **2.91%** loss relative to supply. For 5% it's a **4.76%** loss. + +## What moves the insolvency measure + +Each action moves `diff` in one direction: + +- **Loss** (A falls): `diff` up. +- **Request a withdrawal**: in an impaired vault, `diff` up. In a healthy vault, no change. +- **Claim a withdrawal**: no change. The claim drops A and q by the same amount, so V is unchanged and S is unchanged. +- **Mint**: `diff` down, toward 1. Fresh assets come in 1:1 against new supply and dilute the shortfall. +- **Rebase**: mints new supply to distribute yield, raising supply toward value. `diff` toward 1, but only when over-backed, since rebase never lowers supply. +- **Recovery** (A rises, from yield, dripper, or a top-up): `diff` down. + +Two facts fall out of this: + +1. The freeze builds up from **requests accumulating**, not from claims. Even a small loss can freeze the vault if enough people queue. +2. A **mint can unfreeze** the vault, because it moves `diff` the opposite way from a request. + +## Solvency bands (OETH, d = 3%, supply normalized to 100) + +| Band | Backing (V/S) | Requests | Claims | Mint | State | +|------|---------------|----------|--------|------|-------| +| Healthy | 100% | ok, 1:1 | ok, 1:1 | ok | Normal. No socialization. | +| Mild (in band) | 97.09% to 100% | ok until requests push diff to the edge | ok, 1:1 | ok (lowers diff) | Early exiters escape. Stayers absorb a concentrated loss. | +| At/over threshold | below 97.09% | revert | revert | still open (can unfreeze) | Frozen. Value trapped. Governance situation. | +| Queue > assets | V floored to 0 | revert | revert | still open | Deep insolvency. "Too many outstanding requests". | +| Over-backed surplus | above ~103% | revert | revert | ok | Rare. One-off surplus above the threshold, not yet rebased out. Self-heals as the vault rebases. | + +OUSD is the same shape with d = 5%: freeze at 95.24% backing, max stayer loss 4.76%. + +On over-backing and the dripper: the vault's inbuilt yield smoothing is what normally keeps value glued to supply, which is why the over-backed row almost never fires. Smoothing is now internal to the vault (the rebase rate-limiter, using `dripDuration`, a per-second cap, and a ~2% hard cap per rebase). The old external Dripper slot is deprecated. The catch is that it rate-limits the rebase, not the value coming in, so harvested yield is counted the moment it lands. A one-off surplus larger than the threshold (a big reward sale, a base-asset donation, or a strategy revaluation) can still tip the vault over-backed before rebases distribute it. It self-heals, since `rebase()` doesn't run the solvency check and keeps dripping the surplus out until backing is back under the threshold. + +## Worst case in the mild band + +The mild band is where the loss is split unevenly. Early requesters get paid 1:1 and escape their share of the loss. The remaining holders absorb it, concentrated. + +At the freeze point, remaining holders are always left at exactly `1/(1+d)` backing, no matter how small the original loss was. For OETH that's 97.09%. The size of the loss just decides **how many** holders escape first. + +Loss `X` is on the 100-supply basis. Escapable queue before freeze is `Q_max = S − ((1+d)/d)·X`. + +| Loss X | Can escape at 1:1 | Stayers left | Stayer backing | Stayer loss vs fair share | +|--------|-------------------|--------------|----------------|----------------------------| +| 0.5% | 82.8 | 17.2 | 97.09% | ~2.91% vs 0.5% (about 6x) | +| 1% | 65.7 | 34.3 | 97.09% | ~2.91% vs 1% (about 3x) | +| 2% | 31.3 | 68.7 | 97.09% | ~2.91% vs 2% (about 1.5x) | +| 2.91% | ~0 | ~100 | 97.09% | shared equally, no escape | + +A tiny 0.5% loss can still push the last stayers down to 97.09% backing if enough people escape first. The absolute loss is small, but it's concentrated onto whoever didn't move. + +Above 2.91%, the vault is already frozen before anyone can queue. Nobody escapes, everyone sits at the same backing, and the loss is shared equally. So the uneven split is specific to the mild band. In the catastrophic band the freeze shares the loss evenly on its own. + +The mild band has one more property. A party who knows a loss is about to be booked (an oracle update, an LST slashing report) can request just before it lands and claim just after, locking in 1:1 and escaping their share. This is bounded by the same threshold. + +## How the freeze happens + +Two different "freezes" exist. Keep them separate. + +**1. Automatic threshold freeze (`_postRedeem`).** +Triggered by the math, not a person. `diff` crosses `1 + d` and requests plus claims start reverting. Causes: + +- A loss large enough on its own (over 2.91% for OETH). +- A smaller loss plus enough queued requests to push `diff` over the edge. +- A deep shortfall where the queue exceeds total assets (`V` floors to 0). + +Mint, rebase, and allocate still work during this freeze. + +**2. Guardian pause (`whenNotCapitalPaused`).** +Deliberate. The guardian pauses the vault and blocks mint, request, and claim together. This is the manual emergency stop, used when something is actively wrong (a strategy exploit, a bad oracle). It's a policy tool, not tied to `diff`. + +## How the freeze unfreezes + +The automatic threshold freeze lifts when `diff` falls back under `1 + d`. The paths: + +- **Organic recovery.** Assets earn back, the dripper releases, or a positive rebase raises value. No action needed, but slow and not guaranteed. +- **Capital injection.** Treasury, insurance, or a donation raises A. Makes holders whole, but needs real funds. +- **Governance widens or disables `maxSupplyDiff`.** Fast, but it reopens 1:1 claims, so whoever moves first escapes. The tradeoff is trapped funds versus first-mover escape. +- **A mint.** Lowers `diff` and can lift the freeze on its own. The minter ends up subsidizing the queue by buying an under-backed token. + +The guardian pause lifts only by a governance or guardian action. Nothing moves while paused. + +Note: the freeze only stops withdrawal-driven socialization. It does not stop the underlying loss from getting worse. If a strategy keeps bleeding while frozen, backing keeps falling below 97.09%. The threshold caps the socialized transfer, not the total loss. + +## Options and tradeoffs + +Four options have come up. They are not mutually exclusive. + +### Option A: Document the behavior, no code change + +Describe the current loss handling in the README and on Immunefi as known, intended behavior. No contract change. + +Pros: + +- No code risk. Preserves long-tested mint, redeem, and withdraw paths. +- Neutralizes the "is this a bug" question for bug bounties. A documented behavior is not a payable finding. +- Fast. Keeps the OSDV3 audit scope minimal. +- Respects that loss-sharing fairness is subjective. Discloses the policy and lets users manage their own risk. + +Cons: + +- Does not remove the mild-band extraction (OEV). Disclosure makes it known, not gone. +- Leaves the mint path able to unfreeze the vault and let 1:1 claims resume. +- Remaining holders still bear a concentrated loss in the mild band. +- Keeps leaning on `maxSupplyDiff`, which also serves as a rebase limit. One knob, two jobs. + +### Option B: Add mint gating + +Block `mint` (and `mintForStrategy`) when the vault is under-backed past a small tolerance. Reuses the value check already in `_postRedeem`, applied on the way in, one-sided. + +Pros: + +- Closes the mint unfreeze path. A mint can no longer lift the circuit breaker. +- Protects the minter from buying a token worth less than they paid. +- Stops fresh mint assets from funding the old queue at par. +- Small (about 15 to 25 lines), one-sided, never fires when healthy. Additive to the existing system. + +Cons: + +- The minter is the party who loses, so there is no attacker profit motive. It is self-harming, not a classic exploit. +- Adds a new revert path to a function that has always worked. +- Deep in the frozen zone, unfreezing by mint needs an impractically large mint, so this path mostly matters near the threshold. +- Gating `mintForStrategy` could block AMO rebalancing during a loss, when the strategist may need it. +- Not present since inception, with no incident to date. + +### Option C: Full loss socialization (haircut on claim) + +Stop paying queued withdrawals a fixed 1:1. Pay `min(requested, requested × backing ratio)` at claim, using effective supply (live plus queued) as the denominator. Possibly rework the FIFO gate so claims don't jam in deep impairment. + +Pros: + +- Shares losses across queued and remaining holders instead of concentrating them on stayers. +- Removes the mild-band OEV. A late claim can no longer escape at 1:1. +- Claims self-correct instead of freezing, which removes the freeze-versus-widen governance dilemma. + +Cons: + +- Large, invasive change to battle-tested code. Highest risk of introducing a new bug. +- Changes redemption semantics. Payouts can be below 1:1, which is a product and peg decision, not just an engineering one. +- Fairness is subjective. A queued user can argue they locked in an exit. +- Bigger audit scope. +- Only changes outcomes in the mild band. The threshold already caps stayer loss, and the catastrophic band is already shared evenly. + +### Option D: Retune or decouple maxSupplyDiff + +Lower the threshold for an earlier freeze, or split it from its rebase-limit role so the two can be set independently. + +Pros: + +- A smaller threshold freezes sooner, so less loss is socialized before withdrawals stop. +- Decoupling avoids one knob controlling both the freeze and rebase smoothing. + +Cons: + +- A smaller threshold freezes more easily, trapping funds sooner and on smaller moves. +- Still a freeze, not loss sharing. It changes when the stop happens, not who bears the loss. +- Retuning the current knob affects rebase behavior unless it is decoupled first.