Rate limiting for eBTC cross chain

audit/nethermind-lz-cross-chain-rate-limiting.md

@@ -0,0 +1,15 @@
+# [NM-0217] Veda vault’s cross-chain x PairwiseRateLimiter
+
+**File(s)**: [LayerZeroTeller.sol](https://github.com/Se7en-Seas/boring-vault/blob/730929c7410b75a40547a8cc71104b1748c7e578/src/base/Roles/CrossChain/Bridges/LayerZero/LayerZeroTeller.sol), [PairwiseRateLimiter.sol](https://github.com/Se7en-Seas/boring-vault/blob/730929c7410b75a40547a8cc71104b1748c7e578/src/base/Roles/CrossChain/PairwiseRateLimiter.sol)
+
+### Summary
+
+The purpose of this PR is to add the rate limiting feature to the Veda vault’s cross-chain bridge. It uses the `PairwiseRateLimiter` contract that was audited previously during the "OFT Security Upgrades" audit item.
+
+---
+
+### Findings
+
+After reviewing the updated code, we don't see any clear risk on the changes that were implemented. The code seems to work as expected.
+
+---

src/base/Roles/CrossChain/Bridges/LayerZero/LayerZeroTeller.sol

@@ -8,12 +8,15 @@ import {SafeTransferLib} from "@solmate/utils/SafeTransferLib.sol";
 import {OAppAuth, Origin, MessagingFee, MessagingReceipt} from "@oapp-auth/OAppAuth.sol";
 import {AddressToBytes32Lib} from "src/helper/AddressToBytes32Lib.sol";
 import {OptionsBuilder} from "@oapp-auth/OptionsBuilder.sol";
+import {PairwiseRateLimiter} from "src/base/Roles/CrossChain/PairwiseRateLimiter.sol";
+import {MessageLib} from "src/base/Roles/CrossChain/MessageLib.sol";
 
-contract LayerZeroTeller is CrossChainTellerWithGenericBridge, OAppAuth {
+contract LayerZeroTeller is CrossChainTellerWithGenericBridge, OAppAuth, PairwiseRateLimiter {
     using SafeTransferLib for ERC20;
     using AddressToBytes32Lib for address;
     using AddressToBytes32Lib for bytes32;
     using OptionsBuilder for bytes;
+    using MessageLib for uint256;
 
     // ========================================= STRUCTS =========================================
 
@@ -164,6 +167,22 @@ contract LayerZeroTeller is CrossChainTellerWithGenericBridge, OAppAuth {
         emit ChainStopMessagesTo(chainId);
     }
 
+    /**
+     * @notice Set outbound rate limit configurations.
+     * @dev Callable by MULTISIG_ROLE.
+     */
+    function setOutboundRateLimits(RateLimitConfig[] calldata _rateLimitConfigs) external requiresAuth {
+       _setOutboundRateLimits(_rateLimitConfigs);
+    }
+
+    /**
+     * @notice Set inbound rate limit configurations.
+     * @dev Callable by MULTISIG_ROLE.
+     */
+    function setInboundRateLimits(RateLimitConfig[] calldata _rateLimitConfigs) external requiresAuth {
+        _setInboundRateLimits(_rateLimitConfigs);
+    }
+
     /**
      * @notice Set the gas limit for messages to a chain.
      * @dev Callable by OWNER_ROLE.
@@ -195,6 +214,7 @@ contract LayerZeroTeller is CrossChainTellerWithGenericBridge, OAppAuth {
         Chain memory source = idToChains[_origin.srcEid];
         if (!source.allowMessagesFrom) revert LayerZeroTeller__MessagesNotAllowedFrom(_origin.srcEid);
         uint256 message = abi.decode(_message, (uint256));
+        _checkAndUpdateInboundRateLimit(_origin.srcEid, message.uint256ToMessage().shareAmount);
         _completeMessageReceive(_guid, message);
     }
 
@@ -217,6 +237,7 @@ contract LayerZeroTeller is CrossChainTellerWithGenericBridge, OAppAuth {
         returns (bytes32 messageId)
     {
         uint32 destinationId = abi.decode(bridgeWildCard, (uint32));
+        _checkAndUpdateOutboundRateLimit(destinationId, message.uint256ToMessage().shareAmount);
         Chain memory chain = idToChains[destinationId];
         if (!chain.allowMessagesTo) {
             revert LayerZeroTeller__MessagesNotAllowedTo(destinationId);

src/base/Roles/CrossChain/PairwiseRateLimiter.sol

@@ -0,0 +1,206 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.0;
+
+/**
+ * @title Rate Limiter
+ * @dev Extends LayerZero's evm-oapp v2 RateLimiter contract. The original contract only supports rate limiting for outbound messages.
+ * This contract adds support for rate limiting inbound messages.
+ */
+
+abstract contract PairwiseRateLimiter {
+    /**
+     * @notice Rate Limit struct.
+     * @param amountInFlight The amount in the current window.
+     * @param lastUpdated Timestamp representing the last time the rate limit was checked or updated.
+     * @param limit This represents the maximum allowed amount within a given window.
+     * @param window Defines the duration of the rate limiting window.
+     */
+    struct RateLimit {
+        uint256 amountInFlight;
+        uint256 lastUpdated;
+        uint256 limit;
+        uint256 window;
+    }
+
+    /**
+     * @notice Rate Limit Configuration struct.
+     * @param dstEid The peer endpoint id.
+     * @param limit This represents the maximum allowed amount within a given window.
+     * @param window Defines the duration of the rate limiting window.
+     */
+    struct RateLimitConfig {
+        uint32 peerEid;
+        uint256 limit;
+        uint256 window;
+    }
+
+    /**
+     * @dev Mapping from peer endpoint id to RateLimit Configurations.
+     */
+    mapping(uint32 dstEid => RateLimit limit) public outboundRateLimits;
+    mapping(uint32 srcEid => RateLimit limit) public inboundRateLimits;
+
+    /**
+     * @notice Emitted when _setRateLimits occurs.
+     * @param rateLimitConfigs An array of `RateLimitConfig` structs representing the rate limit configurations set.
+     * - `peerEid`: The peer endpoint id.
+     * - `limit`: This represents the maximum allowed amount within a given window.
+     * - `window`: Defines the duration of the rate limiting window.
+     */
+    event OutboundRateLimitsChanged(RateLimitConfig[] rateLimitConfigs);
+    event InboundRateLimitsChanged(RateLimitConfig[] rateLimitConfigs);
+
+    /**
+     * @notice Error that is thrown when an amount exceeds the rate_limit.
+     */
+    error OutboundRateLimitExceeded();
+    error InboundRateLimitExceeded() ;
+
+    /**
+     * @notice Get the current amount that can be sent to this peer endpoint id for the given rate limit window.
+     * @param _dstEid The destination endpoint id.
+     * @return outboundAmountInFlight The current amount that was sent.
+     * @return amountCanBeSent The amount that can be sent.
+     */
+    function getAmountCanBeSent(
+        uint32 _dstEid
+    ) external view virtual returns (uint256 outboundAmountInFlight, uint256 amountCanBeSent) {
+        RateLimit memory rl = outboundRateLimits[_dstEid];
+        return _amountCanBeSent(rl.amountInFlight, rl.lastUpdated, rl.limit, rl.window);
+    }
+
+    /**
+     * @notice Get the current amount that can received from this peer endpoint for the given rate limit window.
+     * @param _srcEid The source endpoint id.
+     * @return inboundAmountInFlight The current amount has been received.
+     * @return amountCanBeReceived The amount that be received.
+     */
+    function getAmountCanBeReceived(
+        uint32 _srcEid
+    ) external view virtual returns (uint256 inboundAmountInFlight, uint256 amountCanBeReceived) {
+        RateLimit memory rl = inboundRateLimits[_srcEid];
+        return _amountCanBeSent(rl.amountInFlight, rl.lastUpdated, rl.limit, rl.window);
+    }    
+
+    /**
+     * @notice Sets the Rate Limit.
+     * @param _rateLimitConfigs A `RateLimitConfig` struct representing the rate limit configuration.
+     * - `dstEid`: The destination endpoint id.
+     * - `limit`: This represents the maximum allowed amount within a given window.
+     * - `window`: Defines the duration of the rate limiting window.
+     */
+    function _setOutboundRateLimits(RateLimitConfig[] memory _rateLimitConfigs) internal virtual {
+        unchecked {
+            for (uint256 i = 0; i < _rateLimitConfigs.length; i++) {
+                RateLimit storage rl = outboundRateLimits[_rateLimitConfigs[i].peerEid];
+
+                // @dev Ensure we checkpoint the existing rate limit as to not retroactively apply the new decay rate.
+                _checkAndUpdateOutboundRateLimit(_rateLimitConfigs[i].peerEid, 0);
+
+                // @dev Does NOT reset the amountInFlight/lastUpdated of an existing rate limit.
+                rl.limit = _rateLimitConfigs[i].limit;
+                rl.window = _rateLimitConfigs[i].window;
+            }
+        }
+        emit OutboundRateLimitsChanged(_rateLimitConfigs);
+    }
+
+    /**
+     * @notice Sets the Rate Limit.
+     * @param _rateLimitConfigs A `RateLimitConfig` struct representing the rate limit configuration.
+     * - `srcEid`: The source endpoint id.
+     * - `limit`: This represents the maximum allowed amount within a given window.
+     * - `window`: Defines the duration of the rate limiting window.
+     */
+    function _setInboundRateLimits(RateLimitConfig[] memory _rateLimitConfigs) internal virtual {
+        unchecked {
+            for (uint256 i = 0; i < _rateLimitConfigs.length; i++) {
+                RateLimit storage rl = inboundRateLimits[_rateLimitConfigs[i].peerEid];
+
+                // @dev Ensure we checkpoint the existing rate limit as to not retroactively apply the new decay rate.
+                _checkAndUpdateInboundRateLimit(_rateLimitConfigs[i].peerEid, 0);
+
+                // @dev Does NOT reset the amountInFlight/lastUpdated of an existing rate limit.
+                rl.limit = _rateLimitConfigs[i].limit;
+                rl.window = _rateLimitConfigs[i].window;
+            }
+        }
+        emit InboundRateLimitsChanged(_rateLimitConfigs);
+    }
+
+    /**
+     * @notice Checks current amount in flight and amount that can be sent for a given rate limit window.
+     * @param _amountInFlight The amount in the current window.
+     * @param _lastUpdated Timestamp representing the last time the rate limit was checked or updated.
+     * @param _limit This represents the maximum allowed amount within a given window.
+     * @param _window Defines the duration of the rate limiting window.
+     * @return currentAmountInFlight The amount in the current window.
+     * @return amountCanBeSent The amount that can be sent.
+     */
+    function _amountCanBeSent(
+        uint256 _amountInFlight,
+        uint256 _lastUpdated,
+        uint256 _limit,
+        uint256 _window
+    ) internal view virtual returns (uint256 currentAmountInFlight, uint256 amountCanBeSent) {
+        uint256 timeSinceLastDeposit = block.timestamp - _lastUpdated;
+        if (timeSinceLastDeposit >= _window) {
+            currentAmountInFlight = 0;
+            amountCanBeSent = _limit;
+        } else {
+            // @dev Presumes linear decay.
+            uint256 decay = (_limit * timeSinceLastDeposit) / _window;
+            currentAmountInFlight = _amountInFlight <= decay ? 0 : _amountInFlight - decay;
+            // @dev In the event the _limit is lowered, and the 'in-flight' amount is higher than the _limit, set to 0.
+            amountCanBeSent = _limit <= currentAmountInFlight ? 0 : _limit - currentAmountInFlight;
+        }
+    }
+
+    /**
+     * @notice Verifies whether the specified amount falls within the rate limit constraints for the targeted
+     * endpoint ID. On successful verification, it updates amountInFlight and lastUpdated. If the amount exceeds
+     * the rate limit, the operation reverts.
+     * @param _dstEid The destination endpoint id.
+     * @param _amount The amount to check for rate limit constraints.
+     */
+    function _checkAndUpdateOutboundRateLimit(uint32 _dstEid, uint256 _amount) internal virtual {
+        // @dev By default dstEid that have not been explicitly set will return amountCanBeSent == 0.
+        RateLimit storage rl = outboundRateLimits[_dstEid];
+
+        (uint256 currentAmountInFlight, uint256 amountCanBeSent) = _amountCanBeSent(
+            rl.amountInFlight,
+            rl.lastUpdated,
+            rl.limit,
+            rl.window
+        );
+        if (_amount > amountCanBeSent) revert OutboundRateLimitExceeded();
+
+        // @dev Update the storage to contain the new amount and current timestamp.
+        rl.amountInFlight = currentAmountInFlight + _amount;
+        rl.lastUpdated = block.timestamp;
+    }
+
+    /**
+     * @notice Verifies whether the specified amount falls within the rate limit constraints for the targeted
+     * endpoint ID. On successful verification, it updates amountInFlight and lastUpdated. If the amount exceeds
+     * the rate limit, the operation reverts.
+     * @param _srcEid The source endpoint id.
+     * @param _amount The amount to check for rate limit constraints.
+     */
+    function _checkAndUpdateInboundRateLimit(uint32 _srcEid, uint256 _amount) internal virtual {
+        // @dev By default dstEid that have not been explicitly set will return amountCanBeSent == 0.
+        RateLimit storage rl = inboundRateLimits[_srcEid];
+
+        (uint256 currentAmountInFlight, uint256 amountCanBeSent) = _amountCanBeSent(
+            rl.amountInFlight,
+            rl.lastUpdated,
+            rl.limit,
+            rl.window
+        );
+        if (_amount > amountCanBeSent) revert InboundRateLimitExceeded();
+
+        // @dev Update the storage to contain the new amount and current timestamp.
+        rl.amountInFlight = currentAmountInFlight + _amount;
+        rl.lastUpdated = block.timestamp;
+    }
+}

test/LayerZeroTeller.t.sol

@@ -3,6 +3,7 @@ pragma solidity 0.8.21;
 
 import {BoringVault} from "src/base/BoringVault.sol";
 import {LayerZeroTeller} from "src/base/Roles/CrossChain/Bridges/LayerZero/LayerZeroTeller.sol";
+import {PairwiseRateLimiter} from "src/base/Roles/CrossChain/PairwiseRateLimiter.sol";
 import {AccountantWithRateProviders} from "src/base/Roles/AccountantWithRateProviders.sol";
 import {SafeTransferLib} from "@solmate/utils/SafeTransferLib.sol";
 import {FixedPointMathLib} from "@solmate/utils/FixedPointMathLib.sol";
@@ -133,6 +134,10 @@ contract LayerZeroTellerTest is Test, MerkleTreeHelper {
         // Setup chains on bridge.
         sourceTeller.addChain(DESTINATION_ID, true, true, address(destinationTeller), 1_000_000);
         destinationTeller.addChain(SOURCE_ID, true, true, address(sourceTeller), 1_000_000);
+
+        // Setup rate limiting.
+        sourceTeller.setOutboundRateLimits(createRateLimitConfig(DESTINATION_ID, 2000 ether, 4 hours));
+        destinationTeller.setInboundRateLimits(createRateLimitConfig(SOURCE_ID, 2000 ether, 4 hours));
     }
 
     function testBridgingShares(uint96 sharesToBridge) external {
@@ -216,6 +221,35 @@ contract LayerZeroTellerTest is Test, MerkleTreeHelper {
     }
 
     function testReverts() external {
+        uint96 sharesToBridge = uint96(bound(uint96(101), 1, 1_000e18));
+        bytes memory bridgeData = abi.encode(DESTINATION_ID);
+        uint256 bridgeValue = 0.001e18;
+        address to = vm.addr(1);
+
+        // Test outbound rate limit.
+        sourceTeller.setOutboundRateLimits(createRateLimitConfig(DESTINATION_ID, 100, 4 hours));
+        // Expect failure by exceeding limit.
+        vm.expectRevert(PairwiseRateLimiter.OutboundRateLimitExceeded.selector);
+        sourceTeller.bridge{value: bridgeValue}( sharesToBridge, to, bridgeData, NATIVE_ERC20, 1e18);
+
+        // Increase limit and retry
+        sourceTeller.setOutboundRateLimits(createRateLimitConfig(DESTINATION_ID, 2000 ether, 4 hours));
+        sourceTeller.bridge{value: bridgeValue}( sharesToBridge, to, bridgeData, NATIVE_ERC20, 1e18);
+
+        // Test inbound rate limit.
+        destinationTeller.setInboundRateLimits(createRateLimitConfig(SOURCE_ID, 100, 4 hours));
+        MockLayerZeroEndPoint.Packet memory m = endPoint.getLastMessage();
+        // Expect failure by exceeding limit.
+        vm.prank(address(endPoint));
+        vm.expectRevert(PairwiseRateLimiter.InboundRateLimitExceeded.selector);
+        LayerZeroTeller(m.to).lzReceive(m._origin, m._guid, m._message, m._executor, m._extraData);
+
+        // Reset limit and retry.
+        destinationTeller.setInboundRateLimits(createRateLimitConfig(SOURCE_ID, 2000 ether, 4 hours));
+        vm.prank(address(endPoint));
+        LayerZeroTeller(m.to).lzReceive(m._origin, m._guid, m._message, m._executor, m._extraData);
+        assertEq(boringVault.balanceOf(to), sharesToBridge, "To address should have received shares.");
+
         // Adding a chain with a zero message gas limit should revert.
         vm.expectRevert(bytes(abi.encodeWithSelector(LayerZeroTeller.LayerZeroTeller__ZeroMessageGasLimit.selector)));
         sourceTeller.addChain(DESTINATION_ID, true, true, address(destinationTeller), 0);
@@ -268,7 +302,7 @@ contract LayerZeroTellerTest is Test, MerkleTreeHelper {
 
         sourceTeller.bridge{value: 0.001e18}(1e18, address(this), abi.encode(DESTINATION_ID), NATIVE_ERC20, 1e18);
 
-        MockLayerZeroEndPoint.Packet memory m = endPoint.getLastMessage();
+        m = endPoint.getLastMessage();
 
         // Send message to destination.
         vm.startPrank(address(endPoint));
@@ -302,4 +336,18 @@ contract LayerZeroTellerTest is Test, MerkleTreeHelper {
         forkId = vm.createFork(vm.envString(rpcKey), blockNumber);
         vm.selectFork(forkId);
     }
+
+    function createRateLimitConfig(
+        uint32 peerId,
+        uint256 limit,
+        uint256 window
+    ) internal pure returns (PairwiseRateLimiter.RateLimitConfig[] memory) {
+        PairwiseRateLimiter.RateLimitConfig[] memory configs = new PairwiseRateLimiter.RateLimitConfig[](1);
+        configs[0] = PairwiseRateLimiter.RateLimitConfig({
+            peerEid: peerId,
+            limit: limit,
+            window: window
+        });
+        return configs;
+    }
 }