feat: revert if all message bridges fail (#97)

* feat: revert if all message bridges fail * chore: rename error message * chore: add ways for caller to specify successful senders * chore: remove old functions * fix: pr comments * chore: add more tests * chore: add linter
MultiMessageAggregation · Oct 9, 2023 · 0575e02 · 0575e02
1 parent b633bae
commit 0575e02
Show file tree

Hide file tree

Showing 11 changed files with 271 additions and 30 deletions.
diff --git a/src/MultiBridgeMessageSender.sol b/src/MultiBridgeMessageSender.sol
@@ -28,6 +28,7 @@ contract MultiBridgeMessageSender {
  uint256 expiration;
  address refundAddress;
  uint256[] fees;
+ uint256 successThreshold;
  address[] excludedAdapters;
  }
 
@@ -145,6 +146,7 @@ contract MultiBridgeMessageSender {
  /// @param _refundAddress refers to the refund address for any extra native tokens paid
  /// @param _fees refers to the fees to pay to each sender adapter that is not in the exclusion list specified by _excludedAdapters.
  /// The fees are in the same order as the sender adapters in the senderAdapters list, after the exclusion list is applied.
+ /// @param _successThreshold specifies the minimum number of bridges that must successfully dispatch the message for this call to succeed.
  /// @param _excludedAdapters are the sender adapters to be excluded from relaying the message, in ascending order by address
  function remoteCall(
  uint256 _dstChainId,
@@ -154,11 +156,20 @@ contract MultiBridgeMessageSender {
  uint256 _expiration,
  address _refundAddress,
  uint256[] calldata _fees,
- address[] calldata _excludedAdapters
+ uint256 _successThreshold,
+ address[] memory _excludedAdapters
  ) external payable onlyCaller validateExpiration(_expiration) {
  _remoteCall(
  RemoteCallArgs(
- _dstChainId, _target, _callData, _nativeValue, _expiration, _refundAddress, _fees, _excludedAdapters
+ _dstChainId,
+ _target,
+ _callData,
+ _nativeValue,
+ _expiration,
+ _refundAddress,
+ _fees,
+ _successThreshold,
+ _excludedAdapters
  )
  );
  }
@@ -237,7 +248,12 @@ contract MultiBridgeMessageSender {
 
  function _remoteCall(RemoteCallArgs memory _args) private {
  (address mmaReceiver, address[] memory adapters) = _checkAndProcessArgs(
- _args.dstChainId, _args.target, _args.refundAddress, _args.fees, _args.excludedAdapters
+ _args.dstChainId,
+ _args.target,
+ _args.refundAddress,
+ _args.successThreshold,
+ _args.fees,
+ _args.excludedAdapters
  );
 
  /// @dev increments nonce
@@ -253,7 +269,12 @@ contract MultiBridgeMessageSender {
  block.timestamp + _args.expiration
  );
  bytes32 msgId = MessageLibrary.computeMsgId(message);
- bool[] memory adapterSuccess = _dispatchMessages(adapters, mmaReceiver, _args.dstChainId, message, _args.fees);
+ (bool[] memory adapterSuccess, uint256 successCount) =
+ _dispatchMessages(adapters, mmaReceiver, _args.dstChainId, message, _args.fees);
+
+ if (successCount < _args.successThreshold) {
+ revert Error.MULTI_MESSAGE_SEND_FAILED();
+ }
 
  emit MultiBridgeMessageSent(
  msgId,
@@ -277,6 +298,7 @@ contract MultiBridgeMessageSender {
  uint256 _dstChainId,
  address _target,
  address _refundAddress,
+ uint256 _successThreshold,
  uint256[] memory _fees,
  address[] memory _excludedAdapters
  ) private view returns (address mmaReceiver, address[] memory adapters) {
@@ -307,6 +329,11 @@ contract MultiBridgeMessageSender {
  if (adapters.length == 0) {
  revert Error.NO_SENDER_ADAPTER_FOUND();
  }
+
+ if (_successThreshold > adapters.length) {
+ revert Error.MULTI_MESSAGE_SEND_FAILED();
+ }
+
  if (adapters.length != _fees.length) {
  revert Error.INVALID_SENDER_ADAPTER_FEES();
  }
@@ -328,15 +355,18 @@ contract MultiBridgeMessageSender {
  uint256 _dstChainId,
  MessageLibrary.Message memory _message,
  uint256[] memory _fees
- ) private returns (bool[] memory) {
+ ) private returns (bool[] memory, uint256) {
  uint256 len = _adapters.length;
  bool[] memory successes = new bool[](len);
+ uint256 successCount;
+
  for (uint256 i; i < len;) {
  /// @dev if one bridge is paused, the flow shouldn't be broken
  try IMessageSenderAdapter(_adapters[i]).dispatchMessage{value: _fees[i]}(
  _dstChainId, _mmaReceiver, abi.encode(_message)
  ) {
  successes[i] = true;
+ ++successCount;
  } catch {
  successes[i] = false;
  emit MessageSendFailed(_adapters[i], _message);
@@ -346,7 +376,7 @@ contract MultiBridgeMessageSender {
  ++i;
  }
  }
- return successes;
+ return (successes, successCount);
  }
 
  function _filterAdapters(address[] memory _existings, address[] memory _removals)

diff --git a/src/libraries/Error.sol b/src/libraries/Error.sol
@@ -106,6 +106,9 @@ library Error {
  /// @dev is thrown if contract call is invalid (for axelar)
  error NOT_APPROVED_BY_GATEWAY();
 
+ /// @dev is thrown if a message could not be sent through a sufficient number of bridges
+ error MULTI_MESSAGE_SEND_FAILED();
+
  /*/////////////////////////////////////////////////////////////////
  TIMELOCK ERRORS
  ////////////////////////////////////////////////////////////////*/

diff --git a/test/Setup.t.sol b/test/Setup.t.sol
@@ -46,6 +46,7 @@ abstract contract Setup is Test {
  address constant owner = address(420);
  address constant refundAddress = address(420420);
  uint256 constant EXPIRATION_CONSTANT = 5 days;
+ uint256 constant DEFAULT_SUCCESS_THRESHOLD = 2;
 
  /// @dev constants for axelar
  address constant ETH_GATEWAY = 0x4F4495243837681061C4743b74B3eEdf548D56A5;

diff --git a/test/integration-tests/GracePeriodExpiry.t.sol b/test/integration-tests/GracePeriodExpiry.t.sol
@@ -48,6 +48,7 @@ contract GracePeriodExpiryTest is Setup {
  EXPIRATION_CONSTANT,
  refundAddress,
  fees,
+ DEFAULT_SUCCESS_THRESHOLD,
  new address[](0)
  );
 

diff --git a/test/integration-tests/MultiMessageAggregation.t.sol b/test/integration-tests/MultiMessageAggregation.t.sol
@@ -48,6 +48,7 @@ contract MultiBridgeMessageAggregationTest is Setup {
  EXPIRATION_CONSTANT,
  refundAddress,
  fees,
+ DEFAULT_SUCCESS_THRESHOLD,
  new address[](0)
  );
 

diff --git a/test/integration-tests/RemoteAdapterAdd.t.sol b/test/integration-tests/RemoteAdapterAdd.t.sol
@@ -73,6 +73,7 @@ contract RemoteAdapterAdd is Setup {
  EXPIRATION_CONSTANT,
  refundAddress,
  fees,
+ DEFAULT_SUCCESS_THRESHOLD,
  new address[](0)
  );
 

diff --git a/test/integration-tests/RemoteAdapterRemove.t.sol b/test/integration-tests/RemoteAdapterRemove.t.sol
@@ -83,6 +83,7 @@ contract RemoteAdapterRemove is Setup {
  EXPIRATION_CONSTANT,
  refundAddress,
  fees,
+ DEFAULT_SUCCESS_THRESHOLD,
  new address[](0)
  );
 

diff --git a/test/integration-tests/RemoteSetQuorum.t.sol b/test/integration-tests/RemoteSetQuorum.t.sol
@@ -45,6 +45,7 @@ contract RemoteQuorumUpdate is Setup {
  EXPIRATION_CONSTANT,
  refundAddress,
  fees,
+ DEFAULT_SUCCESS_THRESHOLD,
  new address[](0)
  );
 

diff --git a/test/integration-tests/RemoteTimelockUpdate.t.sol b/test/integration-tests/RemoteTimelockUpdate.t.sol
@@ -45,6 +45,7 @@ contract RemoteTimelockUpdate is Setup {
  EXPIRATION_CONSTANT,
  refundAddress,
  fees,
+ DEFAULT_SUCCESS_THRESHOLD,
  new address[](0)
  );
 

diff --git a/test/integration-tests/TimelockCheck.t.sol b/test/integration-tests/TimelockCheck.t.sol
@@ -50,6 +50,7 @@ contract TimelockCheckTest is Setup {
  EXPIRATION_CONSTANT,
  refundAddress,
  fees,
+ DEFAULT_SUCCESS_THRESHOLD,
  new address[](0)
  );