feat: Add rate limit admin to remote UpgradeableBurnMintTokenPool (#14)

* feat: enable remote rateLimitAdmin

* fix: remove unnecessary modifer

* chore: reorder state variables

* fix: add and correct comments

---------

Co-authored-by: miguelmtzinf <miguelmtz.mail@gmail.com>

.github/actions/setup-nodejs/action.yaml

@@ -7,13 +7,13 @@ description: Setup pnpm for contracts
 runs:
   using: composite
   steps:
-    - uses: pnpm/action-setup@c3b53f6a16e57305370b4ae5a540c2077a1d50dd # v2.2.4
+    - uses: pnpm/action-setup@a3252b78c470c02df07e9d59298aecedc3ccdd6d # v3.0.0
       with:
-        version: ^7.0.0
+        version: ^9.0.0
 
-    - uses: actions/setup-node@8f152de45cc393bb48ce5d89d36b731f54556e65 # v4.0.0
+    - uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2
       with:
-        node-version: "16"
+        node-version: "20"
         cache: "pnpm"
         cache-dependency-path: "contracts/pnpm-lock.yaml"
 

contracts/src/v0.8/ccip/pools/GHO/UpgradeableBurnMintTokenPool.sol

@@ -8,6 +8,7 @@ import {IBurnMintERC20} from "../../../shared/token/ERC20/IBurnMintERC20.sol";
 
 import {UpgradeableTokenPool} from "./UpgradeableTokenPool.sol";
 import {UpgradeableBurnMintTokenPoolAbstract} from "./UpgradeableBurnMintTokenPoolAbstract.sol";
+import {RateLimiter} from "../../libraries/RateLimiter.sol";
 
 import {IRouter} from "../../interfaces/IRouter.sol";
 
@@ -17,9 +18,16 @@ import {IRouter} from "../../interfaces/IRouter.sol";
 /// @dev Contract adaptations:
 /// - Implementation of Initializable to allow upgrades
 /// - Move of allowlist and router definition to initialization stage
+/// - Inclusion of rate limit admin who may configure rate limits in addition to owner
 contract UpgradeableBurnMintTokenPool is Initializable, UpgradeableBurnMintTokenPoolAbstract, ITypeAndVersion {
+  error Unauthorized(address caller);
+
   string public constant override typeAndVersion = "BurnMintTokenPool 1.4.0";
 
+  /// @notice The address of the rate limiter admin.
+  /// @dev Can be address(0) if none is configured.
+  address internal s_rateLimitAdmin;
+
   /// @dev Constructor
   /// @param token The bridgeable token that is managed by this pool.
   /// @param armProxy The address of the arm proxy
@@ -49,6 +57,34 @@ contract UpgradeableBurnMintTokenPool is Initializable, UpgradeableBurnMintToken
     }
   }
 
+  /// @notice Sets the rate limiter admin address.
+  /// @dev Only callable by the owner.
+  /// @param rateLimitAdmin The new rate limiter admin address.
+  function setRateLimitAdmin(address rateLimitAdmin) external onlyOwner {
+    s_rateLimitAdmin = rateLimitAdmin;
+  }
+
+  /// @notice Gets the rate limiter admin address.
+  function getRateLimitAdmin() external view returns (address) {
+    return s_rateLimitAdmin;
+  }
+
+  /// @notice Sets the chain rate limiter config.
+  /// @dev Only callable by the owner or the rate limiter admin. NOTE: overwrites the normal
+  /// onlyAdmin check in the base implementation to also allow the rate limiter admin.
+  /// @param remoteChainSelector The remote chain selector for which the rate limits apply.
+  /// @param outboundConfig The new outbound rate limiter config.
+  /// @param inboundConfig The new inbound rate limiter config.
+  function setChainRateLimiterConfig(
+    uint64 remoteChainSelector,
+    RateLimiter.Config memory outboundConfig,
+    RateLimiter.Config memory inboundConfig
+  ) external override {
+    if (msg.sender != s_rateLimitAdmin && msg.sender != owner()) revert Unauthorized(msg.sender);
+
+    _setRateLimitConfig(remoteChainSelector, outboundConfig, inboundConfig);
+  }
+
   /// @inheritdoc UpgradeableBurnMintTokenPoolAbstract
   function _burn(uint256 amount) internal virtual override {
     IBurnMintERC20(address(i_token)).burn(amount);

contracts/src/v0.8/ccip/pools/GHO/UpgradeableLockReleaseTokenPool.sol

@@ -248,7 +248,7 @@ contract UpgradeableLockReleaseTokenPool is Initializable, UpgradeableTokenPool,
     emit LiquidityRemoved(msg.sender, amount);
   }
 
-  /// @notice Sets the rate limiter admin address.
+  /// @notice Sets the chain rate limiter config.
   /// @dev Only callable by the owner or the rate limiter admin. NOTE: overwrites the normal
   /// onlyAdmin check in the base implementation to also allow the rate limiter admin.
   /// @param remoteChainSelector The remote chain selector for which the rate limits apply.

contracts/src/v0.8/ccip/pools/GHO/diffs/UpgradeableBurnMintTokenPool_diff.md

@@ -1,9 +1,9 @@
 ```diff
 diff --git a/src/v0.8/ccip/pools/BurnMintTokenPool.sol b/src/v0.8/ccip/pools/GHO/UpgradeableBurnMintTokenPool.sol
-index 9af0f22f4c..58be87812f 100644
+index 9af0f22f4c..a46ff915e5 100644
 --- a/src/v0.8/ccip/pools/BurnMintTokenPool.sol
 +++ b/src/v0.8/ccip/pools/GHO/UpgradeableBurnMintTokenPool.sol
-@@ -1,28 +1,55 @@
+@@ -1,28 +1,90 @@
  // SPDX-License-Identifier: BUSL-1.1
 -pragma solidity 0.8.19;
 +pragma solidity ^0.8.0;
@@ -16,15 +16,10 @@ index 9af0f22f4c..58be87812f 100644
 -import {BurnMintTokenPoolAbstract} from "./BurnMintTokenPoolAbstract.sol";
 +import {ITypeAndVersion} from "../../../shared/interfaces/ITypeAndVersion.sol";
 +import {IBurnMintERC20} from "../../../shared/token/ERC20/IBurnMintERC20.sol";
-
--/// @notice This pool mints and burns a 3rd-party token.
--/// @dev Pool whitelisting mode is set in the constructor and cannot be modified later.
--/// It either accepts any address as originalSender, or only accepts whitelisted originalSender.
--/// The only way to change whitelisting mode is to deploy a new pool.
--/// If that is expected, please make sure the token's burner/minter roles are adjustable.
--contract BurnMintTokenPool is BurnMintTokenPoolAbstract, ITypeAndVersion {
++
 +import {UpgradeableTokenPool} from "./UpgradeableTokenPool.sol";
 +import {UpgradeableBurnMintTokenPoolAbstract} from "./UpgradeableBurnMintTokenPoolAbstract.sol";
++import {RateLimiter} from "../../libraries/RateLimiter.sol";
 +
 +import {IRouter} from "../../interfaces/IRouter.sol";
 +
@@ -35,8 +30,20 @@ index 9af0f22f4c..58be87812f 100644
 +/// - Implementation of Initializable to allow upgrades
 +/// - Move of allowlist and router definition to initialization stage
 +contract UpgradeableBurnMintTokenPool is Initializable, UpgradeableBurnMintTokenPoolAbstract, ITypeAndVersion {
++  error Unauthorized(address caller);
+
+-/// @notice This pool mints and burns a 3rd-party token.
+-/// @dev Pool whitelisting mode is set in the constructor and cannot be modified later.
+-/// It either accepts any address as originalSender, or only accepts whitelisted originalSender.
+-/// The only way to change whitelisting mode is to deploy a new pool.
+-/// If that is expected, please make sure the token's burner/minter roles are adjustable.
+-contract BurnMintTokenPool is BurnMintTokenPoolAbstract, ITypeAndVersion {
    string public constant override typeAndVersion = "BurnMintTokenPool 1.4.0";
 
++  /// @notice The address of the rate limiter admin.
++  /// @dev Can be address(0) if none is configured.
++  address internal s_rateLimitAdmin;
++
 +  /// @dev Constructor
 +  /// @param token The bridgeable token that is managed by this pool.
 +  /// @param armProxy The address of the arm proxy
@@ -71,6 +78,34 @@ index 9af0f22f4c..58be87812f 100644
 +    }
 +  }
 +
++  /// @notice Sets the rate limiter admin address.
++  /// @dev Only callable by the owner.
++  /// @param rateLimitAdmin The new rate limiter admin address.
++  function setRateLimitAdmin(address rateLimitAdmin) external onlyOwner {
++    s_rateLimitAdmin = rateLimitAdmin;
++  }
++
++  /// @notice Gets the rate limiter admin address.
++  function getRateLimitAdmin() external view returns (address) {
++    return s_rateLimitAdmin;
++  }
++
++  /// @notice Sets the rate limiter admin address.
++  /// @dev Only callable by the owner or the rate limiter admin. NOTE: overwrites the normal
++  /// onlyAdmin check in the base implementation to also allow the rate limiter admin.
++  /// @param remoteChainSelector The remote chain selector for which the rate limits apply.
++  /// @param outboundConfig The new outbound rate limiter config.
++  /// @param inboundConfig The new inbound rate limiter config.
++  function setChainRateLimiterConfig(
++    uint64 remoteChainSelector,
++    RateLimiter.Config memory outboundConfig,
++    RateLimiter.Config memory inboundConfig
++  ) external override {
++    if (msg.sender != s_rateLimitAdmin && msg.sender != owner()) revert Unauthorized(msg.sender);
++
++    _setRateLimitConfig(remoteChainSelector, outboundConfig, inboundConfig);
++  }
++
 +  /// @inheritdoc UpgradeableBurnMintTokenPoolAbstract
    function _burn(uint256 amount) internal virtual override {
      IBurnMintERC20(address(i_token)).burn(amount);

contracts/src/v0.8/ccip/test/BaseTest.t.sol

@@ -8,7 +8,6 @@ import {StdUtils} from "forge-std/StdUtils.sol";
 import {MockARM} from "./mocks/MockARM.sol";
 import {StructFactory} from "./StructFactory.sol";
 
-
 contract BaseTest is Test, StructFactory {
   bool private s_baseTestInitialized;
 
@@ -30,6 +29,4 @@ contract BaseTest is Test, StructFactory {
 
     s_mockARM = new MockARM();
   }
-
-
 }

contracts/src/v0.8/ccip/test/pools/GHO/GhoTokenPoolRemote.t.sol

@@ -242,3 +242,137 @@ contract GhoTokenPoolEthereum_upgradeability is GhoTokenPoolRemoteSetup {
     assertEq(_getProxyAdminAddress(address(s_pool)), PROXY_ADMIN, "Unauthorized admin change");
   }
 }
+
+contract GhoTokenPoolRemote_setChainRateLimiterConfig is GhoTokenPoolRemoteSetup {
+  event ConfigChanged(RateLimiter.Config);
+  event ChainConfigured(
+    uint64 chainSelector,
+    RateLimiter.Config outboundRateLimiterConfig,
+    RateLimiter.Config inboundRateLimiterConfig
+  );
+
+  uint64 internal s_remoteChainSelector;
+
+  function setUp() public virtual override {
+    GhoTokenPoolRemoteSetup.setUp();
+    UpgradeableTokenPool.ChainUpdate[] memory chainUpdates = new UpgradeableTokenPool.ChainUpdate[](1);
+    s_remoteChainSelector = 123124;
+    chainUpdates[0] = UpgradeableTokenPool.ChainUpdate({
+      remoteChainSelector: s_remoteChainSelector,
+      allowed: true,
+      outboundRateLimiterConfig: getOutboundRateLimiterConfig(),
+      inboundRateLimiterConfig: getInboundRateLimiterConfig()
+    });
+    changePrank(AAVE_DAO);
+    s_pool.applyChainUpdates(chainUpdates);
+    changePrank(OWNER);
+  }
+
+  function testFuzz_SetChainRateLimiterConfigSuccess(uint128 capacity, uint128 rate, uint32 newTime) public {
+    // Cap the lower bound to 4 so 4/2 is still >= 2
+    vm.assume(capacity >= 4);
+    // Cap the lower bound to 2 so 2/2 is still >= 1
+    rate = uint128(bound(rate, 2, capacity - 2));
+    // Bucket updates only work on increasing time
+    newTime = uint32(bound(newTime, block.timestamp + 1, type(uint32).max));
+    vm.warp(newTime);
+
+    uint256 oldOutboundTokens = s_pool.getCurrentOutboundRateLimiterState(s_remoteChainSelector).tokens;
+    uint256 oldInboundTokens = s_pool.getCurrentInboundRateLimiterState(s_remoteChainSelector).tokens;
+
+    RateLimiter.Config memory newOutboundConfig = RateLimiter.Config({isEnabled: true, capacity: capacity, rate: rate});
+    RateLimiter.Config memory newInboundConfig = RateLimiter.Config({
+      isEnabled: true,
+      capacity: capacity / 2,
+      rate: rate / 2
+    });
+
+    vm.expectEmit();
+    emit ConfigChanged(newOutboundConfig);
+    vm.expectEmit();
+    emit ConfigChanged(newInboundConfig);
+    vm.expectEmit();
+    emit ChainConfigured(s_remoteChainSelector, newOutboundConfig, newInboundConfig);
+
+    changePrank(AAVE_DAO);
+    s_pool.setChainRateLimiterConfig(s_remoteChainSelector, newOutboundConfig, newInboundConfig);
+
+    uint256 expectedTokens = RateLimiter._min(newOutboundConfig.capacity, oldOutboundTokens);
+
+    RateLimiter.TokenBucket memory bucket = s_pool.getCurrentOutboundRateLimiterState(s_remoteChainSelector);
+    assertEq(bucket.capacity, newOutboundConfig.capacity);
+    assertEq(bucket.rate, newOutboundConfig.rate);
+    assertEq(bucket.tokens, expectedTokens);
+    assertEq(bucket.lastUpdated, newTime);
+
+    expectedTokens = RateLimiter._min(newInboundConfig.capacity, oldInboundTokens);
+
+    bucket = s_pool.getCurrentInboundRateLimiterState(s_remoteChainSelector);
+    assertEq(bucket.capacity, newInboundConfig.capacity);
+    assertEq(bucket.rate, newInboundConfig.rate);
+    assertEq(bucket.tokens, expectedTokens);
+    assertEq(bucket.lastUpdated, newTime);
+  }
+
+  function testOnlyOwnerOrRateLimitAdminSuccess() public {
+    address rateLimiterAdmin = address(28973509103597907);
+
+    changePrank(AAVE_DAO);
+    s_pool.setRateLimitAdmin(rateLimiterAdmin);
+
+    changePrank(rateLimiterAdmin);
+
+    s_pool.setChainRateLimiterConfig(
+      s_remoteChainSelector,
+      getOutboundRateLimiterConfig(),
+      getInboundRateLimiterConfig()
+    );
+
+    changePrank(AAVE_DAO);
+
+    s_pool.setChainRateLimiterConfig(
+      s_remoteChainSelector,
+      getOutboundRateLimiterConfig(),
+      getInboundRateLimiterConfig()
+    );
+  }
+
+  // Reverts
+
+  function testOnlyOwnerReverts() public {
+    changePrank(STRANGER);
+
+    vm.expectRevert(abi.encodeWithSelector(UpgradeableBurnMintTokenPool.Unauthorized.selector, STRANGER));
+    s_pool.setChainRateLimiterConfig(
+      s_remoteChainSelector,
+      getOutboundRateLimiterConfig(),
+      getInboundRateLimiterConfig()
+    );
+  }
+
+  function testNonExistentChainReverts() public {
+    uint64 wrongChainSelector = 9084102894;
+
+    vm.expectRevert(abi.encodeWithSelector(UpgradeableTokenPool.NonExistentChain.selector, wrongChainSelector));
+    changePrank(AAVE_DAO);
+    s_pool.setChainRateLimiterConfig(wrongChainSelector, getOutboundRateLimiterConfig(), getInboundRateLimiterConfig());
+  }
+}
+
+contract GhoTokenPoolRemote_setRateLimitAdmin is GhoTokenPoolRemoteSetup {
+  function testSetRateLimitAdminSuccess() public {
+    assertEq(address(0), s_pool.getRateLimitAdmin());
+    changePrank(AAVE_DAO);
+    s_pool.setRateLimitAdmin(OWNER);
+    assertEq(OWNER, s_pool.getRateLimitAdmin());
+  }
+
+  // Reverts
+
+  function testSetRateLimitAdminReverts() public {
+    vm.startPrank(STRANGER);
+
+    vm.expectRevert("Only callable by owner");
+    s_pool.setRateLimitAdmin(STRANGER);
+  }
+}