Fix operation order in quorum / receiver update

src/MultiBridgeMessageReceiver.sol

@@ -73,17 +73,24 @@ contract MultiBridgeMessageReceiver is IMultiBridgeMessageReceiver, ExecutorAwar
     ////////////////////////////////////////////////////////////////*/
 
     /// @notice sets the initial parameters
-    constructor(uint256 _srcChainId, address _gac) {
-        if (_gac == address(0)) {
-            revert Error.ZERO_ADDRESS_INPUT();
-        }
-
+    constructor(uint256 _srcChainId, address _gac, address[] memory _receiverAdapters, uint64 _quorum) {
         if (_srcChainId == 0) {
             revert Error.INVALID_SENDER_CHAIN_ID();
         }
+        if (_gac == address(0)) {
+            revert Error.ZERO_ADDRESS_INPUT();
+        }
 
-        gac = IGAC(_gac);
         srcChainId = _srcChainId;
+        gac = IGAC(_gac);
+
+        for (uint256 i; i < _receiverAdapters.length;) {
+            _updateReceiverAdapter(_receiverAdapters[i], true);
+            unchecked {
+                ++i;
+            }
+        }
+        _updateQuorum(_quorum);
     }
 
     /*/////////////////////////////////////////////////////////////////
@@ -183,20 +190,18 @@ contract MultiBridgeMessageReceiver is IMultiBridgeMessageReceiver, ExecutorAwar
         onlyGlobalOwner
     {
         _updateReceiverAdapters(_receiverAdapters, _operations);
+        _validateQuorum(quorum);
     }
 
-    /// @notice Update bridge receiver adapters after quorum update
-    /// @dev called by admin to update receiver bridge adapters on all other chains
-    function updateQuorumAndReceiverAdapter(
-        uint64 _newQuorum,
+    /// @notice Update bridge receiver adapters and quorum
+    /// @dev called by admin to update receiver bridge adapters on all other chains along with quorum
+    function updateReceiverAdaptersAndQuorum(
         address[] calldata _receiverAdapters,
-        bool[] calldata _operations
+        bool[] calldata _operations,
+        uint64 _newQuorum
     ) external override onlyGlobalOwner {
-        /// @dev updates quorum here
-        _updateQuorum(_newQuorum);
-
-        /// @dev updates receiver adapter here
         _updateReceiverAdapters(_receiverAdapters, _operations);
+        _updateQuorum(_newQuorum);
     }
 
     /// @notice Update power quorum threshold of message execution.
@@ -238,18 +243,16 @@ contract MultiBridgeMessageReceiver is IMultiBridgeMessageReceiver, ExecutorAwar
                             PRIVATE/INTERNAL FUNCTIONS
     ////////////////////////////////////////////////////////////////*/
 
-    function _updateQuorum(uint64 _quorum) internal {
-        if (_quorum > trustedExecutorsCount() || _quorum == 0) {
-            revert Error.INVALID_QUORUM_THRESHOLD();
-        }
+    function _updateQuorum(uint64 _quorum) private {
+        _validateQuorum(_quorum);
 
         uint64 oldValue = quorum;
 
         quorum = _quorum;
         emit QuorumUpdated(oldValue, _quorum);
     }
 
-    function _updateReceiverAdapters(address[] memory _receiverAdapters, bool[] memory _operations) internal {
+    function _updateReceiverAdapters(address[] memory _receiverAdapters, bool[] memory _operations) private {
         uint256 len = _receiverAdapters.length;
 
         if (len == 0) {
@@ -261,10 +264,6 @@ contract MultiBridgeMessageReceiver is IMultiBridgeMessageReceiver, ExecutorAwar
         }
 
         for (uint256 i; i < len;) {
-            if (_receiverAdapters[i] == address(0)) {
-                revert Error.ZERO_ADDRESS_INPUT();
-            }
-
             _updateReceiverAdapter(_receiverAdapters[i], _operations[i]);
 
             unchecked {
@@ -274,15 +273,20 @@ contract MultiBridgeMessageReceiver is IMultiBridgeMessageReceiver, ExecutorAwar
     }
 
     function _updateReceiverAdapter(address _receiverAdapter, bool _add) private {
+        if (_receiverAdapter == address(0)) {
+            revert Error.ZERO_ADDRESS_INPUT();
+        }
         if (_add) {
             _addTrustedExecutor(_receiverAdapter);
         } else {
             _removeTrustedExecutor(_receiverAdapter);
-
-            if (quorum > trustedExecutorsCount()) {
-                revert Error.INVALID_QUORUM_THRESHOLD();
-            }
         }
         emit BridgeReceiverAdapterUpdated(_receiverAdapter, _add);
     }
+
+    function _validateQuorum(uint256 _quorum) private view {
+        if (_quorum > trustedExecutorsCount() || _quorum == 0) {
+            revert Error.INVALID_QUORUM_THRESHOLD();
+        }
+    }
 }

src/interfaces/IMultiBridgeMessageReceiver.sol

@@ -67,12 +67,12 @@ interface IMultiBridgeMessageReceiver {
     function updateQuorum(uint64 _quorum) external;
 
     /// @notice updates the the list of receiver adapters and the quorum for message validity.
-    /// @param _newQuorum is the new quorum value
     /// @param _receiverAdapters the list of receiver adapters to add or remove
     /// @param _operations the list of operations to perform for corresponding receiver adapters, true for add, false for remove
-    function updateQuorumAndReceiverAdapter(
-        uint64 _newQuorum,
+    /// @param _newQuorum is the new quorum value
+    function updateReceiverAdaptersAndQuorum(
         address[] calldata _receiverAdapters,
-        bool[] calldata _operations
+        bool[] calldata _operations,
+        uint64 _newQuorum
     ) external;
 }

test/Setup.t.sol

@@ -275,8 +275,13 @@ abstract contract Setup is Test {
             uint256 chainId = DST_CHAINS[i];
 
             vm.selectFork(fork[chainId]);
+
+            address[] memory _receiverAdapters = new address[](2);
+            _receiverAdapters[0] = contractAddress[chainId][bytes("WORMHOLE_RECEIVER_ADAPTER")];
+            _receiverAdapters[1] = contractAddress[chainId][bytes("AXELAR_RECEIVER_ADAPTER")];
+
             address mmaReceiver = address(
-                new MultiBridgeMessageReceiver{salt: _salt}(SRC_CHAIN_ID, contractAddress[chainId][bytes("GAC")])
+                new MultiBridgeMessageReceiver{salt: _salt}(SRC_CHAIN_ID, contractAddress[chainId][bytes("GAC")], _receiverAdapters, 2)
             );
             contractAddress[chainId][bytes("MMA_RECEIVER")] = mmaReceiver;
             contractAddress[chainId][bytes("TIMELOCK")] =
@@ -304,21 +309,11 @@ abstract contract Setup is Test {
         for (uint256 i; i < DST_CHAINS.length;) {
             uint256 chainId = DST_CHAINS[i];
 
-            address[] memory _receiverAdapters = new address[](2);
-            _receiverAdapters[0] = contractAddress[chainId][bytes("WORMHOLE_RECEIVER_ADAPTER")];
-            _receiverAdapters[1] = contractAddress[chainId][bytes("AXELAR_RECEIVER_ADAPTER")];
-
-            bool[] memory _operations = new bool[](2);
-            _operations[0] = true;
-            _operations[1] = true;
-
             vm.selectFork(fork[chainId]);
 
             MultiBridgeMessageReceiver dstMMReceiver =
                 MultiBridgeMessageReceiver(contractAddress[chainId][bytes("MMA_RECEIVER")]);
-            dstMMReceiver.updateReceiverAdapters(_receiverAdapters, _operations);
             dstMMReceiver.updateGovernanceTimelock(contractAddress[chainId]["TIMELOCK"]);
-            dstMMReceiver.updateQuorum(2);
 
             MessageReceiverGAC receiverGAC = MessageReceiverGAC(contractAddress[chainId][bytes("GAC")]);
             receiverGAC.setMultiBridgeMessageReceiver(address(dstMMReceiver));

test/integration-tests/RemoteAdapterRemove.t.sol

@@ -74,10 +74,10 @@ contract RemoteAdapterRemove is Setup {
             DST_CHAIN_ID,
             address(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]),
             abi.encodeWithSelector(
-                MultiBridgeMessageReceiver.updateQuorumAndReceiverAdapter.selector,
-                newQuorum,
+                MultiBridgeMessageReceiver.updateReceiverAdaptersAndQuorum.selector,
                 adaptersToRemove,
-                operation
+                operation,
+                newQuorum
             ),
             0,
             EXPIRATION_CONSTANT,

test/unit-tests/MultiBridgeMessageReceiver.t.sol

@@ -37,16 +37,58 @@ contract MultiBridgeMessageReceiverTest is Setup {
         timelockAddr = contractAddress[DST_CHAIN_ID]["TIMELOCK"];
     }
 
+    /// @dev verifies default setup
+    function test_constructor() public {
+        assertEq(receiver.srcChainId(), SRC_CHAIN_ID);
+        assertEq(address(receiver.gac()), contractAddress[DST_CHAIN_ID]["GAC"]);
+        assertEq(receiver.quorum(), 2);
+        assertTrue(receiver.isTrustedExecutor(wormholeAdapterAddr));
+        assertTrue(receiver.isTrustedExecutor(axelarAdapterAddr));
+    }
+
     /// @dev cannot be called with zero source chain id
     function test_constructor_zero_chain_id_input() public {
+        address[] memory receiverAdapters = new address[](1);
+        receiverAdapters[0] = address(43);
+
         vm.expectRevert(Error.INVALID_SENDER_CHAIN_ID.selector);
-        new MultiBridgeMessageReceiver(0, address(42));
+        new MultiBridgeMessageReceiver(0, address(42), receiverAdapters, 1);
     }
 
     /// @dev cannot be called with zero address GAC
     function test_constructor_zero_gac_address_input() public {
+        address[] memory receiverAdapters = new address[](1);
+        receiverAdapters[0] = address(43);
+
+        vm.expectRevert(Error.ZERO_ADDRESS_INPUT.selector);
+        new MultiBridgeMessageReceiver(SRC_CHAIN_ID, address(0), receiverAdapters, 1);
+    }
+
+    /// @dev cannot be called with receiver adapters containing zero address
+    function test_constructor_zero_address_adapter() public {
+        address[] memory receiverAdapters = new address[](1);
+        receiverAdapters[0] = address(0);
+
         vm.expectRevert(Error.ZERO_ADDRESS_INPUT.selector);
-        new MultiBridgeMessageReceiver(1, address(0));
+        new MultiBridgeMessageReceiver(SRC_CHAIN_ID, address(42), receiverAdapters, 1);
+    }
+
+    /// @dev cannot be called with zero quorum
+    function test_constructor_zero_quorum() public {
+        address[] memory receiverAdapters = new address[](1);
+        receiverAdapters[0] = address(42);
+
+        vm.expectRevert(Error.INVALID_QUORUM_THRESHOLD.selector);
+        new MultiBridgeMessageReceiver(SRC_CHAIN_ID, address(43), receiverAdapters, 0);
+    }
+
+    /// @dev cannot be called with quorum too large
+    function test_constructor_quorum_too_large() public {
+        address[] memory receiverAdapters = new address[](1);
+        receiverAdapters[0] = address(42);
+
+        vm.expectRevert(Error.INVALID_QUORUM_THRESHOLD.selector);
+        new MultiBridgeMessageReceiver(SRC_CHAIN_ID, address(43), receiverAdapters, 2);
     }
 
     /// @dev receives message from one adapter
@@ -322,6 +364,22 @@ contract MultiBridgeMessageReceiverTest is Setup {
         receiver.scheduleMessageExecution(msgId);
     }
 
+    /// @dev updates governance timelock
+    function test_update_governance_timelock() public {
+        vm.startPrank(timelockAddr);
+
+        receiver.updateGovernanceTimelock(address(42));
+        assertEq(receiver.governanceTimelock(), address(42));
+    }
+
+    /// @dev cannot update governance timelock with zero address
+    function test_update_governance_timelock_zero_address() public {
+        vm.startPrank(timelockAddr);
+
+        vm.expectRevert(Error.ZERO_GOVERNANCE_TIMELOCK.selector);
+        receiver.updateGovernanceTimelock(address(0));
+    }
+
     /// @dev adds one receiver adapter
     function test_update_receiver_adapter_add() public {
         vm.startPrank(timelockAddr);
@@ -383,6 +441,27 @@ contract MultiBridgeMessageReceiverTest is Setup {
         receiver.updateReceiverAdapters(adapters, operations);
     }
 
+    /// @dev cannot update with empty adapters list
+    function test_update_receiver_adapter_empty_lst() public {
+        vm.startPrank(timelockAddr);
+
+        vm.expectRevert(Error.ZERO_RECEIVER_ADAPTER.selector);
+
+        receiver.updateReceiverAdapters(new address[](0), new bool[](0));
+    }
+
+    /// @dev cannot update with zero adapter address
+    function test_update_receiver_adapter_zero_address() public {
+        vm.startPrank(timelockAddr);
+
+        address[] memory adapters = new address[](1);
+        adapters[0] = address(0);
+
+        vm.expectRevert(Error.ZERO_ADDRESS_INPUT.selector);
+
+        receiver.updateReceiverAdapters(adapters, new bool[](1));
+    }
+
     /// @dev cannot remove one receiver adapter without reducing quorum first
     function test_update_receiver_adapter_remove_invalid_quorum_threshold() public {
         vm.startPrank(timelockAddr);
@@ -462,13 +541,86 @@ contract MultiBridgeMessageReceiverTest is Setup {
         uint64 newQuorum = 1;
 
         /// @dev removes the newly updated adapter by reducing quorum by one
-        receiver.updateQuorumAndReceiverAdapter(newQuorum, adapters, new bool[](1));
+        receiver.updateReceiverAdaptersAndQuorum(adapters, new bool[](1), newQuorum);
 
         /// @dev asserts the quorum and adapter lengths
         assertEq(receiver.quorum(), newQuorum);
         assertEq(receiver.isTrustedExecutor(adapters[0]), false);
     }
 
+    /// @dev valid quorum and receiver updater in one single call, adding adapters and increasing quorum
+    function test_quorum_and_receiver_updater_add_increase() public {
+        vm.startPrank(timelockAddr);
+
+        // First, add one adapter
+        address[] memory addOneAdapter = new address[](1);
+        addOneAdapter[0] = address(42);
+        bool[] memory addOneOps = new bool[](1);
+        addOneOps[0] = true;
+
+        // Add two more and update quorum to 4
+        address[] memory addTwoAdapters = new address[](2);
+        addTwoAdapters[0] = address(420);
+        addTwoAdapters[1] = address(421);
+
+        bool[] memory addTwoOps = new bool[](2);
+        addTwoOps[0] = true;
+        addTwoOps[1] = true;
+
+        uint64 newQuorum = 4;
+
+        receiver.updateReceiverAdaptersAndQuorum(addTwoAdapters, addTwoOps, newQuorum);
+
+        /// @dev asserts the quorum and adapter lengths
+        assertEq(receiver.quorum(), newQuorum);
+        assertEq(receiver.isTrustedExecutor(addTwoAdapters[0]), true);
+        assertEq(receiver.isTrustedExecutor(addTwoAdapters[1]), true);
+    }
+
+    /// @dev valid quorum and receiver updater in one single call, removing adapter and decreasing quorum
+    function test_quorum_and_receiver_updater_remove_decrease() public {
+        vm.startPrank(timelockAddr);
+
+        // Remove one adapter and update quorum to 1
+        address[] memory removeOneAdapter = new address[](1);
+        removeOneAdapter[0] = axelarAdapterAddr;
+
+        uint64 newQuorum = 1;
+
+        receiver.updateReceiverAdaptersAndQuorum(removeOneAdapter, new bool[](1), newQuorum);
+
+        /// @dev asserts the quorum and adapter lengths
+        assertEq(receiver.quorum(), newQuorum);
+        assertEq(receiver.isTrustedExecutor(wormholeAdapterAddr), true);
+        assertEq(receiver.isTrustedExecutor(axelarAdapterAddr), false);
+    }
+
+    /// @dev valid quorum and receiver updater in one single call, removing one, adding two and increasing quorum
+    function test_quorum_and_receiver_updater_remove_add_increase() public {
+        vm.startPrank(timelockAddr);
+
+        // Remove one adapter and update quorum to 1
+        address[] memory removeAddAdapters = new address[](3);
+        removeAddAdapters[0] = axelarAdapterAddr;
+        removeAddAdapters[1] = address(42);
+        removeAddAdapters[2] = address(43);
+        bool[] memory removeAddOps = new bool[](3);
+        removeAddOps[0] = false;
+        removeAddOps[1] = true;
+        removeAddOps[2] = true;
+
+        uint64 newQuorum = 3;
+
+        receiver.updateReceiverAdaptersAndQuorum(removeAddAdapters, removeAddOps, newQuorum);
+
+        /// @dev asserts the quorum and adapter lengths
+        assertEq(receiver.quorum(), newQuorum);
+        assertEq(receiver.isTrustedExecutor(wormholeAdapterAddr), true);
+        assertEq(receiver.isTrustedExecutor(axelarAdapterAddr), false);
+        assertEq(receiver.isTrustedExecutor(removeAddAdapters[1]), true);
+        assertEq(receiver.isTrustedExecutor(removeAddAdapters[2]), true);
+    }
+
     /// @dev should get message info
     function test_get_message_info() public {
         vm.startPrank(wormholeAdapterAddr);