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

@@ -40,13 +40,21 @@ contract MultiBridgeMessageReceiverTest is Setup {
  /// @dev cannot be called with zero source chain id
  function test_constructor_zero_chain_id_input() public {
  vm.expectRevert(Error.INVALID_SENDER_CHAIN_ID.selector);
- new MultiBridgeMessageReceiver(0, address(42));
+
+ address[] memory receiverAdapters = new address[](1);
+ receiverAdapters[0] = address(43);
+
+ new MultiBridgeMessageReceiver(0, address(42), receiverAdapters, 1);
  }
 
  /// @dev cannot be called with zero address GAC
  function test_constructor_zero_gac_address_input() public {
  vm.expectRevert(Error.ZERO_ADDRESS_INPUT.selector);
- new MultiBridgeMessageReceiver(1, address(0));
+
+ address[] memory receiverAdapters = new address[](1);
+ receiverAdapters[0] = address(43);
+
+ new MultiBridgeMessageReceiver(1, address(0), receiverAdapters, 1);
  }
 
  /// @dev receives message from one adapter
@@ -462,13 +470,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);