feat(paymaster): add separate whitelistAdmin param to BondTreasuryPaymaster (#106)

The BondTreasuryPaymaster constructor previously coupled DEFAULT_ADMIN_ROLE
and WHITELIST_ADMIN_ROLE to the same 'admin' address. This made it impossible
to assign whitelist management to an operational key (e.g. FLEET_OPERATOR)
while keeping DEFAULT_ADMIN_ROLE on a multisig.

Changes:
- Add 'whitelistAdmin' as the second constructor parameter
- When whitelistAdmin != admin, grant WHITELIST_ADMIN_ROLE to whitelistAdmin
- Update deploy script to pass fleetOperator as whitelistAdmin
- Update all test instantiations and add test_separateWhitelistAdmin
- Update spec doc deployment example

Author: aliXsed

script/DeploySwarmUpgradeableZkSync.s.sol

@@ -107,7 +107,7 @@ contract DeploySwarmUpgradeableZkSync is Script {
         whitelistedUsers[0] = fleetOperator;
         bondTreasuryPaymaster = address(
             new BondTreasuryPaymaster(
-                owner, withdrawer, whitelistedContracts, whitelistedUsers, bondToken, bondQuota, bondPeriod
+                owner, fleetOperator, withdrawer, whitelistedContracts, whitelistedUsers, bondToken, bondQuota, bondPeriod
             )
         );
         console.log("   Address:", bondTreasuryPaymaster);

src/paymasters/BondTreasuryPaymaster.sol

@@ -23,35 +23,41 @@ contract BondTreasuryPaymaster is WhitelistPaymaster, QuotaControl {
 
     constructor(
         address admin,
+        address whitelistAdmin,
         address withdrawer,
         address[] memory initialWhitelistedContracts,
         address[] memory initialWhitelistedUsers,
         address bondToken_,
         uint256 initialQuota,
         uint256 initialPeriod
     ) WhitelistPaymaster(admin, withdrawer) QuotaControl(initialQuota, initialPeriod, admin) {
+        if (whitelistAdmin != admin) {
+            _grantRole(WHITELIST_ADMIN_ROLE, whitelistAdmin);
+        }
         bondToken = IERC20(bondToken_);
+
         uint256 n = initialWhitelistedContracts.length;
         for (uint256 i = 0; i < n; ++i) {
             isWhitelistedContract[initialWhitelistedContracts[i]] = true;
         }
         if (n > 0) {
             emit WhitelistedContractsAdded(initialWhitelistedContracts);
         }
-        uint256 m = initialWhitelistedUsers.length;
-        for (uint256 j = 0; j < m; ++j) {
-            isWhitelistedUser[initialWhitelistedUsers[j]] = true;
-        }
-        if (m > 0) {
-            emit WhitelistedUsersAdded(initialWhitelistedUsers);
-        }
 
         if (!isWhitelistedContract[address(this)]) {
             isWhitelistedContract[address(this)] = true;
             address[] memory selfDest = new address[](1);
             selfDest[0] = address(this);
             emit WhitelistedContractsAdded(selfDest);
         }
+
+        uint256 m = initialWhitelistedUsers.length;
+        for (uint256 j = 0; j < m; ++j) {
+            isWhitelistedUser[initialWhitelistedUsers[j]] = true;
+        }
+        if (m > 0) {
+            emit WhitelistedUsersAdded(initialWhitelistedUsers);
+        }
     }
 
     /// @notice Validate whitelist + consume quota for a sponsored bond.

src/swarms/doc/spec/swarm-specification.md

@@ -660,6 +660,7 @@ address[] memory initialUsers = new address[](1);
 initialUsers[0] = nodleSwarmOperator;
 new BondTreasuryPaymaster(
     admin,
+    whitelistAdmin,
     withdrawer,
     initialContracts,
     initialUsers,

test/paymasters/BondTreasuryPaymaster.t.sol

@@ -42,6 +42,7 @@ contract SponsoredBondPuller {
 contract MockBondTreasuryPaymaster is BondTreasuryPaymaster {
     constructor(
         address admin,
+        address whitelistAdmin,
         address withdrawer,
         address[] memory initialWhitelistedContracts,
         address[] memory initialWhitelistedUsers,
@@ -51,6 +52,7 @@ contract MockBondTreasuryPaymaster is BondTreasuryPaymaster {
     )
         BondTreasuryPaymaster(
             admin,
+            whitelistAdmin,
             withdrawer,
             initialWhitelistedContracts,
             initialWhitelistedUsers,
@@ -130,6 +132,7 @@ contract BondTreasuryPaymasterTest is Test {
         initialUsers[1] = admin;
 
         paymaster = new MockBondTreasuryPaymaster(
+            admin,
             admin,
             withdrawer,
             _initialContractWhitelist(address(fleet)),
@@ -154,6 +157,25 @@ contract BondTreasuryPaymasterTest is Test {
         assertTrue(paymaster.hasRole(paymaster.WITHDRAWER_ROLE(), withdrawer));
     }
 
+    function test_separateWhitelistAdmin() public {
+        address whitelistAdmin = address(0x3333);
+        MockBondTreasuryPaymaster pm = new MockBondTreasuryPaymaster(
+            admin,
+            whitelistAdmin,
+            withdrawer,
+            _initialContractWhitelist(address(fleet)),
+            _emptyAddresses(),
+            address(bondToken),
+            QUOTA,
+            PERIOD
+        );
+        assertTrue(pm.hasRole(pm.DEFAULT_ADMIN_ROLE(), admin));
+        assertTrue(pm.hasRole(pm.WHITELIST_ADMIN_ROLE(), admin));
+        assertTrue(pm.hasRole(pm.WHITELIST_ADMIN_ROLE(), whitelistAdmin));
+        assertTrue(pm.hasRole(pm.WITHDRAWER_ROLE(), withdrawer));
+        assertFalse(pm.hasRole(pm.DEFAULT_ADMIN_ROLE(), whitelistAdmin));
+    }
+
     function test_immutables() public view {
         assertEq(address(paymaster.bondToken()), address(bondToken));
     }
@@ -173,6 +195,7 @@ contract BondTreasuryPaymasterTest is Test {
 
     function test_constructorWithEmptyWhitelistedUsers() public {
         MockBondTreasuryPaymaster pm = new MockBondTreasuryPaymaster(
+            admin,
             admin,
             withdrawer,
             _initialContractWhitelist(address(fleet)),
@@ -193,7 +216,7 @@ contract BondTreasuryPaymasterTest is Test {
         users[2] = charlie;
 
         MockBondTreasuryPaymaster pm = new MockBondTreasuryPaymaster(
-            admin, withdrawer, _initialContractWhitelist(address(fleet)), users, address(bondToken), QUOTA, PERIOD
+            admin, admin, withdrawer, _initialContractWhitelist(address(fleet)), users, address(bondToken), QUOTA, PERIOD
         );
         assertTrue(pm.isWhitelistedUser(alice));
         assertTrue(pm.isWhitelistedUser(bob));
@@ -208,13 +231,14 @@ contract BondTreasuryPaymasterTest is Test {
         vm.expectEmit();
         emit WhitelistPaymaster.WhitelistedUsersAdded(users);
         new MockBondTreasuryPaymaster(
-            admin, withdrawer, _initialContractWhitelist(address(fleet)), users, address(bondToken), QUOTA, PERIOD
+            admin, admin, withdrawer, _initialContractWhitelist(address(fleet)), users, address(bondToken), QUOTA, PERIOD
         );
     }
 
     function test_constructorEmptyUsersDoesNotEmitEvent() public {
         vm.recordLogs();
         new MockBondTreasuryPaymaster(
+            admin,
             admin,
             withdrawer,
             _initialContractWhitelist(address(fleet)),
@@ -501,6 +525,7 @@ contract BondTreasuryPaymasterTest is Test {
 
     function test_quotaTracksBaseBondNotClaimCount() public {
         MockBondTreasuryPaymaster tightPaymaster = new MockBondTreasuryPaymaster(
+            admin,
             admin,
             withdrawer,
             _initialContractWhitelist(address(fleet)),
@@ -581,6 +606,7 @@ contract BondTreasuryPaymasterTest is Test {
     function test_RevertIf_constructorZeroPeriod() public {
         vm.expectRevert(QuotaControl.ZeroPeriod.selector);
         new MockBondTreasuryPaymaster(
+            admin,
             admin,
             withdrawer,
             _initialContractWhitelist(address(fleet)),
@@ -594,6 +620,7 @@ contract BondTreasuryPaymasterTest is Test {
     function test_RevertIf_constructorTooLongPeriod() public {
         vm.expectRevert(QuotaControl.TooLongPeriod.selector);
         new MockBondTreasuryPaymaster(
+            admin,
             admin,
             withdrawer,
             _initialContractWhitelist(address(fleet)),