Skip to content

Commit

Permalink
Rename types and errors for clarity
Browse files Browse the repository at this point in the history
  • Loading branch information
@ermyas
ermyas committed Sep 17, 2023
1 parent 9d5c9f0 commit b321989
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 69 deletions.
26 changes: 13 additions & 13 deletions src/MultiMessageReceiver.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ

/// @notice stores each msg id related info
mapping(bytes32 => bool) public isExecuted;
mapping(bytes32 => ExecutionData) public msgReceived;
mapping(bytes32 => mapping(address => bool)) public isDuplicateAdapter;
mapping(bytes32 => uint256) public messageVotes;
mapping(bytes32 => ExecutionData) public msgExecData;
mapping(bytes32 => mapping(address => bool)) public msgDeliveries;
mapping(bytes32 => uint256) public msgDeliveryCount;

/*/////////////////////////////////////////////////////////////////
MODIFIERS
Expand Down Expand Up @@ -108,25 +108,25 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ
/// although each adapters' internal msgId is attached at the end of calldata, it's not useful to MultiMessageReceiver.
bytes32 msgId = MessageLibrary.computeMsgId(_message);

if (isDuplicateAdapter[msgId][msg.sender]) {
if (msgDeliveries[msgId][msg.sender]) {
revert Error.DUPLICATE_MESSAGE_DELIVERY_BY_ADAPTER();
}

if (isExecuted[msgId]) {
revert Error.MSG_ID_ALREADY_EXECUTED();
}

isDuplicateAdapter[msgId][msg.sender] = true;
msgDeliveries[msgId][msg.sender] = true;

/// increment quorum
++messageVotes[msgId];
++msgDeliveryCount[msgId];

/// stores the execution data required
ExecutionData memory prevStored = msgReceived[msgId];
ExecutionData memory prevStored = msgExecData[msgId];

/// stores the message if the amb is the first one delivering the message
if (prevStored.target == address(0)) {
msgReceived[msgId] = ExecutionData(
msgExecData[msgId] = ExecutionData(
_message.target, _message.callData, _message.nativeValue, _message.nonce, _message.expiration
);
}
Expand All @@ -138,7 +138,7 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ
/// @dev has reached the power threshold (the same message has been delivered by enough multiple bridges).
/// Param values can be found in the MultiMessageMsgSent event from the source chain MultiMessageSender contract.
function executeMessage(bytes32 msgId) external {
ExecutionData memory _execData = msgReceived[msgId];
ExecutionData memory _execData = msgExecData[msgId];

/// @dev validates if msg execution is not beyond expiration
if (block.timestamp > _execData.expiration) {
Expand All @@ -153,8 +153,8 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ
isExecuted[msgId] = true;

/// @dev validates message quorum
if (messageVotes[msgId] < quorum) {
revert Error.INVALID_QUORUM_FOR_EXECUTION();
if (msgDeliveryCount[msgId] < quorum) {
revert Error.QUORUM_NOT_ACHIEVED();
}

/// @dev queues the action on timelock for execution
Expand Down Expand Up @@ -199,13 +199,13 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ

/// @notice view message info, return (executed, msgPower, delivered adapters)
function getMessageInfo(bytes32 msgId) public view returns (bool, uint256, string[] memory) {
uint256 msgCurrentVotes = messageVotes[msgId];
uint256 msgCurrentVotes = msgDeliveryCount[msgId];
string[] memory successfulBridge = new string[](msgCurrentVotes);

if (msgCurrentVotes != 0) {
uint256 currIndex;
for (uint256 i; i < trustedExecutor.length;) {
if (isDuplicateAdapter[msgId][trustedExecutor[i]]) {
if (msgDeliveries[msgId][trustedExecutor[i]]) {
successfulBridge[currIndex] = IMessageReceiverAdapter(trustedExecutor[i]).name();
++currIndex;
}
Expand Down
76 changes: 38 additions & 38 deletions src/MultiMessageSender.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import "./interfaces/IGAC.sol";
/// libraries
import "./libraries/Message.sol";
import "./libraries/Error.sol";
import "forge-std/console.sol";

/// @title MultiMessageSender
/// @dev handles the routing of message from external sender to bridge adapters
Expand All @@ -19,8 +20,8 @@ contract MultiMessageSender {
//////////////////////////////////////////////////////////////*/
IGAC public immutable gac;

uint256 public constant MINIMUM_EXPIRATION = 2 days;
uint256 public constant MAXIMUM_EXPIRATION = 30 days;
uint256 public constant MIN_EXPIRATION = 2 days;
uint256 public constant MAX_EXPIRATION = 30 days;

/*/////////////////////////////////////////////////////////////////
STATE VARIABLES
Expand Down Expand Up @@ -78,13 +79,10 @@ contract MultiMessageSender {

/// @dev validates the expiration provided by the user
modifier validateExpiration(uint256 _expiration) {
if (_expiration < MINIMUM_EXPIRATION) {
revert Error.INVALID_EXPIRATION_MIN();
if (_expiration < MIN_EXPIRATION || _expiration > MAX_EXPIRATION) {
revert Error.INVALID_EXPIRATION_DURATION();
}

if (_expiration > MAXIMUM_EXPIRATION) {
revert Error.INVALID_EXPIRATION_MAX();
}
_;
}

Expand Down Expand Up @@ -146,10 +144,10 @@ contract MultiMessageSender {
}

/// @notice Add bridge sender adapters
/// @param _senderAdapters is the adapter address to add
function addSenderAdapters(address[] calldata _senderAdapters) external onlyOwner {
for (uint256 i; i < _senderAdapters.length;) {
_addSenderAdapter(_senderAdapters[i]);
/// @param _adapters is the adapter address to add
function addSenderAdapters(address[] calldata _adapters) external onlyOwner {
for (uint256 i; i < _adapters.length;) {
_addSenderAdapter(_adapters[i]);

unchecked {
++i;
Expand All @@ -158,10 +156,10 @@ contract MultiMessageSender {
}

/// @notice Remove bridge sender adapters
/// @param _senderAdapters is the adapter address to remove
function removeSenderAdapters(address[] calldata _senderAdapters) external onlyOwner {
for (uint256 i; i < _senderAdapters.length;) {
_removeSenderAdapter(_senderAdapters[i]);
/// @param _adapters is the adapter address to remove
function removeSenderAdapters(address[] calldata _adapters) external onlyOwner {
for (uint256 i; i < _adapters.length;) {
_removeSenderAdapter(_adapters[i]);

unchecked {
++i;
Expand Down Expand Up @@ -236,10 +234,9 @@ contract MultiMessageSender {
);
bytes32 msgId = MessageLibrary.computeMsgId(message);

address[] memory adapters = _getSenderAdapters(_excludedAdapters);
uint256 adapterLength = adapters.length;
(address[] memory adapters, uint256 adaptersCount) = _getSenderAdapters(_excludedAdapters);

if (adapterLength == 0) {
if (adaptersCount == 0) {
revert Error.NO_SENDER_ADAPTER_FOUND();
}

Expand All @@ -249,36 +246,35 @@ contract MultiMessageSender {
);

/// refund remaining fee
/// FIXME: add an explicit refund address config
if (address(this).balance > 0) {
_safeTransferETH(gac.getRefundAddress(), address(this).balance);
}
}

function _addSenderAdapter(address _senderAdapter) private {
if (_senderAdapter == address(0)) {
function _addSenderAdapter(address _adapter) private {
if (_adapter == address(0)) {
revert Error.ZERO_ADDRESS_INPUT();
}

/// @dev reverts if it finds a duplicate
_checkDuplicates(_senderAdapter);
_checkDuplicates(_adapter);

senderAdapters.push(_senderAdapter);
emit SenderAdapterUpdated(_senderAdapter, true);
senderAdapters.push(_adapter);
emit SenderAdapterUpdated(_adapter, true);
}

function _removeSenderAdapter(address _senderAdapter) private {
function _removeSenderAdapter(address _adapter) private {
uint256 lastIndex = senderAdapters.length - 1;

for (uint256 i; i < senderAdapters.length; ++i) {
if (senderAdapters[i] == _senderAdapter) {
if (senderAdapters[i] == _adapter) {
if (i < lastIndex) {
senderAdapters[i] = senderAdapters[lastIndex];
}

senderAdapters.pop();

emit SenderAdapterUpdated(_senderAdapter, false);
emit SenderAdapterUpdated(_adapter, false);
return;
}
}
Expand All @@ -292,8 +288,10 @@ contract MultiMessageSender {
) internal returns (bool[] memory) {
uint256 len = _adapters.length;
bool[] memory successes = new bool[](len);
console.log("Dispatching Messages");
for (uint256 i; i < len;) {
IMessageSenderAdapter bridgeAdapter = IMessageSenderAdapter(_adapters[i]);
console.log("Dispatching Message: ", _adapters[i]);

/// @dev assumes CREATE2 deployment for mma sender & receiver
uint256 fee = bridgeAdapter.getMessageFee(_dstChainId, _mmaReceiver, abi.encode(_message));
Expand All @@ -315,12 +313,15 @@ contract MultiMessageSender {
return successes;
}

function _getSenderAdapters(address[] memory _exclusions) internal view returns (address[] memory) {
address[] memory targetAdapters = new address[](senderAdapters.length - _exclusions.length);
uint256 len;
for (uint256 i; i < senderAdapters.length;) {
function _getSenderAdapters(address[] memory _exclusions) internal view returns (address[] memory, uint256) {
uint256 allLen = senderAdapters.length;
uint256 exclLen = _exclusions.length;

address[] memory inclAdapters = new address[](allLen - exclLen);
uint256 inclAdaptersLen;
for (uint256 i; i < allLen;) {
bool excluded = false;
for (uint256 j; j < _exclusions.length;) {
for (uint256 j; j < exclLen;) {
if (senderAdapters[i] == _exclusions[j]) {
excluded = true;
break;
Expand All @@ -330,23 +331,22 @@ contract MultiMessageSender {
}
}
if (!excluded) {
targetAdapters[len] = senderAdapters[i];
++len;
inclAdapters[inclAdaptersLen++] = senderAdapters[i];
}
unchecked {
++i;
}
}
return targetAdapters;
return (inclAdapters, inclAdaptersLen);
}

/// @dev validates if the sender adapter already exists
/// @param _senderAdapter is the address of the sender to check
function _checkDuplicates(address _senderAdapter) internal view {
/// @param _adapter is the address of the sender to check
function _checkDuplicates(address _adapter) internal view {
uint256 len = senderAdapters.length;

for (uint256 i; i < len;) {
if (senderAdapters[i] == _senderAdapter) {
if (senderAdapters[i] == _adapter) {
revert Error.DUPLICATE_SENDER_ADAPTER();
}

Expand Down
9 changes: 3 additions & 6 deletions src/libraries/Error.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ library Error {
error MSG_EXECUTION_PASSED_DEADLINE();

/// @dev is thrown if quorum is not reached
error INVALID_QUORUM_FOR_EXECUTION();
error QUORUM_NOT_ACHIEVED();

/// @dev is thrown if message execution fails on the destination chain
error EXECUTION_FAILS_ON_DST();
Expand All @@ -53,11 +53,8 @@ library Error {
/// @dev is thrown if caller is not admin of timelock
error CALLER_NOT_ADMIN();

/// @dev is thrown if the expiration is less than minimum expiration
error INVALID_EXPIRATION_MIN();

/// @dev is thrown if the delay is more than maximum delay
error INVALID_EXPIRATION_MAX();
/// @dev is thrown if the expiration is outside a defined range
error INVALID_EXPIRATION_DURATION();

/*/////////////////////////////////////////////////////////////////
ADAPTER ERRORS
Expand Down
12 changes: 6 additions & 6 deletions test/unit-tests/MultiMessageReceiver.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,12 +172,12 @@ contract MultiMessageReceiverTest is Setup {

assertFalse(receiver.isExecuted(msgId));

assertTrue(receiver.isDuplicateAdapter(msgId, wormholeAdapterAddr));
assertTrue(receiver.msgDeliveries(msgId, wormholeAdapterAddr));

assertEq(receiver.messageVotes(msgId), 1);
assertEq(receiver.msgDeliveryCount(msgId), 1);

(address target, bytes memory callData, uint256 nativeValue, uint256 nonce, uint256 expiration) =
receiver.msgReceived(msgId);
receiver.msgExecData(msgId);
assertEq(target, message.target);
assertEq(callData, message.callData);
assertEq(nativeValue, message.nativeValue);
Expand Down Expand Up @@ -205,7 +205,7 @@ contract MultiMessageReceiverTest is Setup {
vm.startPrank(axelarAdapterAddr);
receiver.receiveMessage(message, "AXELAR");

assertEq(receiver.messageVotes(msgId), 2);
assertEq(receiver.msgDeliveryCount(msgId), 2);
}

/// @dev only adapters can call
Expand Down Expand Up @@ -407,7 +407,7 @@ contract MultiMessageReceiverTest is Setup {
}

/// @dev cannot execute message without quorum
function test_execute_message_invalid_quorum_for_execution() public {
function test_execute_message_quorum_not_met_for_exec() public {
vm.startPrank(wormholeAdapterAddr);

MessageLibrary.Message memory message = MessageLibrary.Message({
Expand All @@ -423,7 +423,7 @@ contract MultiMessageReceiverTest is Setup {

receiver.receiveMessage(message, "WORMHOLE");

vm.expectRevert(Error.INVALID_QUORUM_FOR_EXECUTION.selector);
vm.expectRevert(Error.QUORUM_NOT_ACHIEVED.selector);
receiver.executeMessage(msgId);
}

Expand Down
12 changes: 6 additions & 6 deletions test/unit-tests/MultiMessageSender.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,24 +160,24 @@ contract MultiMessageSenderTest is Setup {

/// @dev message expiration has to be within allowed range
function test_remote_call_invalid_expiration() public {
uint256 invalidExpMin = sender.MINIMUM_EXPIRATION() - 1 days;
uint256 invalidExpMax = sender.MAXIMUM_EXPIRATION() + 1 days;
uint256 invalidExpMin = sender.MIN_EXPIRATION() - 1 days;
uint256 invalidExpMax = sender.MAX_EXPIRATION() + 1 days;

// test expiration validation in remoteCall() which does not accept excluded adapters
vm.startPrank(caller);
vm.expectRevert(Error.INVALID_EXPIRATION_MIN.selector);
vm.expectRevert(Error.INVALID_EXPIRATION_DURATION.selector);
sender.remoteCall(DST_CHAIN_ID, address(42), bytes("42"), 0, invalidExpMin);

vm.expectRevert(Error.INVALID_EXPIRATION_MAX.selector);
vm.expectRevert(Error.INVALID_EXPIRATION_DURATION.selector);
sender.remoteCall(DST_CHAIN_ID, address(42), bytes("42"), 0, invalidExpMax);

// test expiration validation in remoteCall() which accepts excluded adapters
address[] memory excludedAdapters = new address[](0);
vm.startPrank(caller);
vm.expectRevert(Error.INVALID_EXPIRATION_MIN.selector);
vm.expectRevert(Error.INVALID_EXPIRATION_DURATION.selector);
sender.remoteCall(DST_CHAIN_ID, address(42), bytes("42"), 0, invalidExpMin, excludedAdapters);

vm.expectRevert(Error.INVALID_EXPIRATION_MAX.selector);
vm.expectRevert(Error.INVALID_EXPIRATION_DURATION.selector);
sender.remoteCall(DST_CHAIN_ID, address(42), bytes("42"), 0, invalidExpMax, excludedAdapters);
}

Expand Down

0 comments on commit b321989

Please sign in to comment.