feat: wip- acl cleanup script and other cleanup

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);
 

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);
+    }
+}

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");
+    }
 }

mainnet-contracts/script/Roles.sol

@@ -7,24 +7,22 @@ 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;
 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;

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 {

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;