diff --git a/l2-contracts/test/unit/L2RewardManager.t.sol b/l2-contracts/test/unit/L2RewardManager.t.sol index 3aa043a1..bb0a54b9 100644 --- a/l2-contracts/test/unit/L2RewardManager.t.sol +++ b/l2-contracts/test/unit/L2RewardManager.t.sol @@ -16,11 +16,7 @@ import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils import { BridgeMock } from "../mocks/BridgeMock.sol"; import { Merkle } from "murky/Merkle.sol"; import { - ROLE_ID_BRIDGE, - PUBLIC_ROLE, - ROLE_ID_DAO, - ROLE_ID_REWARD_WATCHER, - ROLE_ID_OPERATIONS_PAYMASTER + ROLE_ID_BRIDGE, PUBLIC_ROLE, ROLE_ID_DAO, ROLE_ID_OPERATIONS_PAYMASTER } from "mainnet-contracts/script/Roles.sol"; import { XERC20Lockbox } from "mainnet-contracts/src/XERC20Lockbox.sol"; import { xPufETH } from "mainnet-contracts/src/l2/xPufETH.sol"; @@ -162,7 +158,6 @@ contract L2RewardManagerTest is Test { (s,) = address(accessManager).call(cd); require(s, "failed access manager 2"); - accessManager.grantRole(ROLE_ID_REWARD_WATCHER, address(this), 0); accessManager.grantRole(ROLE_ID_DAO, address(this), 0); accessManager.grantRole(ROLE_ID_OPERATIONS_PAYMASTER, address(this), 0); diff --git a/mainnet-contracts/script/AccessManagerMigrations/07_AccessManagerCleanup.s.sol b/mainnet-contracts/script/AccessManagerMigrations/07_AccessManagerCleanup.s.sol new file mode 100644 index 00000000..cc97a4cd --- /dev/null +++ b/mainnet-contracts/script/AccessManagerMigrations/07_AccessManagerCleanup.s.sol @@ -0,0 +1,111 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity >=0.8.0 <0.9.0; + +// - Guardian role 88 is unused +// - Puffer Oracle role 999 is unused +// - Reward Watcher 21 role is unused +// - Conflicting role ID 25::Granted VT pricer role 25 to MergedAdapterWithoutRoundsPufStakingV1 proxy - 0xf9dfbf71f2d9c8a4e565e1346aeb2c3e1dc765de +// - Currently Active on ValidatorTicketPricer - 0x9830ad1bd5cf73640e253edf97dee3791c4a53c3 + +// - Add PufferOracleV2 - 0x8eFd1Dc43AD073232F3e2924e22F173879119489 to ACL? +// - There is another PufferOracle 0x0be2ae0edbebb517541df217ef0074fc9a9e994f also +// - PufferOracleV2 has PufferProtocol 1234 role for few functions like provisionNode and exitValidators +// - Granted function role on v2 on setMintPrice + +// - DUPLICATE OperationsCoordinator 0xe6d798165a32f37927af20d7ccb1f80fb731a3c0 +// - Original OperationsCoordinator- 0x3fee92765f5cf8f9909a3c89f4907ea5e1cd9bf7 +// - Granted Operations Coordinator 24 role to duplicate one +// - Granted function role setValidatorTicketMintPrice on duplicate one + +// - Upgrader role 1 is granted to 0x0000000000000000000000000000000000000000 contract for upgradeToAndCall(address,bytes) function +// - Lockbox no address on L1? + +// TODO: + +// create a PR to do cleanup & remove unused roles from the code +// DONE - fix the conflict for role 25 by creating a new role +// DONE - create a migration script like 06_AccessManagerCleanup.s.sol +// DONE - that script should revoke any roles from the contracts that we don't use anymore (oracle, operations coordinator) + +// DONE - ROLE_ID_LOCKBOX seems unused as well. +// on xPufETH there is `setLockbox` which is restricted to DAO, we don't need this role anymore. + +// Upgrader role is useless as well, we should revoke it + +import { Script } from "forge-std/Script.sol"; +import { AccessManager } from "@openzeppelin/contracts/access/manager/AccessManager.sol"; +import { Multicall } from "@openzeppelin/contracts/utils/Multicall.sol"; +import { ROLE_ID_OPERATIONS_COORDINATOR } from "../../script/Roles.sol"; +import { DeployerHelper } from "../../script/DeployerHelper.s.sol"; +import { console } from "forge-std/console.sol"; +import { PufferWithdrawalManager } from "../../src/PufferWithdrawalManager.sol"; +import { ROLE_ID_VT_PRICER, ROLE_ID_WITHDRAWAL_FINALIZER } from "../../script/Roles.sol"; + +contract AccessManagerCleanup is DeployerHelper { + function run() public view { + address duplicateOperationsCoordinator = 0xe6d798165A32F37927AF20D7Ccb1f80fB731a3C0; + address withdrawalManagerProxy = _getPufferWithdrawalManager(); + + address paymaster = _getPaymaster(); + address withdrawalFinalizer = _getOPSMultisig(); + address communityMultisig = _getCommunityMultisig(); + + uint64 ROLE_ID_UPGRADER = 1; + + // ------------ 1. Revoke existing ROLE_ID_WITHDRAWAL_FINALIZER roles and re-grant as we have changed the role ID which was 25 earlier and now 27 ------------ + bytes[] memory calldatas = new bytes[](9); + + // 1.1 set target role for finalizeWithdrawals + bytes4[] memory paymasterSelectors = new bytes4[](1); + paymasterSelectors[0] = PufferWithdrawalManager.finalizeWithdrawals.selector; + + calldatas[0] = abi.encodeWithSelector( + AccessManager.setTargetFunctionRole.selector, + withdrawalManagerProxy, + paymasterSelectors, + ROLE_ID_WITHDRAWAL_FINALIZER + ); + // 1.2 revoke existing roles + calldatas[1] = + abi.encodeWithSelector(AccessManager.revokeRole.selector, ROLE_ID_WITHDRAWAL_FINALIZER, paymaster); + calldatas[2] = + abi.encodeWithSelector(AccessManager.revokeRole.selector, ROLE_ID_WITHDRAWAL_FINALIZER, withdrawalFinalizer); + + // 1.3 grant finalizer roles to paymaster and withdrawalFinalizer addresses + calldatas[3] = + abi.encodeWithSelector(AccessManager.grantRole.selector, ROLE_ID_WITHDRAWAL_FINALIZER, paymaster, 0); + + calldatas[4] = abi.encodeWithSelector( + AccessManager.grantRole.selector, ROLE_ID_WITHDRAWAL_FINALIZER, withdrawalFinalizer, 0 + ); + + // 1.4 label role + calldatas[5] = abi.encodeWithSelector( + AccessManager.labelRole.selector, ROLE_ID_WITHDRAWAL_FINALIZER, "Withdrawal Finalizer" + ); + + // 1.5 label role 25 as VT pricer + calldatas[6] = + abi.encodeWithSelector(AccessManager.labelRole.selector, ROLE_ID_VT_PRICER, "Validator Ticket Pricer"); + // ------------------- + + // 2. Revoke Operations Coordinator role from duplicate contract + // 2.1 revoke role from duplicate contract + calldatas[7] = abi.encodeWithSelector( + AccessManager.revokeRole.selector, ROLE_ID_OPERATIONS_COORDINATOR, duplicateOperationsCoordinator + ); + // NOTE: this duplicate contract has setValidatorTicketMintPrice function which has been granted Operations Paymaster (ID: 23) role + + // 3. Revoke PufferOracle role from PufferOracleV2 + // NOTE: PufferOracleV2 has PufferProtocol 1234 role for few functions like provisionNode and exitValidators, so we can skip it, or can change it some unaccessible role? + + // 4. revoke uint64 constant ROLE_ID_UPGRADER = 1; from Community Multisig 0x446d4d6b26815f9ba78b5d454e303315d586cb2a + calldatas[8] = abi.encodeWithSelector(AccessManager.revokeRole.selector, ROLE_ID_UPGRADER, communityMultisig); + + bytes memory encodedMulticall = abi.encodeCall(Multicall.multicall, (calldatas)); + + console.log("Timelock -> AccessManager"); + console.log("AccessManagerCleanup calldatas:"); + console.logBytes(encodedMulticall); + } +} diff --git a/mainnet-contracts/script/DeployerHelper.s.sol b/mainnet-contracts/script/DeployerHelper.s.sol index 50b604c3..ac7faaf1 100644 --- a/mainnet-contracts/script/DeployerHelper.s.sol +++ b/mainnet-contracts/script/DeployerHelper.s.sol @@ -488,6 +488,19 @@ abstract contract DeployerHelper is Script { } function _getPaymaster() internal view returns (address) { + if (block.chainid == mainnet) { + // https://etherscan.io/address/0xb4F1f4d59bd7590e92c386083Aa260C1e09cC58b + return 0xb4F1f4d59bd7590e92c386083Aa260C1e09cC58b; + } else if (block.chainid == holesky) { + // https://holesky.etherscan.io/address/0xDDDeAfB492752FC64220ddB3E7C9f1d5CcCdFdF0 + return 0xDDDeAfB492752FC64220ddB3E7C9f1d5CcCdFdF0; + } + + revert("Paymaster not available for this chain"); + } + // Keep this address for revoking old paymaster in future + + function _getDeprecatedPaymaster() internal view returns (address) { if (block.chainid == mainnet) { // https://etherscan.io/address/0x65d2dd7A66a2733a36559fE900A236280A05FBD6 return 0x65d2dd7A66a2733a36559fE900A236280A05FBD6; @@ -582,4 +595,13 @@ abstract contract DeployerHelper is Script { revert("RevenueDepositor not available for this chain"); } + + function _getPufferWithdrawalManager() internal view returns (address) { + if (block.chainid == mainnet) { + // https://etherscan.io/address/0xDdA0483184E75a5579ef9635ED14BacCf9d50283 + return 0xDdA0483184E75a5579ef9635ED14BacCf9d50283; + } + + revert("PufferWithdrawalManager not available for this chain"); + } } diff --git a/mainnet-contracts/script/Roles.sol b/mainnet-contracts/script/Roles.sol index f969b0b8..326a972f 100644 --- a/mainnet-contracts/script/Roles.sol +++ b/mainnet-contracts/script/Roles.sol @@ -7,15 +7,15 @@ pragma solidity >=0.8.0 <0.9.0; uint64 constant ROLE_ID_UPGRADER = 1; uint64 constant ROLE_ID_L1_REWARD_MANAGER = 20; -uint64 constant ROLE_ID_REWARD_WATCHER = 21; + uint64 constant ROLE_ID_OPERATIONS_MULTISIG = 22; uint64 constant ROLE_ID_OPERATIONS_PAYMASTER = 23; uint64 constant ROLE_ID_OPERATIONS_COORDINATOR = 24; -uint64 constant ROLE_ID_WITHDRAWAL_FINALIZER = 25; -uint64 constant ROLE_ID_REVENUE_DEPOSITOR = 26; // Role assigned to validator ticket price setter uint64 constant ROLE_ID_VT_PRICER = 25; +uint64 constant ROLE_ID_REVENUE_DEPOSITOR = 26; +uint64 constant ROLE_ID_WITHDRAWAL_FINALIZER = 27; // Role assigned to the Puffer Protocol uint64 constant ROLE_ID_PUFFER_PROTOCOL = 1234; @@ -23,8 +23,6 @@ uint64 constant ROLE_ID_VAULT_WITHDRAWER = 1235; uint64 constant ROLE_ID_PUFETH_BURNER = 1236; uint64 constant ROLE_ID_DAO = 77; -uint64 constant ROLE_ID_GUARDIANS = 88; -uint64 constant ROLE_ID_PUFFER_ORACLE = 999; // Public role (defined in AccessManager.sol) uint64 constant PUBLIC_ROLE = type(uint64).max; @@ -34,8 +32,5 @@ uint64 constant ADMIN_ROLE = 0; // Allowlister role for AVSContractsRegistry uint64 constant ROLE_ID_AVS_COORDINATOR_ALLOWLISTER = 5; -// Lockbox role for ETH Mainnet -uint64 constant ROLE_ID_LOCKBOX = 7; - // Bridge role for L2RewardManager uint64 constant ROLE_ID_BRIDGE = 8; diff --git a/mainnet-contracts/test/helpers/UnitTestHelper.sol b/mainnet-contracts/test/helpers/UnitTestHelper.sol index 03b931b7..27be897a 100644 --- a/mainnet-contracts/test/helpers/UnitTestHelper.sol +++ b/mainnet-contracts/test/helpers/UnitTestHelper.sol @@ -32,12 +32,7 @@ import { L1RewardManager } from "src/L1RewardManager.sol"; import { PufferRevenueDepositor } from "src/PufferRevenueDepositor.sol"; import { L2RewardManager } from "l2-contracts/src/L2RewardManager.sol"; import { ConnextMock } from "../mocks/ConnextMock.sol"; -import { - ROLE_ID_DAO, - ROLE_ID_OPERATIONS_PAYMASTER, - ROLE_ID_OPERATIONS_MULTISIG, - ROLE_ID_LOCKBOX -} from "../../script/Roles.sol"; +import { ROLE_ID_DAO, ROLE_ID_OPERATIONS_PAYMASTER, ROLE_ID_OPERATIONS_MULTISIG } from "../../script/Roles.sol"; import { GenerateSlashingELCalldata } from "../../script/AccessManagerMigrations/07_GenerateSlashingELCalldata.s.sol"; contract UnitTestHelper is Test, BaseScript { diff --git a/mainnet-contracts/test/unit/ValidatorTicket.t.sol b/mainnet-contracts/test/unit/ValidatorTicket.t.sol index 2432aa4e..d7547cdd 100644 --- a/mainnet-contracts/test/unit/ValidatorTicket.t.sol +++ b/mainnet-contracts/test/unit/ValidatorTicket.t.sol @@ -13,10 +13,10 @@ import { PUBLIC_ROLE, ROLE_ID_PUFETH_BURNER } from "../../script/Roles.sol"; import { Permit } from "../../src/structs/Permit.sol"; import "forge-std/console.sol"; import { Math } from "@openzeppelin/contracts/utils/math/Math.sol"; + /** * @dev This test is for the ValidatorTicket smart contract with `src/PufferOracle.sol` */ - contract ValidatorTicketTest is UnitTestHelper { using ECDSA for bytes32; using Address for address; @@ -104,7 +104,7 @@ contract ValidatorTicketTest is UnitTestHelper { uint256 vtPrice = pufferOracle.getValidatorTicketPrice(); uint256 amount = 5.123 ether; - uint256 expectedTotal = (amount * 1 ether / vtPrice); + uint256 expectedTotal = ((amount * 1 ether) / vtPrice); vm.deal(address(this), amount); uint256 mintedAmount = validatorTicket.purchaseValidatorTicket{ value: amount }(address(this)); @@ -217,7 +217,7 @@ contract ValidatorTicketTest is UnitTestHelper { address recipient = actors[2]; uint256 vtPrice = pufferOracle.getValidatorTicketPrice(); - uint256 requiredETH = vtAmount * vtPrice / 1 ether; + uint256 requiredETH = (vtAmount * vtPrice) / 1 ether; uint256 pufETHToETHExchangeRate = pufferVault.convertToAssets(1 ether); uint256 expectedPufEthUsed = (requiredETH * 1 ether) / pufETHToETHExchangeRate;