-
Notifications
You must be signed in to change notification settings - Fork 110
Implement Rate Limiter Contract #851
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0711201
c0e82aa
f6d4ba2
aeb3cf1
4c4c0a8
bbdcf8f
ffac482
297d971
2e329cd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ pragma solidity ^0.8.22; | |
| import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; | ||
| import {IAtomicBridgeCounterpartyMOVE} from "./IAtomicBridgeCounterpartyMOVE.sol"; | ||
| import {AtomicBridgeInitiatorMOVE} from "./AtomicBridgeInitiatorMOVE.sol"; | ||
| import {RateLimiter} from "./RateLimiter.sol"; | ||
|
|
||
| contract AtomicBridgeCounterpartyMOVE is IAtomicBridgeCounterpartyMOVE, OwnableUpgradeable { | ||
| enum MessageState { | ||
|
|
@@ -22,17 +23,26 @@ contract AtomicBridgeCounterpartyMOVE is IAtomicBridgeCounterpartyMOVE, OwnableU | |
| } | ||
|
|
||
| AtomicBridgeInitiatorMOVE public atomicBridgeInitiatorMOVE; | ||
| RateLimiter public rateLimiter; | ||
| mapping(bytes32 => BridgeTransferDetails) public bridgeTransfers; | ||
|
|
||
| // Configurable time lock duration | ||
| uint256 public counterpartyTimeLockDuration; | ||
|
|
||
| function initialize(address _atomicBridgeInitiator, address owner, uint256 _timeLockDuration) public initializer { | ||
| // Initialize with initiator, RateLimiter, owner, and time lock duration | ||
| function initialize( | ||
| address _atomicBridgeInitiator, | ||
| address _rateLimiter, | ||
| address owner, | ||
| uint256 _timeLockDuration | ||
| ) public initializer { | ||
| if (_atomicBridgeInitiator == address(0)) revert ZeroAddress(); | ||
| if (_rateLimiter == address(0)) revert ZeroAddress(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Input validation: require (_rateLimiter != address(0), ZeroAddress()); |
||
|
|
||
| atomicBridgeInitiatorMOVE = AtomicBridgeInitiatorMOVE(_atomicBridgeInitiator); | ||
| rateLimiter = RateLimiter(_rateLimiter); | ||
| __Ownable_init(owner); | ||
|
|
||
| // Set the configurable time lock duration | ||
| counterpartyTimeLockDuration = _timeLockDuration; | ||
| } | ||
|
|
||
|
|
@@ -41,6 +51,11 @@ contract AtomicBridgeCounterpartyMOVE is IAtomicBridgeCounterpartyMOVE, OwnableU | |
| atomicBridgeInitiatorMOVE = AtomicBridgeInitiatorMOVE(_atomicBridgeInitiator); | ||
| } | ||
|
|
||
| function setRateLimiter(address _rateLimiter) external onlyOwner { | ||
| if (_rateLimiter == address(0)) revert ZeroAddress(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Change to: require (_rateLimiter != address(0), ZeroAddress()); |
||
| rateLimiter = RateLimiter(_rateLimiter); | ||
| } | ||
|
|
||
| function setTimeLockDuration(uint256 _timeLockDuration) external onlyOwner { | ||
| counterpartyTimeLockDuration = _timeLockDuration; | ||
| } | ||
|
|
@@ -55,7 +70,12 @@ contract AtomicBridgeCounterpartyMOVE is IAtomicBridgeCounterpartyMOVE, OwnableU | |
| if (amount == 0) revert ZeroAmount(); | ||
| if (atomicBridgeInitiatorMOVE.poolBalance() < amount) revert InsufficientMOVEBalance(); | ||
|
|
||
| // The time lock is now based on the configurable duration | ||
| bool isWithinRateLimit = rateLimiter.initiateTransfer(amount, RateLimiter.TransferDirection.L2_TO_L1); | ||
| if (!isWithinRateLimit) { | ||
| revert("RATE_LIMIT_EXCEEDED"); | ||
| } | ||
|
|
||
| // The time lock is based on the configurable duration | ||
| uint256 timeLock = block.timestamp + counterpartyTimeLockDuration; | ||
|
|
||
| bridgeTransfers[bridgeTransferId] = BridgeTransferDetails({ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import {IAtomicBridgeInitiatorMOVE} from "./IAtomicBridgeInitiatorMOVE.sol"; | |
| import {MockMOVEToken} from "./MockMOVEToken.sol"; | ||
| import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; | ||
| import {ERC20Upgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol"; | ||
| import {RateLimiter} from "./RateLimiter.sol"; | ||
|
|
||
| contract AtomicBridgeInitiatorMOVE is IAtomicBridgeInitiatorMOVE, OwnableUpgradeable { | ||
| enum MessageState { | ||
|
|
@@ -35,23 +36,25 @@ contract AtomicBridgeInitiatorMOVE is IAtomicBridgeInitiatorMOVE, OwnableUpgrade | |
| // Configurable time lock duration | ||
| uint256 public initiatorTimeLockDuration; | ||
|
|
||
| // Initialize the contract with MOVE token address, owner, custom time lock duration, and initial pool balance | ||
| // RateLimiter contract instance | ||
| RateLimiter public rateLimiter; | ||
|
|
||
| // Initialize the contract with MOVE token address, owner, custom time lock duration, initial pool balance, and RateLimiter contract address | ||
| function initialize( | ||
| address _moveToken, | ||
| address owner, | ||
| uint256 _timeLockDuration, | ||
| uint256 _initialPoolBalance | ||
| uint256 _initialPoolBalance, | ||
| address _rateLimiter | ||
| ) public initializer { | ||
| if (_moveToken == address(0)) { | ||
| revert ZeroAddress(); | ||
| } | ||
| if (_moveToken == address(0)) revert ZeroAddress(); | ||
| if (_rateLimiter == address(0)) revert ZeroAddress(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Input validation: require (_rateLimiter != address(0), ZeroAddress()); |
||
|
|
||
| moveToken = ERC20Upgradeable(_moveToken); | ||
| rateLimiter = RateLimiter(_rateLimiter); | ||
| __Ownable_init(owner); | ||
|
|
||
| // Set the custom time lock duration | ||
| initiatorTimeLockDuration = _timeLockDuration; | ||
|
|
||
| // Set the initial pool balance | ||
| poolBalance = _initialPoolBalance; | ||
| } | ||
|
|
||
|
|
@@ -67,8 +70,11 @@ contract AtomicBridgeInitiatorMOVE is IAtomicBridgeInitiatorMOVE, OwnableUpgrade | |
| address originator = msg.sender; | ||
|
|
||
| // Ensure there is a valid amount | ||
| if (moveAmount == 0) { | ||
| revert ZeroAmount(); | ||
| if (moveAmount == 0) revert ZeroAmount(); | ||
|
|
||
| // Check the rate limit before proceeding with the transfer | ||
| if (!rateLimiter.initiateTransfer(moveAmount, RateLimiter.TransferDirection.L1_TO_L2)) { | ||
| revert("RATE_LIMIT_EXCEEDED"); | ||
| } | ||
|
|
||
| // Transfer the MOVE tokens from the user to the contract | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| // SPDX-License-Identifier: MIT | ||
| pragma solidity ^0.8.22; | ||
|
|
||
| import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; | ||
|
|
||
| contract RateLimiter is OwnableUpgradeable { | ||
| enum TransferDirection { | ||
| L1_TO_L2, | ||
| L2_TO_L1 | ||
| } | ||
|
|
||
| // Maximum amount that can be transferred in each direction within the risk period | ||
| uint256 public rateLimitL1L2; | ||
| uint256 public rateLimitL2L1; | ||
|
|
||
| // Track the accumulated budget per transfer direction | ||
| uint256 public budgetL1L2; | ||
| uint256 public budgetL2L1; | ||
|
|
||
| // Risk period for rate limiting (in seconds) | ||
| uint256 public riskPeriod; | ||
|
|
||
| // Security fund balance | ||
| uint256 public securityFund; | ||
|
|
||
| event RateLimitExceeded(TransferDirection direction); | ||
| event RateLimitUpdated(uint256 newRateLimitL1L2, uint256 newRateLimitL2L1); | ||
| event SecurityFundUpdated(uint256 newSecurityFund); | ||
|
|
||
| // Initialize the contract with initial rate limits and risk period | ||
| function initialize(address owner, uint256 _riskPeriod, uint256 _securityFund) public initializer { | ||
| riskPeriod = _riskPeriod; | ||
| securityFund = _securityFund; | ||
| __Ownable_init(owner); | ||
| _updateRateLimits(); | ||
| } | ||
|
|
||
| // Modifier to check if a transfer exceeds the rate limit | ||
| modifier withinRateLimit(uint256 amount, TransferDirection direction) { | ||
| uint256 currentBudget = (direction == TransferDirection.L1_TO_L2) ? budgetL1L2 : budgetL2L1; | ||
| uint256 rateLimit = (direction == TransferDirection.L1_TO_L2) ? rateLimitL1L2 : rateLimitL2L1; | ||
|
|
||
| require(currentBudget + amount <= rateLimit, "RATE_LIMIT_EXCEEDED"); | ||
| _; | ||
| } | ||
|
|
||
| function initiateTransfer(uint256 amount, TransferDirection direction) external returns (bool) { | ||
| uint256 currentBudget = (direction == TransferDirection.L1_TO_L2) ? budgetL1L2 : budgetL2L1; | ||
| uint256 rateLimit = (direction == TransferDirection.L1_TO_L2) ? rateLimitL1L2 : rateLimitL2L1; | ||
|
|
||
| if (currentBudget + amount > rateLimit) { | ||
| emit RateLimitExceeded(direction); | ||
| return false; | ||
| } | ||
|
|
||
| // Update the budget for the specified direction | ||
| if (direction == TransferDirection.L1_TO_L2) { | ||
| budgetL1L2 += amount; | ||
| } else { | ||
| budgetL2L1 += amount; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| // Update the security fund and recalculate rate limits | ||
| function updateSecurityFund(uint256 newSecurityFund) external onlyOwner { | ||
| securityFund = newSecurityFund; | ||
| _updateRateLimits(); | ||
| emit SecurityFundUpdated(newSecurityFund); | ||
| } | ||
|
|
||
| // Private function to update the rate limits based on the security fund and risk period | ||
| function _updateRateLimits() private { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks correct, but choice of representation here and in MIP is funny. I think the MIP should express this more clearly, i.e., use |
||
| rateLimitL1L2 = (securityFund * 5) / (riskPeriod * 10); // 0.5 * securityFund / riskPeriod | ||
| rateLimitL2L1 = (securityFund * 5) / (riskPeriod * 10); // Same calculation as for L1 to L2 | ||
|
|
||
| emit RateLimitUpdated(rateLimitL1L2, rateLimitL2L1); | ||
| } | ||
|
|
||
| // Reset the budget for each direction; this could be called periodically or by governance if all transfers are confirmed | ||
| function resetBudget() external onlyOwner { | ||
| budgetL1L2 = 0; | ||
| budgetL2L1 = 0; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is input validation and should be written as: