Add Access Control, Refunder and Rate Limiter

protocol-units/bridge/contracts/src/AtomicBridgeCounterpartyMOVE.sol

@@ -1,12 +1,12 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.22;
 
-import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
+import {AccessControlUpgradeable} from "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
 import {IAtomicBridgeCounterpartyMOVE} from "./IAtomicBridgeCounterpartyMOVE.sol";
 import {AtomicBridgeInitiatorMOVE} from "./AtomicBridgeInitiatorMOVE.sol";
 import {RateLimiter} from "./RateLimiter.sol";
 
-contract AtomicBridgeCounterpartyMOVE is IAtomicBridgeCounterpartyMOVE, OwnableUpgradeable {
+contract AtomicBridgeCounterpartyMOVE is IAtomicBridgeCounterpartyMOVE, AccessControlUpgradeable {
     enum MessageState {
         PENDING,
         COMPLETED,
@@ -29,25 +29,41 @@ contract AtomicBridgeCounterpartyMOVE is IAtomicBridgeCounterpartyMOVE, OwnableU
     // Configurable time lock duration
     uint256 public counterpartyTimeLockDuration;
 
-    function initialize(address _atomicBridgeInitiator, address owner, uint256 _timeLockDuration) public initializer {
+    bytes32 public constant RELAYER_ROLE = keccak256("RELAYER_ROLE");
+    bytes32 public constant REFUNDER_ROLE = keccak256("REFUNDER_ROLE");
+
+    // Prevents initialization of implementation contract exploits
+    constructor() {
+        _disableInitializers();
+    }
+
+    function initialize(
+        address _atomicBridgeInitiator,
+        address _owner,
+        address _relayer,
+        address _refunder,
+        uint256 _timeLockDuration
+    ) public initializer {
         if (_atomicBridgeInitiator == address(0)) revert ZeroAddress();
         atomicBridgeInitiatorMOVE = AtomicBridgeInitiatorMOVE(_atomicBridgeInitiator);
-        __Ownable_init(owner);
+        _grantRole(DEFAULT_ADMIN_ROLE, _owner);
+        _grantRole(RELAYER_ROLE, _relayer);
+        _grantRole(REFUNDER_ROLE, _refunder);
 
         // Set the configurable time lock duration
         counterpartyTimeLockDuration = _timeLockDuration;
     }
 
-    function setAtomicBridgeInitiator(address _atomicBridgeInitiator) external onlyOwner {
+    function setAtomicBridgeInitiator(address _atomicBridgeInitiator) external onlyRole(DEFAULT_ADMIN_ROLE) {
         if (_atomicBridgeInitiator == address(0)) revert ZeroAddress();
         atomicBridgeInitiatorMOVE = AtomicBridgeInitiatorMOVE(_atomicBridgeInitiator);
     }
 
-    function setTimeLockDuration(uint256 _timeLockDuration) external onlyOwner {
+    function setTimeLockDuration(uint256 _timeLockDuration) external onlyRole(DEFAULT_ADMIN_ROLE) {
         counterpartyTimeLockDuration = _timeLockDuration;
     }
 
-    function setRateLimiter(address _rateLimiter) external onlyOwner {
+    function setRateLimiter(address _rateLimiter) external onlyRole(DEFAULT_ADMIN_ROLE) {
         if (_rateLimiter == address(0)) revert ZeroAddress();
         rateLimiter = RateLimiter(_rateLimiter);
     }
@@ -58,9 +74,8 @@ contract AtomicBridgeCounterpartyMOVE is IAtomicBridgeCounterpartyMOVE, OwnableU
         bytes32 hashLock,
         address recipient,
         uint256 amount
-    ) external onlyOwner {
+    ) external onlyRole(RELAYER_ROLE) {
         if (amount == 0) revert ZeroAmount();
-
         // The time lock is now based on the configurable duration
         uint256 timeLock = block.timestamp + counterpartyTimeLockDuration;
 
@@ -88,7 +103,7 @@ contract AtomicBridgeCounterpartyMOVE is IAtomicBridgeCounterpartyMOVE, OwnableU
         emit BridgeTransferCompleted(bridgeTransferId, preImage);
     }
 
-    function abortBridgeTransfer(bytes32 bridgeTransferId) external onlyOwner {
+    function abortBridgeTransfer(bytes32 bridgeTransferId) external onlyRole(REFUNDER_ROLE) {
         BridgeTransferDetails storage details = bridgeTransfers[bridgeTransferId];
         if (details.state != MessageState.PENDING) revert BridgeTransferStateNotPending();
         if (block.timestamp <= details.timeLock) revert TimeLockNotExpired();

protocol-units/bridge/contracts/src/AtomicBridgeInitiatorMOVE.sol

@@ -2,12 +2,11 @@
 pragma solidity ^0.8.22;
 
 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 {AccessControlUpgradeable} from "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
+import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import {RateLimiter} from "./RateLimiter.sol";
 
-contract AtomicBridgeInitiatorMOVE is IAtomicBridgeInitiatorMOVE, OwnableUpgradeable {
+contract AtomicBridgeInitiatorMOVE is IAtomicBridgeInitiatorMOVE, AccessControlUpgradeable {
     enum MessageState {
         INITIALIZED,
         COMPLETED,
@@ -26,41 +25,49 @@ contract AtomicBridgeInitiatorMOVE is IAtomicBridgeInitiatorMOVE, OwnableUpgrade
     // Mapping of bridge transfer ids to BridgeTransfer structs
     mapping(bytes32 => BridgeTransfer) public bridgeTransfers;
 
-    // Total MOVE token pool balance
-    uint256 public poolBalance;
-
     address public counterpartyAddress;
     RateLimiter public rateLimiter;
-    ERC20Upgradeable public moveToken;
+    IERC20 public moveToken;
     uint256 private nonce;
 
     // Configurable time lock duration
     uint256 public initiatorTimeLockDuration;
 
-    // Initialize the contract with MOVE token address, owner, custom time lock duration, and initial pool balance
+    bytes32 public constant RELAYER_ROLE = keccak256("RELAYER_ROLE");
+    bytes32 public constant REFUNDER_ROLE = keccak256("REFUNDER_ROLE");
+
+    // Prevents initialization of implementation contract exploits
+    constructor() {
+        _disableInitializers();
+    }
+
+    // Initialize the contract with MOVE token address, owner, and custom time lock duration
     function initialize(
         address _moveToken,
-        address owner,
-        uint256 _timeLockDuration,
-        uint256 _initialPoolBalance
+        address _owner,
+        address _relayer,
+        address _refunder,
+        uint256 _timeLockDuration
     ) public initializer {
-        require(_moveToken != address(0) && owner != address(0), "ZeroAddress");
-        moveToken = ERC20Upgradeable(_moveToken);
-        __Ownable_init(owner);
+        if (_moveToken == address(0) && _owner == address(0) && _relayer == address(0) && _refunder == address(0)) {
+            revert ZeroAddress();
+        }
+        require(_timeLockDuration > 0, ZeroAmount());
+        moveToken = IERC20(_moveToken);
+        _grantRole(DEFAULT_ADMIN_ROLE, _owner);
+        _grantRole(RELAYER_ROLE, _relayer);
+        _grantRole(REFUNDER_ROLE, _refunder);
 
         // Set the custom time lock duration
         initiatorTimeLockDuration = _timeLockDuration;
-
-        // Set the initial pool balance
-        poolBalance = _initialPoolBalance;
     }
 
-    function setCounterpartyAddress(address _counterpartyAddress) external onlyOwner {
+    function setCounterpartyAddress(address _counterpartyAddress) external onlyRole(DEFAULT_ADMIN_ROLE) {
         require(_counterpartyAddress != address(0), "ZeroAddress");
         counterpartyAddress = _counterpartyAddress;
     }
 
-    function setRateLimiter(address _rateLimiter) external onlyOwner {
+    function setRateLimiter(address _rateLimiter) external onlyRole(DEFAULT_ADMIN_ROLE) {
         if (_rateLimiter == address(0)) revert ZeroAddress();
         rateLimiter = RateLimiter(_rateLimiter);
     }
@@ -72,7 +79,7 @@ contract AtomicBridgeInitiatorMOVE is IAtomicBridgeInitiatorMOVE, OwnableUpgrade
         rateLimiter.rateLimitOutbound(moveAmount);
         address originator = msg.sender;
 
-        require(moveAmount > 0, "ZeroAmount");
+        require(moveAmount > 0, ZeroAmount());
 
         // Ensure there is a valid amount
         if (moveAmount == 0) {
@@ -84,9 +91,6 @@ contract AtomicBridgeInitiatorMOVE is IAtomicBridgeInitiatorMOVE, OwnableUpgrade
             revert MOVETransferFailed();
         }
 
-        // Update the pool balance
-        poolBalance += moveAmount;
-
         // Generate a unique nonce to prevent replay attacks, and generate a transfer ID
         bridgeTransferId = keccak256(abi.encodePacked(originator, recipient, hashLock, initiatorTimeLockDuration, block.timestamp, nonce++));
 
@@ -103,7 +107,7 @@ contract AtomicBridgeInitiatorMOVE is IAtomicBridgeInitiatorMOVE, OwnableUpgrade
         return bridgeTransferId;
     }
 
-    function completeBridgeTransfer(bytes32 bridgeTransferId, bytes32 preImage) external onlyOwner {
+    function completeBridgeTransfer(bytes32 bridgeTransferId, bytes32 preImage) external onlyRole(RELAYER_ROLE) {
         BridgeTransfer storage bridgeTransfer = bridgeTransfers[bridgeTransferId];
 
         rateLimiter.rateLimitInbound(bridgeTransfer.amount);
@@ -116,7 +120,7 @@ contract AtomicBridgeInitiatorMOVE is IAtomicBridgeInitiatorMOVE, OwnableUpgrade
         emit BridgeTransferCompleted(bridgeTransferId, preImage);
     }
 
-    function refundBridgeTransfer(bytes32 bridgeTransferId) external onlyOwner {
+    function refundBridgeTransfer(bytes32 bridgeTransferId) external onlyRole(REFUNDER_ROLE) {
         BridgeTransfer storage bridgeTransfer = bridgeTransfers[bridgeTransferId];
         rateLimiter.rateLimitInbound(bridgeTransfer.amount);
         require(bridgeTransfer.state == MessageState.INITIALIZED, "BridgeTransferStateNotInitialized");
@@ -131,8 +135,6 @@ contract AtomicBridgeInitiatorMOVE is IAtomicBridgeInitiatorMOVE, OwnableUpgrade
 
     function withdrawMOVE(address recipient, uint256 amount) external {
         if (msg.sender != counterpartyAddress) revert Unauthorized();
-        if (poolBalance < amount) revert InsufficientMOVEBalance();
-        poolBalance -= amount;
         if (!moveToken.transfer(recipient, amount)) revert MOVETransferFailed();
     }
 }

protocol-units/bridge/contracts/src/IAtomicBridgeCounterpartyMOVE.sol

@@ -31,7 +31,7 @@ interface IAtomicBridgeCounterpartyMOVE {
      * @param bridgeTransferId A unique id representing this BridgeTransfer
      * @param hashLock The hash of the secret (HASH) that will unlock the funds
      * @param recipient The address to which to transfer the funds
-     * @param amount The amount of WETH to lock
+     * @param amount The amount of MOVE to lock
      *
      */
     function lockBridgeTransfer(
@@ -43,7 +43,7 @@ interface IAtomicBridgeCounterpartyMOVE {
     ) external;
 
     /**
-     * @dev Completes the bridge transfer and withdraws WETH to the recipient
+     * @dev Completes the bridge transfer and withdraws MOVE to the recipient
      * @param bridgeTransferId Unique identifier for the BridgeTransfer
      * @param preImage The secret that unlocks the funds
      *

protocol-units/bridge/contracts/src/IAtomicBridgeInitiatorMOVE.sol

@@ -30,14 +30,14 @@ interface IAtomicBridgeInitiatorMOVE {
 
 
     /**
-     * @dev Creates a new atomic bridge transfer using native ETH
-     * @param _wethAmount The amount of WETH to send
+     * @dev Creates a new atomic bridge transfer using MOVE tokens
+     * @param _amount The amount of MOVE to send
      * @param _recipient The address on the other chain to which to transfer the funds
      * @param _hashLock The hash of the secret (HASH) that will unlock the funds
      * @return _bridgeTransferId A unique id representing this BridgeTransfer
      *
      */
-    function initiateBridgeTransfer(uint256 _wethAmount, bytes32 _recipient, bytes32 _hashLock)
+    function initiateBridgeTransfer(uint256 _amount, bytes32 _recipient, bytes32 _hashLock)
         external
         returns (bytes32 _bridgeTransferId);
 

protocol-units/bridge/contracts/test/AtomicBridgeCounterpartyMOVE.t.sol

@@ -53,11 +53,12 @@ contract AtomicBridgeCounterpartyMOVETest is Test {
             address(atomicBridgeInitiatorMOVEImplementation),
             address(deployer),
             abi.encodeWithSignature(
-                "initialize(address,address,uint256,uint256)",
+                "initialize(address,address,address,address,uint256)",
                 address(moveToken),
                 deployer,
-                initiatorTimeLockDuration,
-                0 ether // Initial pool balance
+                deployer,
+                deployer,
+                initiatorTimeLockDuration
             )
         );
         atomicBridgeInitiatorMOVE = AtomicBridgeInitiatorMOVE(address(proxy));
@@ -68,9 +69,11 @@ contract AtomicBridgeCounterpartyMOVETest is Test {
             address(atomicBridgeCounterpartyMOVEImplementation),
             address(deployer),
             abi.encodeWithSignature(
-                "initialize(address,address,uint256)",
+                "initialize(address,address,address,address,uint256)",
                 address(atomicBridgeInitiatorMOVE),
                 deployer,
+                deployer,
+                deployer,
                 counterpartyTimeLockDuration
             )
         );

protocol-units/bridge/contracts/test/AtomicBridgeInitiatorMOVE.t.sol

@@ -3,12 +3,14 @@ pragma solidity ^0.8.22;
 pragma abicoder v2;
 
 import {Test, console} from "forge-std/Test.sol";
-import {AtomicBridgeInitiatorMOVE, IAtomicBridgeInitiatorMOVE, OwnableUpgradeable} from "../src/AtomicBridgeInitiatorMOVE.sol";
+import {AtomicBridgeInitiatorMOVE, IAtomicBridgeInitiatorMOVE} from "../src/AtomicBridgeInitiatorMOVE.sol";
+import {AccessControlUpgradeable, IAccessControl} from "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
 import {ProxyAdmin} from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";
 import {TransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
 import {MockMOVEToken} from "../src/MockMOVEToken.sol";  
 import {RateLimiter} from "../src/RateLimiter.sol";
 
+
 contract AtomicBridgeInitiatorMOVETest is Test {
     AtomicBridgeInitiatorMOVE public atomicBridgeInitiatorImplementation;
     MockMOVEToken public moveToken;   
@@ -40,11 +42,12 @@ contract AtomicBridgeInitiatorMOVETest is Test {
             address(atomicBridgeInitiatorImplementation),
             address(proxyAdmin),
             abi.encodeWithSignature(
-                "initialize(address,address,uint256,uint256)", 
+                "initialize(address,address,address,address,uint256)", 
                 address(moveToken), 
-                address(this), 
-                timeLockDuration,
-                0 ether
+                address(this),
+                address(this),
+                address(this),
+                timeLockDuration
             )
         );
 
@@ -178,7 +181,7 @@ contract AtomicBridgeInitiatorMOVETest is Test {
 
         // Test that a non-owner cannot call refund
         vm.startPrank(originator);
-        vm.expectRevert(abi.encodeWithSelector(OwnableUpgradeable.OwnableUnauthorizedAccount.selector, originator));
+        vm.expectRevert(abi.encodeWithSelector(IAccessControl.AccessControlUnauthorizedAccount.selector, originator, keccak256("REFUNDER_ROLE")));
         atomicBridgeInitiatorMOVE.refundBridgeTransfer(bridgeTransferId);
         vm.stopPrank();
 

protocol-units/bridge/setup/src/deploy.rs

@@ -351,7 +351,7 @@ pub fn deploy_on_movement_framework(config: &mut MovementConfig) -> Result<(), a
 
 	if !update_bridge_operator_output.stdout.is_empty() {
 		println!(
-			"run-script update_bridge_operatorstdout: {}",
+			"run-script update_bridge_operator_stdout: {}",
 			String::from_utf8_lossy(&update_bridge_operator_output.stdout)
 		);
 	}
@@ -362,6 +362,35 @@ pub fn deploy_on_movement_framework(config: &mut MovementConfig) -> Result<(), a
 		);
 	}
 
+	let update_refunder_output = Command::new("movement")
+			.args(&[
+				"move",
+				"run-script",
+				"--compiled-script-path",
+				"protocol-units/bridge/move-modules/build/bridge-modules/bytecode_scripts/update_refunder.mv",
+				"--args",
+				"address:0xf90391c81027f03cdea491ed8b36ffaced26b6df208a9b569e5baf2590eb9b16",
+				"--profile",
+				"default",
+				"--assume-yes",
+			])
+			.stdout(Stdio::piped())
+			.stderr(Stdio::piped())
+			.output()?;
+
+	if !update_refunder_output.stdout.is_empty() {
+		println!(
+			"run-script update_refunder_stdout: {}",
+			String::from_utf8_lossy(&update_refunder_output.stdout)
+		);
+	}
+	if !update_refunder_output.stderr.is_empty() {
+		eprintln!(
+			"run-script update_refunder supdate_refunder tderr: {}",
+			String::from_utf8_lossy(&update_refunder_output.stderr)
+		);
+	}
+
 	let set_initiator_time_lock_script_output = Command::new("movement")
 		.args(&[
 			"move",