From 068a480a8bd25c69e85866d8bbe0e33cb769aa4b Mon Sep 17 00:00:00 2001 From: Sujith Date: Tue, 3 Oct 2023 23:16:54 -0400 Subject: [PATCH 1/7] feat: revert if all message bridges fail --- src/MultiBridgeMessageSender.sol | 14 +++++++++++--- src/libraries/Error.sol | 3 +++ test/unit-tests/MultiBridgeMessageSender.t.sol | 10 ++++++++++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/MultiBridgeMessageSender.sol b/src/MultiBridgeMessageSender.sol index cfdc9ae..53e546a 100644 --- a/src/MultiBridgeMessageSender.sol +++ b/src/MultiBridgeMessageSender.sol @@ -281,7 +281,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 == 0) { + revert Error.MESSAGE_BRIDGE_FAILURE(); + } emit MultiBridgeMessageSent( msgId, @@ -356,15 +361,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); @@ -374,7 +382,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 index 7c00d3a..d3565ae 100644 --- a/src/libraries/Error.sol +++ b/src/libraries/Error.sol @@ -102,6 +102,9 @@ library Error { /// @dev is thrown if contract call is invalid (for axelar) error NOT_APPROVED_BY_GATEWAY(); + /// @dev is thrown if all message bridges fail + error MESSAGE_BRIDGE_FAILURE(); + /*///////////////////////////////////////////////////////////////// TIMELOCK ERRORS ////////////////////////////////////////////////////////////////*/ diff --git a/test/unit-tests/MultiBridgeMessageSender.t.sol b/test/unit-tests/MultiBridgeMessageSender.t.sol index bf890b5..8861472 100644 --- a/test/unit-tests/MultiBridgeMessageSender.t.sol +++ b/test/unit-tests/MultiBridgeMessageSender.t.sol @@ -609,4 +609,14 @@ contract MultiBridgeMessageSenderTest is Setup { vm.expectRevert(Error.SENDER_ADAPTER_NOT_EXIST.selector); sender.removeSenderAdapters(adapters); } + + /// @dev if all message bridges fail (not pay enough fees) + function test_all_message_bridge_failure() public { + vm.startPrank(caller); + + vm.expectRevert(Error.MESSAGE_BRIDGE_FAILURE.selector); + sender.remoteCall( + DST_CHAIN_ID, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, refundAddress, new uint256[](2) + ); + } } From c18dc673b42b882c3b1a9abb44fda10e2f2804c8 Mon Sep 17 00:00:00 2001 From: Sujith Date: Thu, 5 Oct 2023 09:08:30 -0400 Subject: [PATCH 2/7] chore: rename error message --- src/MultiBridgeMessageSender.sol | 2 +- src/libraries/Error.sol | 2 +- test/unit-tests/MultiBridgeMessageSender.t.sol | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/MultiBridgeMessageSender.sol b/src/MultiBridgeMessageSender.sol index 53e546a..1afcc44 100644 --- a/src/MultiBridgeMessageSender.sol +++ b/src/MultiBridgeMessageSender.sol @@ -285,7 +285,7 @@ contract MultiBridgeMessageSender { _dispatchMessages(adapters, mmaReceiver, _args.dstChainId, message, _args.fees); if (successCount == 0) { - revert Error.MESSAGE_BRIDGE_FAILURE(); + revert Error.MULTI_MESSAGE_SEND_FAILED(); } emit MultiBridgeMessageSent( diff --git a/src/libraries/Error.sol b/src/libraries/Error.sol index d3565ae..987f716 100644 --- a/src/libraries/Error.sol +++ b/src/libraries/Error.sol @@ -103,7 +103,7 @@ library Error { error NOT_APPROVED_BY_GATEWAY(); /// @dev is thrown if all message bridges fail - error MESSAGE_BRIDGE_FAILURE(); + error MULTI_MESSAGE_SEND_FAILED(); /*///////////////////////////////////////////////////////////////// TIMELOCK ERRORS diff --git a/test/unit-tests/MultiBridgeMessageSender.t.sol b/test/unit-tests/MultiBridgeMessageSender.t.sol index 8861472..0a9b997 100644 --- a/test/unit-tests/MultiBridgeMessageSender.t.sol +++ b/test/unit-tests/MultiBridgeMessageSender.t.sol @@ -611,10 +611,10 @@ contract MultiBridgeMessageSenderTest is Setup { } /// @dev if all message bridges fail (not pay enough fees) - function test_all_message_bridge_failure() public { + function test_all_MULTI_MESSAGE_SEND_FAILED() public { vm.startPrank(caller); - vm.expectRevert(Error.MESSAGE_BRIDGE_FAILURE.selector); + vm.expectRevert(Error.MULTI_MESSAGE_SEND_FAILED.selector); sender.remoteCall( DST_CHAIN_ID, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, refundAddress, new uint256[](2) ); From 6cedaa55a3ceb852c2a3194a5c903a3cd2471297 Mon Sep 17 00:00:00 2001 From: Sujith Date: Thu, 5 Oct 2023 09:33:27 -0400 Subject: [PATCH 3/7] chore: add ways for caller to specify successful senders --- src/MultiBridgeMessageSender.sol | 37 +++- test/Setup.t.sol | 1 + .../integration-tests/GracePeriodExpiry.t.sol | 3 +- .../MultiMessageAggregation.t.sol | 3 +- test/integration-tests/RemoteAdapterAdd.t.sol | 3 +- .../RemoteAdapterRemove.t.sol | 3 +- test/integration-tests/RemoteSetQuorum.t.sol | 3 +- .../RemoteTimelockUpdate.t.sol | 3 +- test/integration-tests/TimelockCheck.t.sol | 3 +- .../unit-tests/MultiBridgeMessageSender.t.sol | 180 +++++++++++++++--- 10 files changed, 198 insertions(+), 41 deletions(-) diff --git a/src/MultiBridgeMessageSender.sol b/src/MultiBridgeMessageSender.sol index 1afcc44..df35fda 100644 --- a/src/MultiBridgeMessageSender.sol +++ b/src/MultiBridgeMessageSender.sol @@ -28,6 +28,7 @@ contract MultiBridgeMessageSender { uint256 expiration; address refundAddress; uint256[] fees; + uint256 successThreshold; address[] excludedAdapters; } @@ -149,18 +150,28 @@ contract MultiBridgeMessageSender { /// @param _expiration refers to the number of seconds that a message remains valid before it is considered stale and can no longer be executed. /// @param _refundAddress refers to the refund address for any extra native tokens paid /// @param _fees refers to the fees to pay to each adapter + /// @param _successThreshold refers to minimum number of bridges required to relay the message function remoteCall( uint256 _dstChainId, address _target, - bytes calldata _callData, + bytes memory _callData, uint256 _nativeValue, uint256 _expiration, address _refundAddress, - uint256[] calldata _fees + uint256[] memory _fees, + uint256 _successThreshold ) external payable onlyCaller validateExpiration(_expiration) { _remoteCall( RemoteCallArgs( - _dstChainId, _target, _callData, _nativeValue, _expiration, _refundAddress, _fees, new address[](0) + _dstChainId, + _target, + _callData, + _nativeValue, + _expiration, + _refundAddress, + _fees, + _successThreshold, + new address[](0) ) ); } @@ -173,20 +184,30 @@ 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 refers to minimum number of bridges required to relay the message /// @param _excludedAdapters are the sender adapters to be excluded from relaying the message, in ascending order by address function remoteCall( uint256 _dstChainId, address _target, - bytes calldata _callData, + bytes memory _callData, uint256 _nativeValue, uint256 _expiration, address _refundAddress, - uint256[] calldata _fees, - address[] calldata _excludedAdapters + uint256[] memory _fees, + 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 ) ); } @@ -284,7 +305,7 @@ contract MultiBridgeMessageSender { (bool[] memory adapterSuccess, uint256 successCount) = _dispatchMessages(adapters, mmaReceiver, _args.dstChainId, message, _args.fees); - if (successCount == 0) { + if (successCount < _args.successThreshold) { revert Error.MULTI_MESSAGE_SEND_FAILED(); } diff --git a/test/Setup.t.sol b/test/Setup.t.sol index b29d3b9..7292f3b 100644 --- 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 index be2df83..ef2bb53 100644 --- a/test/integration-tests/GracePeriodExpiry.t.sol +++ b/test/integration-tests/GracePeriodExpiry.t.sol @@ -47,7 +47,8 @@ contract GracePeriodExpiryTest is Setup { 0, EXPIRATION_CONSTANT, refundAddress, - fees + fees, + DEFAULT_SUCCESS_THRESHOLD ); Vm.Log[] memory logs = vm.getRecordedLogs(); diff --git a/test/integration-tests/MultiMessageAggregation.t.sol b/test/integration-tests/MultiMessageAggregation.t.sol index fdd0d1e..3bc19a0 100644 --- a/test/integration-tests/MultiMessageAggregation.t.sol +++ b/test/integration-tests/MultiMessageAggregation.t.sol @@ -47,7 +47,8 @@ contract MultiBridgeMessageAggregationTest is Setup { 0, EXPIRATION_CONSTANT, refundAddress, - fees + fees, + DEFAULT_SUCCESS_THRESHOLD ); Vm.Log[] memory logs = vm.getRecordedLogs(); diff --git a/test/integration-tests/RemoteAdapterAdd.t.sol b/test/integration-tests/RemoteAdapterAdd.t.sol index 7315f95..bcd0cca 100644 --- a/test/integration-tests/RemoteAdapterAdd.t.sol +++ b/test/integration-tests/RemoteAdapterAdd.t.sol @@ -72,7 +72,8 @@ contract RemoteAdapterAdd is Setup { 0, EXPIRATION_CONSTANT, refundAddress, - fees + fees, + DEFAULT_SUCCESS_THRESHOLD ); Vm.Log[] memory logs = vm.getRecordedLogs(); diff --git a/test/integration-tests/RemoteAdapterRemove.t.sol b/test/integration-tests/RemoteAdapterRemove.t.sol index 1ae0c7e..02e95b9 100644 --- a/test/integration-tests/RemoteAdapterRemove.t.sol +++ b/test/integration-tests/RemoteAdapterRemove.t.sol @@ -82,7 +82,8 @@ contract RemoteAdapterRemove is Setup { 0, EXPIRATION_CONSTANT, refundAddress, - fees + fees, + DEFAULT_SUCCESS_THRESHOLD ); Vm.Log[] memory logs = vm.getRecordedLogs(); diff --git a/test/integration-tests/RemoteSetQuorum.t.sol b/test/integration-tests/RemoteSetQuorum.t.sol index 028252c..9b22b3a 100644 --- a/test/integration-tests/RemoteSetQuorum.t.sol +++ b/test/integration-tests/RemoteSetQuorum.t.sol @@ -44,7 +44,8 @@ contract RemoteQuorumUpdate is Setup { 0, EXPIRATION_CONSTANT, refundAddress, - fees + fees, + DEFAULT_SUCCESS_THRESHOLD ); Vm.Log[] memory logs = vm.getRecordedLogs(); diff --git a/test/integration-tests/RemoteTimelockUpdate.t.sol b/test/integration-tests/RemoteTimelockUpdate.t.sol index 3ede00f..c557103 100644 --- a/test/integration-tests/RemoteTimelockUpdate.t.sol +++ b/test/integration-tests/RemoteTimelockUpdate.t.sol @@ -44,7 +44,8 @@ contract RemoteTimelockUpdate is Setup { 0, EXPIRATION_CONSTANT, refundAddress, - fees + fees, + DEFAULT_SUCCESS_THRESHOLD ); Vm.Log[] memory logs = vm.getRecordedLogs(); diff --git a/test/integration-tests/TimelockCheck.t.sol b/test/integration-tests/TimelockCheck.t.sol index 8b481bc..9487db1 100644 --- a/test/integration-tests/TimelockCheck.t.sol +++ b/test/integration-tests/TimelockCheck.t.sol @@ -49,7 +49,8 @@ contract TimelockCheckTest is Setup { 0, EXPIRATION_CONSTANT, refundAddress, - fees + fees, + DEFAULT_SUCCESS_THRESHOLD ); Vm.Log[] memory logs = vm.getRecordedLogs(); diff --git a/test/unit-tests/MultiBridgeMessageSender.t.sol b/test/unit-tests/MultiBridgeMessageSender.t.sol index 0a9b997..06c4483 100644 --- a/test/unit-tests/MultiBridgeMessageSender.t.sol +++ b/test/unit-tests/MultiBridgeMessageSender.t.sol @@ -94,7 +94,7 @@ contract MultiBridgeMessageSenderTest is Setup { ); sender.remoteCall{value: fees[0] + fees[1]}( - DST_CHAIN_ID, address(42), bytes("42"), 0, expiration, refundAddress, fees + DST_CHAIN_ID, address(42), bytes("42"), 0, expiration, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD ); assertEq(sender.nonce(), 1); @@ -116,7 +116,7 @@ contract MultiBridgeMessageSenderTest is Setup { (, uint256[] memory fees) = _sortTwoAdaptersWithFees(axelarAdapterAddr, wormholeAdapterAddr, 0.01 ether, wormholeFee); sender.remoteCall{value: nativeValue}( - DST_CHAIN_ID, address(42), bytes("42"), 0, expiration, refundAddress, fees + DST_CHAIN_ID, address(42), bytes("42"), 0, expiration, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD ); uint256 balanceAfter = refundAddress.balance; @@ -136,8 +136,6 @@ contract MultiBridgeMessageSenderTest is Setup { address[] memory excludedAdapters = new address[](1); excludedAdapters[0] = axelarAdapterAddr; - uint256 expiration = EXPIRATION_CONSTANT; - MessageLibrary.Message memory message = MessageLibrary.Message({ srcChainId: SRC_CHAIN_ID, dstChainId: DST_CHAIN_ID, @@ -145,7 +143,7 @@ contract MultiBridgeMessageSenderTest is Setup { nonce: 1, callData: bytes("42"), nativeValue: 0, - expiration: block.timestamp + expiration + expiration: block.timestamp + EXPIRATION_CONSTANT }); bytes32 msgId = MessageLibrary.computeMsgId(message); @@ -157,11 +155,19 @@ contract MultiBridgeMessageSenderTest is Setup { vm.expectEmit(true, true, true, true, address(sender)); emit MultiBridgeMessageSent( - msgId, 1, DST_CHAIN_ID, address(42), bytes("42"), 0, expiration, senderAdapters, adapterSuccess + msgId, 1, DST_CHAIN_ID, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, senderAdapters, adapterSuccess ); sender.remoteCall{value: 0.01 ether}( - DST_CHAIN_ID, address(42), bytes("42"), 0, expiration, refundAddress, fees, excludedAdapters + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + EXPIRATION_CONSTANT, + refundAddress, + fees, + DEFAULT_SUCCESS_THRESHOLD - excludedAdapters.length, + excludedAdapters ); } @@ -178,14 +184,30 @@ contract MultiBridgeMessageSenderTest is Setup { vm.expectRevert(); sender.remoteCall{value: 1 ether}( - DST_CHAIN_ID, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, refundAddress, fees, duplicateExclusions + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + EXPIRATION_CONSTANT, + refundAddress, + fees, + DEFAULT_SUCCESS_THRESHOLD, + duplicateExclusions ); address[] memory nonExistentAdapter = new address[](1); nonExistentAdapter[0] = address(42); vm.expectRevert(); sender.remoteCall{value: 1 ether}( - DST_CHAIN_ID, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, refundAddress, fees, duplicateExclusions + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + EXPIRATION_CONSTANT, + refundAddress, + fees, + DEFAULT_SUCCESS_THRESHOLD, + duplicateExclusions ); } @@ -198,7 +220,16 @@ contract MultiBridgeMessageSenderTest is Setup { fees[1] = 0.01 ether; vm.expectRevert(Error.INVALID_PRIVILEGED_CALLER.selector); - sender.remoteCall(DST_CHAIN_ID, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, refundAddress, fees); + sender.remoteCall( + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + EXPIRATION_CONSTANT, + refundAddress, + fees, + DEFAULT_SUCCESS_THRESHOLD + ); } /// @dev message expiration has to be within allowed range @@ -213,22 +244,42 @@ contract MultiBridgeMessageSenderTest is Setup { // test expiration validation in remoteCall() which does not accept excluded adapters vm.startPrank(caller); vm.expectRevert(Error.INVALID_EXPIRATION_DURATION.selector); - sender.remoteCall(DST_CHAIN_ID, address(42), bytes("42"), 0, invalidExpMin, refundAddress, fees); + sender.remoteCall( + DST_CHAIN_ID, address(42), bytes("42"), 0, invalidExpMin, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD + ); vm.expectRevert(Error.INVALID_EXPIRATION_DURATION.selector); - sender.remoteCall(DST_CHAIN_ID, address(42), bytes("42"), 0, invalidExpMax, refundAddress, fees); + sender.remoteCall( + DST_CHAIN_ID, address(42), bytes("42"), 0, invalidExpMax, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD + ); // test expiration validation in remoteCall() which accepts excluded adapters address[] memory excludedAdapters = new address[](0); vm.startPrank(caller); vm.expectRevert(Error.INVALID_EXPIRATION_DURATION.selector); sender.remoteCall( - DST_CHAIN_ID, address(42), bytes("42"), 0, invalidExpMin, refundAddress, fees, excludedAdapters + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + invalidExpMin, + refundAddress, + fees, + DEFAULT_SUCCESS_THRESHOLD, + excludedAdapters ); vm.expectRevert(Error.INVALID_EXPIRATION_DURATION.selector); sender.remoteCall( - DST_CHAIN_ID, address(42), bytes("42"), 0, invalidExpMax, refundAddress, fees, excludedAdapters + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + invalidExpMax, + refundAddress, + fees, + DEFAULT_SUCCESS_THRESHOLD, + excludedAdapters ); } @@ -241,7 +292,9 @@ contract MultiBridgeMessageSenderTest is Setup { fees[0] = 0.01 ether; fees[1] = 0.01 ether; vm.expectRevert(Error.INVALID_REFUND_ADDRESS.selector); - sender.remoteCall(DST_CHAIN_ID, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, address(0), fees); + sender.remoteCall( + DST_CHAIN_ID, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, address(0), fees, DEFAULT_SUCCESS_THRESHOLD + ); vm.expectRevert(Error.INVALID_REFUND_ADDRESS.selector); sender.remoteCall( @@ -251,7 +304,8 @@ contract MultiBridgeMessageSenderTest is Setup { 0, EXPIRATION_CONSTANT, contractAddress[SRC_CHAIN_ID]["MMA_SENDER"], - fees + fees, + DEFAULT_SUCCESS_THRESHOLD ); // test refund address validation in remoteCall() which accepts excluded adapters @@ -259,7 +313,15 @@ contract MultiBridgeMessageSenderTest is Setup { vm.startPrank(caller); vm.expectRevert(Error.INVALID_REFUND_ADDRESS.selector); sender.remoteCall( - DST_CHAIN_ID, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, address(0), fees, excludedAdapters + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + EXPIRATION_CONSTANT, + address(0), + fees, + DEFAULT_SUCCESS_THRESHOLD, + excludedAdapters ); vm.expectRevert(Error.INVALID_REFUND_ADDRESS.selector); @@ -271,6 +333,7 @@ contract MultiBridgeMessageSenderTest is Setup { EXPIRATION_CONSTANT, contractAddress[SRC_CHAIN_ID]["MMA_SENDER"], fees, + DEFAULT_SUCCESS_THRESHOLD, excludedAdapters ); } @@ -284,7 +347,16 @@ contract MultiBridgeMessageSenderTest is Setup { fees[1] = 0.01 ether; vm.expectRevert(Error.INVALID_DST_CHAIN.selector); - sender.remoteCall(block.chainid, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, refundAddress, fees); + sender.remoteCall( + block.chainid, + address(42), + bytes("42"), + 0, + EXPIRATION_CONSTANT, + refundAddress, + fees, + DEFAULT_SUCCESS_THRESHOLD + ); } /// @dev cannot call with dst chain ID of 0 @@ -296,7 +368,9 @@ contract MultiBridgeMessageSenderTest is Setup { fees[1] = 0.01 ether; vm.expectRevert(Error.ZERO_CHAIN_ID.selector); - sender.remoteCall(0, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, refundAddress, fees); + sender.remoteCall( + 0, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD + ); } /// @dev cannot call with target address of 0 @@ -308,7 +382,16 @@ contract MultiBridgeMessageSenderTest is Setup { fees[1] = 0.01 ether; vm.expectRevert(Error.INVALID_TARGET.selector); - sender.remoteCall(DST_CHAIN_ID, address(0), bytes("42"), 0, EXPIRATION_CONSTANT, refundAddress, fees); + sender.remoteCall( + DST_CHAIN_ID, + address(0), + bytes("42"), + 0, + EXPIRATION_CONSTANT, + refundAddress, + fees, + DEFAULT_SUCCESS_THRESHOLD + ); } /// @dev cannot call with receiver address of 0 @@ -322,7 +405,16 @@ contract MultiBridgeMessageSenderTest is Setup { MultiBridgeMessageSender dummySender = new MultiBridgeMessageSender(address(new ZeroAddressReceiverGAC(caller))); vm.expectRevert(Error.ZERO_RECEIVER_ADAPTER.selector); - dummySender.remoteCall(DST_CHAIN_ID, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, refundAddress, fees); + dummySender.remoteCall( + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + EXPIRATION_CONSTANT, + refundAddress, + fees, + DEFAULT_SUCCESS_THRESHOLD + ); } /// @dev cannot call with no sender adapter @@ -341,7 +433,16 @@ contract MultiBridgeMessageSenderTest is Setup { vm.startPrank(caller); vm.expectRevert(Error.NO_SENDER_ADAPTER_FOUND.selector); - sender.remoteCall(DST_CHAIN_ID, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, refundAddress, fees); + sender.remoteCall( + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + EXPIRATION_CONSTANT, + refundAddress, + fees, + DEFAULT_SUCCESS_THRESHOLD + ); } /// @dev should proceed with the call despite one failing adapter, emitting an error message @@ -394,7 +495,9 @@ contract MultiBridgeMessageSenderTest is Setup { msgId, 1, DST_CHAIN_ID, address(42), bytes("42"), 0, expiration, senderAdapters, adapterSuccess ); - sender.remoteCall{value: 0.1 ether}(DST_CHAIN_ID, address(42), bytes("42"), 0, expiration, refundAddress, fees); + sender.remoteCall{value: 0.1 ether}( + DST_CHAIN_ID, address(42), bytes("42"), 0, expiration, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD + ); } /// @dev cannot call with invalid fee array @@ -405,7 +508,16 @@ contract MultiBridgeMessageSenderTest is Setup { fees[0] = 0.01 ether; vm.expectRevert(Error.INVALID_SENDER_ADAPTER_FEES.selector); - sender.remoteCall(DST_CHAIN_ID, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, refundAddress, fees); + sender.remoteCall( + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + EXPIRATION_CONSTANT, + refundAddress, + fees, + DEFAULT_SUCCESS_THRESHOLD + ); } /// @dev cannot call with msg.value less than total fees @@ -417,7 +529,16 @@ contract MultiBridgeMessageSenderTest is Setup { fees[1] = 0.01 ether; vm.expectRevert(Error.INVALID_MSG_VALUE.selector); - sender.remoteCall(DST_CHAIN_ID, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, refundAddress, fees); + sender.remoteCall( + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + EXPIRATION_CONSTANT, + refundAddress, + fees, + DEFAULT_SUCCESS_THRESHOLD + ); } /// @dev adds two sender adapters @@ -616,7 +737,14 @@ contract MultiBridgeMessageSenderTest is Setup { vm.expectRevert(Error.MULTI_MESSAGE_SEND_FAILED.selector); sender.remoteCall( - DST_CHAIN_ID, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, refundAddress, new uint256[](2) + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + EXPIRATION_CONSTANT, + refundAddress, + new uint256[](2), + DEFAULT_SUCCESS_THRESHOLD ); } } From 5bbd74d4d1c99164a9851789a26371a9fc5a2c8b Mon Sep 17 00:00:00 2001 From: Sujith Date: Thu, 5 Oct 2023 20:35:50 -0400 Subject: [PATCH 4/7] chore: remove old functions --- src/MultiBridgeMessageSender.sol | 38 --------------- .../integration-tests/GracePeriodExpiry.t.sol | 3 +- .../MultiMessageAggregation.t.sol | 3 +- test/integration-tests/RemoteAdapterAdd.t.sol | 3 +- .../RemoteAdapterRemove.t.sol | 3 +- test/integration-tests/RemoteSetQuorum.t.sol | 3 +- .../RemoteTimelockUpdate.t.sol | 3 +- test/integration-tests/TimelockCheck.t.sol | 3 +- .../unit-tests/MultiBridgeMessageSender.t.sol | 48 ++++++++++++------- 9 files changed, 46 insertions(+), 61 deletions(-) diff --git a/src/MultiBridgeMessageSender.sol b/src/MultiBridgeMessageSender.sol index 69e8110..5e6097f 100644 --- a/src/MultiBridgeMessageSender.sol +++ b/src/MultiBridgeMessageSender.sol @@ -138,44 +138,6 @@ contract MultiBridgeMessageSender { EXTERNAL FUNCTIONS ////////////////////////////////////////////////////////////////*/ - /// @notice Call a remote function on a destination chain by sending multiple copies of a cross-chain message - /// via all available bridges. This function can only be called by the authorised called configured in the GAC. - /// @dev a fee in native token may be required by each message bridge to send messages. Any native token fee remained - /// will be refunded back to a refund address defined in the _refundAddress parameter. - /// Caller needs to specify fees for each adapter when calling this function. - /// @param _dstChainId is the destination chainId - /// @param _target is the target execution point on the destination chain - /// @param _callData is the data to be sent to _target by low-level call(eg. address(_target).call(_callData)) - /// @param _nativeValue is the value to be sent to _target by low-level call (eg. address(_target).call{value: _nativeValue}(_callData)) - /// @param _expiration refers to the number of seconds that a message remains valid before it is considered stale and can no longer be executed. - /// @param _refundAddress refers to the refund address for any extra native tokens paid - /// @param _fees refers to the fees to pay to each adapter - /// @param _successThreshold refers to minimum number of bridges required to relay the message - function remoteCall( - uint256 _dstChainId, - address _target, - bytes memory _callData, - uint256 _nativeValue, - uint256 _expiration, - address _refundAddress, - uint256[] memory _fees, - uint256 _successThreshold - ) external payable onlyCaller validateExpiration(_expiration) { - _remoteCall( - RemoteCallArgs( - _dstChainId, - _target, - _callData, - _nativeValue, - _expiration, - _refundAddress, - _fees, - _successThreshold, - new address[](0) - ) - ); - } - /// @param _dstChainId is the destination chainId /// @param _target is the target execution point on the destination chain /// @param _callData is the data to be sent to _target by low-level call(eg. address(_target).call(_callData)) diff --git a/test/integration-tests/GracePeriodExpiry.t.sol b/test/integration-tests/GracePeriodExpiry.t.sol index ef2bb53..6f9e14a 100644 --- a/test/integration-tests/GracePeriodExpiry.t.sol +++ b/test/integration-tests/GracePeriodExpiry.t.sol @@ -48,7 +48,8 @@ contract GracePeriodExpiryTest is Setup { EXPIRATION_CONSTANT, refundAddress, fees, - DEFAULT_SUCCESS_THRESHOLD + DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); Vm.Log[] memory logs = vm.getRecordedLogs(); diff --git a/test/integration-tests/MultiMessageAggregation.t.sol b/test/integration-tests/MultiMessageAggregation.t.sol index 3bc19a0..ce83c2f 100644 --- a/test/integration-tests/MultiMessageAggregation.t.sol +++ b/test/integration-tests/MultiMessageAggregation.t.sol @@ -48,7 +48,8 @@ contract MultiBridgeMessageAggregationTest is Setup { EXPIRATION_CONSTANT, refundAddress, fees, - DEFAULT_SUCCESS_THRESHOLD + DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); Vm.Log[] memory logs = vm.getRecordedLogs(); diff --git a/test/integration-tests/RemoteAdapterAdd.t.sol b/test/integration-tests/RemoteAdapterAdd.t.sol index bcd0cca..87c6309 100644 --- a/test/integration-tests/RemoteAdapterAdd.t.sol +++ b/test/integration-tests/RemoteAdapterAdd.t.sol @@ -73,7 +73,8 @@ contract RemoteAdapterAdd is Setup { EXPIRATION_CONSTANT, refundAddress, fees, - DEFAULT_SUCCESS_THRESHOLD + DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); Vm.Log[] memory logs = vm.getRecordedLogs(); diff --git a/test/integration-tests/RemoteAdapterRemove.t.sol b/test/integration-tests/RemoteAdapterRemove.t.sol index 02e95b9..8631240 100644 --- a/test/integration-tests/RemoteAdapterRemove.t.sol +++ b/test/integration-tests/RemoteAdapterRemove.t.sol @@ -83,7 +83,8 @@ contract RemoteAdapterRemove is Setup { EXPIRATION_CONSTANT, refundAddress, fees, - DEFAULT_SUCCESS_THRESHOLD + DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); Vm.Log[] memory logs = vm.getRecordedLogs(); diff --git a/test/integration-tests/RemoteSetQuorum.t.sol b/test/integration-tests/RemoteSetQuorum.t.sol index 9b22b3a..5a239a2 100644 --- a/test/integration-tests/RemoteSetQuorum.t.sol +++ b/test/integration-tests/RemoteSetQuorum.t.sol @@ -45,7 +45,8 @@ contract RemoteQuorumUpdate is Setup { EXPIRATION_CONSTANT, refundAddress, fees, - DEFAULT_SUCCESS_THRESHOLD + DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); Vm.Log[] memory logs = vm.getRecordedLogs(); diff --git a/test/integration-tests/RemoteTimelockUpdate.t.sol b/test/integration-tests/RemoteTimelockUpdate.t.sol index c557103..731de41 100644 --- a/test/integration-tests/RemoteTimelockUpdate.t.sol +++ b/test/integration-tests/RemoteTimelockUpdate.t.sol @@ -45,7 +45,8 @@ contract RemoteTimelockUpdate is Setup { EXPIRATION_CONSTANT, refundAddress, fees, - DEFAULT_SUCCESS_THRESHOLD + DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); Vm.Log[] memory logs = vm.getRecordedLogs(); diff --git a/test/integration-tests/TimelockCheck.t.sol b/test/integration-tests/TimelockCheck.t.sol index 9487db1..8a7265d 100644 --- a/test/integration-tests/TimelockCheck.t.sol +++ b/test/integration-tests/TimelockCheck.t.sol @@ -50,7 +50,8 @@ contract TimelockCheckTest is Setup { EXPIRATION_CONSTANT, refundAddress, fees, - DEFAULT_SUCCESS_THRESHOLD + DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); Vm.Log[] memory logs = vm.getRecordedLogs(); diff --git a/test/unit-tests/MultiBridgeMessageSender.t.sol b/test/unit-tests/MultiBridgeMessageSender.t.sol index 1d2a77a..5dad296 100644 --- a/test/unit-tests/MultiBridgeMessageSender.t.sol +++ b/test/unit-tests/MultiBridgeMessageSender.t.sol @@ -94,7 +94,8 @@ contract MultiBridgeMessageSenderTest is Setup { ); sender.remoteCall{value: fees[0] + fees[1]}( - DST_CHAIN_ID, address(42), bytes("42"), 0, expiration, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD + DST_CHAIN_ID, address(42), bytes("42"), 0, expiration, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); assertEq(sender.nonce(), 1); @@ -116,7 +117,8 @@ contract MultiBridgeMessageSenderTest is Setup { (, uint256[] memory fees) = _sortTwoAdaptersWithFees(axelarAdapterAddr, wormholeAdapterAddr, 0.01 ether, wormholeFee); sender.remoteCall{value: nativeValue}( - DST_CHAIN_ID, address(42), bytes("42"), 0, expiration, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD + DST_CHAIN_ID, address(42), bytes("42"), 0, expiration, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); uint256 balanceAfter = refundAddress.balance; @@ -228,7 +230,8 @@ contract MultiBridgeMessageSenderTest is Setup { EXPIRATION_CONSTANT, refundAddress, fees, - DEFAULT_SUCCESS_THRESHOLD + DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); } @@ -246,12 +249,14 @@ contract MultiBridgeMessageSenderTest is Setup { vm.startPrank(caller); vm.expectRevert(Error.INVALID_EXPIRATION_DURATION.selector); sender.remoteCall( - DST_CHAIN_ID, address(42), bytes("42"), 0, invalidExpMin, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD + DST_CHAIN_ID, address(42), bytes("42"), 0, invalidExpMin, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); vm.expectRevert(Error.INVALID_EXPIRATION_DURATION.selector); sender.remoteCall( - DST_CHAIN_ID, address(42), bytes("42"), 0, invalidExpMax, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD + DST_CHAIN_ID, address(42), bytes("42"), 0, invalidExpMax, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); // test expiration validation in remoteCall() which accepts excluded adapters @@ -294,7 +299,8 @@ contract MultiBridgeMessageSenderTest is Setup { fees[1] = 0.01 ether; vm.expectRevert(Error.INVALID_REFUND_ADDRESS.selector); sender.remoteCall( - DST_CHAIN_ID, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, address(0), fees, DEFAULT_SUCCESS_THRESHOLD + DST_CHAIN_ID, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, address(0), fees, DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); vm.expectRevert(Error.INVALID_REFUND_ADDRESS.selector); @@ -306,7 +312,8 @@ contract MultiBridgeMessageSenderTest is Setup { EXPIRATION_CONSTANT, contractAddress[SRC_CHAIN_ID]["MMA_SENDER"], fees, - DEFAULT_SUCCESS_THRESHOLD + DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); // test refund address validation in remoteCall() which accepts excluded adapters @@ -355,7 +362,8 @@ contract MultiBridgeMessageSenderTest is Setup { EXPIRATION_CONSTANT, refundAddress, fees, - DEFAULT_SUCCESS_THRESHOLD + DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); } @@ -369,7 +377,8 @@ contract MultiBridgeMessageSenderTest is Setup { vm.expectRevert(Error.ZERO_CHAIN_ID.selector); sender.remoteCall( - 0, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD + 0, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); } @@ -390,7 +399,8 @@ contract MultiBridgeMessageSenderTest is Setup { EXPIRATION_CONSTANT, refundAddress, fees, - DEFAULT_SUCCESS_THRESHOLD + DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); } @@ -413,7 +423,8 @@ contract MultiBridgeMessageSenderTest is Setup { EXPIRATION_CONSTANT, refundAddress, fees, - DEFAULT_SUCCESS_THRESHOLD + DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); } @@ -441,7 +452,8 @@ contract MultiBridgeMessageSenderTest is Setup { EXPIRATION_CONSTANT, refundAddress, fees, - DEFAULT_SUCCESS_THRESHOLD + DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); } @@ -496,7 +508,8 @@ contract MultiBridgeMessageSenderTest is Setup { ); sender.remoteCall{value: 0.1 ether}( - DST_CHAIN_ID, address(42), bytes("42"), 0, expiration, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD + DST_CHAIN_ID, address(42), bytes("42"), 0, expiration, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); } @@ -516,7 +529,8 @@ contract MultiBridgeMessageSenderTest is Setup { EXPIRATION_CONSTANT, refundAddress, fees, - DEFAULT_SUCCESS_THRESHOLD + DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); } @@ -537,7 +551,8 @@ contract MultiBridgeMessageSenderTest is Setup { EXPIRATION_CONSTANT, refundAddress, fees, - DEFAULT_SUCCESS_THRESHOLD + DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); } @@ -744,7 +759,8 @@ contract MultiBridgeMessageSenderTest is Setup { EXPIRATION_CONSTANT, refundAddress, new uint256[](2), - DEFAULT_SUCCESS_THRESHOLD + DEFAULT_SUCCESS_THRESHOLD, + new address[](0) ); } } From b4df6eddef45fc1d0c4f9ac0167c3cd0700ac050 Mon Sep 17 00:00:00 2001 From: Sujith Date: Fri, 6 Oct 2023 22:16:54 -0400 Subject: [PATCH 5/7] fix: pr comments --- src/MultiBridgeMessageSender.sol | 15 +++- src/libraries/Error.sol | 2 +- .../unit-tests/MultiBridgeMessageSender.t.sol | 75 +++++++++++++++---- 3 files changed, 76 insertions(+), 16 deletions(-) diff --git a/src/MultiBridgeMessageSender.sol b/src/MultiBridgeMessageSender.sol index 5e6097f..0556fac 100644 --- a/src/MultiBridgeMessageSender.sol +++ b/src/MultiBridgeMessageSender.sol @@ -146,7 +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 refers to minimum number of bridges required to relay the message + /// @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, @@ -248,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 @@ -293,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) { @@ -323,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(); } diff --git a/src/libraries/Error.sol b/src/libraries/Error.sol index 987f716..83e87c5 100644 --- a/src/libraries/Error.sol +++ b/src/libraries/Error.sol @@ -102,7 +102,7 @@ library Error { /// @dev is thrown if contract call is invalid (for axelar) error NOT_APPROVED_BY_GATEWAY(); - /// @dev is thrown if all message bridges fail + /// @dev is thrown if a message could not be sent through a sufficient number of bridges error MULTI_MESSAGE_SEND_FAILED(); /*///////////////////////////////////////////////////////////////// diff --git a/test/unit-tests/MultiBridgeMessageSender.t.sol b/test/unit-tests/MultiBridgeMessageSender.t.sol index 5dad296..3500bf3 100644 --- a/test/unit-tests/MultiBridgeMessageSender.t.sol +++ b/test/unit-tests/MultiBridgeMessageSender.t.sol @@ -94,7 +94,14 @@ contract MultiBridgeMessageSenderTest is Setup { ); sender.remoteCall{value: fees[0] + fees[1]}( - DST_CHAIN_ID, address(42), bytes("42"), 0, expiration, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD, + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + expiration, + refundAddress, + fees, + DEFAULT_SUCCESS_THRESHOLD, new address[](0) ); @@ -117,7 +124,14 @@ contract MultiBridgeMessageSenderTest is Setup { (, uint256[] memory fees) = _sortTwoAdaptersWithFees(axelarAdapterAddr, wormholeAdapterAddr, 0.01 ether, wormholeFee); sender.remoteCall{value: nativeValue}( - DST_CHAIN_ID, address(42), bytes("42"), 0, expiration, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD, + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + expiration, + refundAddress, + fees, + DEFAULT_SUCCESS_THRESHOLD, new address[](0) ); @@ -249,14 +263,28 @@ contract MultiBridgeMessageSenderTest is Setup { vm.startPrank(caller); vm.expectRevert(Error.INVALID_EXPIRATION_DURATION.selector); sender.remoteCall( - DST_CHAIN_ID, address(42), bytes("42"), 0, invalidExpMin, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD, - new address[](0) + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + invalidExpMin, + refundAddress, + fees, + DEFAULT_SUCCESS_THRESHOLD, + excludedAdapters ); vm.expectRevert(Error.INVALID_EXPIRATION_DURATION.selector); sender.remoteCall( - DST_CHAIN_ID, address(42), bytes("42"), 0, invalidExpMax, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD, - new address[](0) + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + invalidExpMax, + refundAddress, + fees, + DEFAULT_SUCCESS_THRESHOLD, + excludedAdapters ); // test expiration validation in remoteCall() which accepts excluded adapters @@ -299,8 +327,15 @@ contract MultiBridgeMessageSenderTest is Setup { fees[1] = 0.01 ether; vm.expectRevert(Error.INVALID_REFUND_ADDRESS.selector); sender.remoteCall( - DST_CHAIN_ID, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, address(0), fees, DEFAULT_SUCCESS_THRESHOLD, - new address[](0) + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + EXPIRATION_CONSTANT, + address(0), + fees, + DEFAULT_SUCCESS_THRESHOLD, + excludedAdapters ); vm.expectRevert(Error.INVALID_REFUND_ADDRESS.selector); @@ -313,7 +348,7 @@ contract MultiBridgeMessageSenderTest is Setup { contractAddress[SRC_CHAIN_ID]["MMA_SENDER"], fees, DEFAULT_SUCCESS_THRESHOLD, - new address[](0) + excludedAdapters ); // test refund address validation in remoteCall() which accepts excluded adapters @@ -377,7 +412,14 @@ contract MultiBridgeMessageSenderTest is Setup { vm.expectRevert(Error.ZERO_CHAIN_ID.selector); sender.remoteCall( - 0, address(42), bytes("42"), 0, EXPIRATION_CONSTANT, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD, + 0, + address(42), + bytes("42"), + 0, + EXPIRATION_CONSTANT, + refundAddress, + fees, + DEFAULT_SUCCESS_THRESHOLD, new address[](0) ); } @@ -508,7 +550,14 @@ contract MultiBridgeMessageSenderTest is Setup { ); sender.remoteCall{value: 0.1 ether}( - DST_CHAIN_ID, address(42), bytes("42"), 0, expiration, refundAddress, fees, DEFAULT_SUCCESS_THRESHOLD, + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + expiration, + refundAddress, + fees, + DEFAULT_SUCCESS_THRESHOLD, new address[](0) ); } @@ -746,8 +795,8 @@ contract MultiBridgeMessageSenderTest is Setup { sender.removeSenderAdapters(adapters); } - /// @dev if all message bridges fail (not pay enough fees) - function test_all_MULTI_MESSAGE_SEND_FAILED() public { + /// @dev if the message could not be sent through a sufficient number of bridge + function test_revert_for_insufficient_number_of_bridges() public { vm.startPrank(caller); vm.expectRevert(Error.MULTI_MESSAGE_SEND_FAILED.selector); From 0a3501ca68e1353dc7975c33fe66e452a34fb4fa Mon Sep 17 00:00:00 2001 From: Sujith Date: Sun, 8 Oct 2023 21:04:50 -0400 Subject: [PATCH 6/7] chore: add more tests --- src/MultiBridgeMessageSender.sol | 4 ++-- .../unit-tests/MultiBridgeMessageSender.t.sol | 20 +++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/MultiBridgeMessageSender.sol b/src/MultiBridgeMessageSender.sol index 0556fac..6a3351c 100644 --- a/src/MultiBridgeMessageSender.sol +++ b/src/MultiBridgeMessageSender.sol @@ -151,11 +151,11 @@ contract MultiBridgeMessageSender { function remoteCall( uint256 _dstChainId, address _target, - bytes memory _callData, + bytes calldata _callData, uint256 _nativeValue, uint256 _expiration, address _refundAddress, - uint256[] memory _fees, + uint256[] calldata _fees, uint256 _successThreshold, address[] memory _excludedAdapters ) external payable onlyCaller validateExpiration(_expiration) { diff --git a/test/unit-tests/MultiBridgeMessageSender.t.sol b/test/unit-tests/MultiBridgeMessageSender.t.sol index 3500bf3..f25daf4 100644 --- a/test/unit-tests/MultiBridgeMessageSender.t.sol +++ b/test/unit-tests/MultiBridgeMessageSender.t.sol @@ -812,4 +812,24 @@ contract MultiBridgeMessageSenderTest is Setup { new address[](0) ); } + + function test_revert_for_invalid_success_threshold() public { + vm.startPrank(caller); + + uint256 nativeValue = 2 ether; + uint256 invalidSuccessThrehsold = 10; + + vm.expectRevert(Error.MULTI_MESSAGE_SEND_FAILED.selector); + sender.remoteCall{value: nativeValue}( + DST_CHAIN_ID, + address(42), + bytes("42"), + 0, + EXPIRATION_CONSTANT, + refundAddress, + new uint256[](2), + invalidSuccessThrehsold, + new address[](0) + ); + } } From 2156819908d8810d39118371c5eaf7162b2d5228 Mon Sep 17 00:00:00 2001 From: Sujith Date: Sun, 8 Oct 2023 21:15:38 -0400 Subject: [PATCH 7/7] chore: add linter --- test/unit-tests/MultiBridgeMessageSender.t.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit-tests/MultiBridgeMessageSender.t.sol b/test/unit-tests/MultiBridgeMessageSender.t.sol index f25daf4..5e4fa4e 100644 --- a/test/unit-tests/MultiBridgeMessageSender.t.sol +++ b/test/unit-tests/MultiBridgeMessageSender.t.sol @@ -813,12 +813,12 @@ contract MultiBridgeMessageSenderTest is Setup { ); } - function test_revert_for_invalid_success_threshold() public { + function test_revert_for_invalid_success_threshold() public { vm.startPrank(caller); uint256 nativeValue = 2 ether; uint256 invalidSuccessThrehsold = 10; - + vm.expectRevert(Error.MULTI_MESSAGE_SEND_FAILED.selector); sender.remoteCall{value: nativeValue}( DST_CHAIN_ID,