fix: issue 3.2.4 from audit report, introduce setRelayer function for…

… gateway.sol
primev · Nov 7, 2024 · 530add7 · 530add7
1 parent 6d49ffd
commit 530add7
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 3 deletions.
diff --git a/contracts/contracts/interfaces/IGateway.sol b/contracts/contracts/interfaces/IGateway.sol
@@ -34,10 +34,14 @@ interface IGateway {
 
     event FinalizationFeeSet(uint256 finalizationFee);
     event CounterpartyFeeSet(uint256 counterpartyFee);
+    event RelayerSet(address indexed relayer);
 
     error SenderNotRelayer(address sender, address relayer);
     error AmountTooSmall(uint256 amount, uint256 counterpartyFee);
     error InvalidCounterpartyIndex(uint256 counterpartyIdx, uint256 transferFinalizedIdx);
+    error FinalizationFeeTooSmall(uint256 _finalizationFee);
+    error CounterpartyFeeTooSmall(uint256 _counterpartyFee);
+    error RelayerCannotBeZeroAddress();
 
     /**
      * @notice Initiates a cross-chain transfer.

diff --git a/contracts/contracts/standard-bridge/Gateway.sol b/contracts/contracts/standard-bridge/Gateway.sol
@@ -11,9 +11,6 @@ import {GatewayStorage} from "./GatewayStorage.sol";
 abstract contract Gateway is IGateway, GatewayStorage,
     Ownable2StepUpgradeable, UUPSUpgradeable, PausableUpgradeable, ReentrancyGuardUpgradeable {   
 
-    error FinalizationFeeTooSmall(uint256 _finalizationFee);
-    error CounterpartyFeeTooSmall(uint256 _counterpartyFee);
-
     modifier onlyRelayer() {
         require(msg.sender == relayer, SenderNotRelayer(msg.sender, relayer));
         _;
@@ -53,6 +50,13 @@ abstract contract Gateway is IGateway, GatewayStorage,
     /// @dev Allows owner to unpause the contract.
     function unpause() external onlyOwner { _unpause(); }
 
+    /// @dev Allows owner to set a new relayer account.
+    function setRelayer(address _relayer) external onlyOwner {
+        require(_relayer != address(0), RelayerCannotBeZeroAddress());
+        relayer = _relayer;
+        emit RelayerSet(_relayer);
+    }
+
     /// @dev Allows owner to set a new finalization fee.
     /// @notice If using this function, ensure the same value is set as the `counterpartyFee` in the counterparty contract.
     function setFinalizationFee(uint256 _finalizationFee) external onlyOwner {

diff --git a/contracts/test/standard-bridge/L1GatewayTest.sol b/contracts/test/standard-bridge/L1GatewayTest.sol
@@ -71,6 +71,24 @@ contract L1GatewayTest is Test {
         assertEq(l1Gateway.counterpartyFee(), 0.0005 ether);
     }
 
+    event RelayerSet(address indexed relayer);
+
+    function test_SetRelayer() public {
+
+        vm.expectRevert(
+            abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, vm.addr(888))
+        );
+        vm.prank(vm.addr(888));
+        l1Gateway.setRelayer(address(0x123));
+
+        assertEq(l1Gateway.relayer(), address(0x78));
+
+        vm.expectEmit(true, true, true, true);
+        emit RelayerSet(address(0x12345));
+        l1Gateway.setRelayer(address(0x12345));
+        assertEq(l1Gateway.relayer(), address(0x12345));
+    }
+
     // Expected event signature emitted in initiateTransfer()
     event TransferInitiated(
         address indexed sender, address indexed recipient, uint256 amount, uint256 indexed transferIdx);

diff --git a/contracts/test/standard-bridge/SettlementGatewayTest.sol b/contracts/test/standard-bridge/SettlementGatewayTest.sol
@@ -9,6 +9,7 @@ import {IGateway} from "../../contracts/interfaces/IGateway.sol";
 import {IAllocator} from "../../contracts/interfaces/IAllocator.sol";
 import {RevertingReceiver} from "./RevertingReceiver.sol";
 import {EventReceiver} from "./EventReceiver.sol";
+import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
 
 contract SettlementGatewayTest is Test {
 
@@ -65,6 +66,55 @@ contract SettlementGatewayTest is Test {
     event TransferNeedsWithdrawal(address indexed recipient, uint256 amount);
     event TransferSuccess(address indexed recipient, uint256 amount);
 
+    event FinalizationFeeSet(uint256 finalizationFee);
+    event CounterpartyFeeSet(uint256 counterpartyFee);
+
+    function test_SetFinalizationFee() public {
+        vm.expectRevert(
+            abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, vm.addr(888))
+        );
+        vm.prank(vm.addr(888));
+        settlementGateway.setFinalizationFee(0.0015 ether);
+
+        assertEq(settlementGateway.finalizationFee(), 0.05 ether);
+        vm.expectEmit(true, true, true, true);
+        emit FinalizationFeeSet(0.0015 ether);
+        settlementGateway.setFinalizationFee(0.0015 ether);
+        assertEq(settlementGateway.finalizationFee(), 0.0015 ether);
+    }
+
+    function test_SetCounterpartyFee() public {
+        vm.expectRevert(
+            abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, vm.addr(888))
+        );
+        vm.prank(vm.addr(888));
+        settlementGateway.setCounterpartyFee(0.0005 ether);
+
+        assertEq(settlementGateway.counterpartyFee(), 0.1 ether);
+        vm.expectEmit(true, true, true, true);
+        emit CounterpartyFeeSet(0.0005 ether);
+        settlementGateway.setCounterpartyFee(0.0005 ether);
+        assertEq(settlementGateway.counterpartyFee(), 0.0005 ether);
+    }
+
+    event RelayerSet(address indexed relayer);
+
+    function test_SetRelayer() public {
+
+        vm.expectRevert(
+            abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, vm.addr(888))
+        );
+        vm.prank(vm.addr(888));
+        settlementGateway.setRelayer(address(0x123));
+
+        assertEq(settlementGateway.relayer(), address(0x78));
+
+        vm.expectEmit(true, true, true, true);
+        emit RelayerSet(address(0x12345));
+        settlementGateway.setRelayer(address(0x12345));
+        assertEq(settlementGateway.relayer(), address(0x12345));
+    }
+
     function test_InitiateTransferSuccess() public {
         vm.deal(bridgeUser, 100 ether);
         uint256 amount = 7 ether;