Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimise trusted executor registry #70

Merged
merged 3 commits into from
Sep 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions src/MultiMessageReceiver.sol
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ
/// @dev adds the new receiver adapters before setting quorum and validations
_updateReceiverAdapters(_receiverAdapters, _operations);

if (_quorum > trustedExecutor.length || _quorum == 0) {
if (_quorum > trustedExecutorsCount() || _quorum == 0) {
revert Error.INVALID_QUORUM_THRESHOLD();
}

Expand All @@ -85,7 +85,7 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ

/// @notice receive messages from allowed bridge receiver adapters
/// @param _message is the crosschain message sent by the mma sender
/// @param _bridgeName is the name of the bridge the relays the message
/// @param _bridgeName is the name of the bridge that relays the message
function receiveMessage(MessageLibrary.Message calldata _message, string memory _bridgeName)
external
override
Expand Down Expand Up @@ -204,9 +204,10 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ

if (msgCurrentVotes != 0) {
uint256 currIndex;
for (uint256 i; i < trustedExecutor.length;) {
if (isDuplicateAdapter[msgId][trustedExecutor[i]]) {
successfulBridge[currIndex] = IMessageReceiverAdapter(trustedExecutor[i]).name();
address[] memory executors = getTrustedExecutors();
for (uint256 i; i < executors.length;) {
if (isDuplicateAdapter[msgId][executors[i]]) {
successfulBridge[currIndex] = IMessageReceiverAdapter(executors[i]).name();
++currIndex;
}

Expand All @@ -224,9 +225,10 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ
////////////////////////////////////////////////////////////////*/

function _updateQuorum(uint64 _quorum) internal {
if (_quorum > trustedExecutor.length || _quorum == 0) {
if (_quorum > trustedExecutorsCount() || _quorum == 0) {
revert Error.INVALID_QUORUM_THRESHOLD();
}

uint64 oldValue = quorum;

quorum = _quorum;
Expand Down Expand Up @@ -263,7 +265,7 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ
} else {
_removeTrustedExecutor(_receiverAdapter);

if (quorum > trustedExecutor.length) {
if (quorum > trustedExecutorsCount()) {
revert Error.INVALID_QUORUM_THRESHOLD();
}
}
Expand Down
2 changes: 0 additions & 2 deletions src/MultiMessageSender.sol
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,9 @@ contract MultiMessageSender {

/// @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 GAC.
/// Caller can use estimateTotalMessageFee() to get total message fees before 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))
Expand Down
94 changes: 29 additions & 65 deletions src/interfaces/EIP5164/ExecutorAware.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
// Copied from https://github.com/pooltogether/ERC5164/blob/main/src/abstract/ExecutorAware.sol
// Modifications:
// 1. support higher version of solidity
// 2. support multiple trustedExecutor
// 2. support multiple trustedExecutors

pragma solidity ^0.8.16;

import "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
/**
* @title ExecutorAware abstract contract
* @notice The ExecutorAware contract allows contracts on a receiving chain to execute messages from an origin chain.
Expand All @@ -15,95 +16,58 @@ pragma solidity ^0.8.16;
* @dev This contract implements EIP-2771 (https://eips.ethereum.org/EIPS/eip-2771)
* to ensure that messages are sent by a trusted `MessageExecutor` contract.
*/

abstract contract ExecutorAware {
/* ============ Variables ============ */
using EnumerableSet for EnumerableSet.AddressSet;

/// @notice Address of the trusted executor contract.
address[] public trustedExecutor;
/* ============ Variables ============ */
/// @notice Addresses of the trusted executor contracts.
EnumerableSet.AddressSet private trustedExecutors;

/* ============ External Functions ============ */

/**
* @notice Check which executor this contract trust.
* @notice Check whether the provided executor is trusted
* @param _executor Address to check
* @return Returns true if the provided executor is trusted
*/
function isTrustedExecutor(address _executor) public view returns (bool) {
for (uint256 i; i < trustedExecutor.length; ++i) {
if (trustedExecutor[i] == _executor) {
return true;
}
}
return false;
return EnumerableSet.contains(trustedExecutors, _executor);
}

/* ============ Internal Functions ============ */

/**
* @notice Add a new trusted executor.
* @param _executor Address of the `MessageExecutor` contract
* @notice Get the list of trusted executors
* @return Returns an array of trusted executors
*/
function _addTrustedExecutor(address _executor) internal {
if (!isTrustedExecutor(_executor)) {
trustedExecutor.push(_executor);
}
function getTrustedExecutors() public view returns (address[] memory) {
return EnumerableSet.values(trustedExecutors);
}

/**
* @notice Remove a trusted executor.
* @param _executor Address of the `MessageExecutor` contract
* @notice Get the total number of trusted executors
* @return Returns the total number of trusted executors
*/
function _removeTrustedExecutor(address _executor) internal {
uint256 lastIndex = trustedExecutor.length - 1;
for (uint256 i; i < trustedExecutor.length; ++i) {
if (trustedExecutor[i] == _executor) {
if (i < lastIndex) {
trustedExecutor[i] = trustedExecutor[lastIndex];
}
trustedExecutor.pop();
return;
}
}
function trustedExecutorsCount() public view returns (uint256) {
return EnumerableSet.length(trustedExecutors);
}

/**
* @notice Retrieve messageId from message data.
* @return _msgDataMessageId ID uniquely identifying the message that was executed
*/
function _messageId() internal pure returns (bytes32 _msgDataMessageId) {
_msgDataMessageId;

if (msg.data.length >= 84) {
assembly {
_msgDataMessageId := calldataload(sub(calldatasize(), 84))
}
}
}
/* ============ Internal Functions ============ */

/**
* @notice Retrieve fromChainId from message data.
* @return _msgDataFromChainId ID of the chain that dispatched the messages
* @notice Add a new trusted executor, if it is has not already been registered as trusted.
* @param _executor Address of the `MessageExecutor` contract
* @return _success Returns true if the executor was not already registered, and was added successfully
*/
function _fromChainId() internal pure returns (uint256 _msgDataFromChainId) {
_msgDataFromChainId;

if (msg.data.length >= 52) {
assembly {
_msgDataFromChainId := calldataload(sub(calldatasize(), 52))
}
}
function _addTrustedExecutor(address _executor) internal returns (bool) {
return EnumerableSet.add(trustedExecutors, _executor);
}

/**
* @notice Retrieve signer address from message data.
* @return _signer Address of the signer
* @notice Remove a trusted executor, if it is registered as trusted.
* @param _executor Address of the `MessageExecutor` contract
* @return _success Returns true if the executor was previously registered, and was removed successfully
*/
function _msgSender() internal view returns (address payable _signer) {
_signer = payable(msg.sender);

if (msg.data.length >= 20 && isTrustedExecutor(_signer)) {
assembly {
_signer := shr(96, calldataload(sub(calldatasize(), 20)))
}
}
function _removeTrustedExecutor(address _executor) internal returns (bool) {
return EnumerableSet.remove(trustedExecutors, _executor);
}
}
16 changes: 9 additions & 7 deletions test/unit-tests/MultiMessageReceiver.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ contract MultiMessageReceiverTest is Setup {
dummyReceiver.initialize(adapters, operation, 2, timelockAddr);

assertEq(dummyReceiver.quorum(), 2);
assertEq(dummyReceiver.trustedExecutor(0), wormholeAdapterAddr);
assertEq(dummyReceiver.trustedExecutor(1), axelarAdapterAddr);
assertTrue(dummyReceiver.isTrustedExecutor(wormholeAdapterAddr));
assertTrue(dummyReceiver.isTrustedExecutor(axelarAdapterAddr));
}

/// @dev initializer cannot be called twice
Expand Down Expand Up @@ -436,14 +436,15 @@ contract MultiMessageReceiverTest is Setup {
bool[] memory operations = new bool[](1);
operations[0] = true;

assertFalse(receiver.isTrustedExecutor(address(42)));

vm.expectEmit(true, true, true, true, address(receiver));
emit ReceiverAdapterUpdated(address(42), true);

receiver.updateReceiverAdapters(updatedAdapters, operations);

assertEq(receiver.trustedExecutor(0), wormholeAdapterAddr);
assertEq(receiver.trustedExecutor(1), axelarAdapterAddr);
assertEq(receiver.trustedExecutor(2), address(42));
assertTrue(receiver.isTrustedExecutor(wormholeAdapterAddr));
assertTrue(receiver.isTrustedExecutor(axelarAdapterAddr));
assertTrue(receiver.isTrustedExecutor(address(42)));
}

/// @dev removes one receiver adapter
Expand All @@ -462,7 +463,8 @@ contract MultiMessageReceiverTest is Setup {
emit ReceiverAdapterUpdated(wormholeAdapterAddr, false);

receiver.updateReceiverAdapters(updatedAdapters, operations);
assertEq(receiver.trustedExecutor(0), axelarAdapterAddr);
assertFalse(receiver.isTrustedExecutor(wormholeAdapterAddr));
assertTrue(receiver.isTrustedExecutor(axelarAdapterAddr));
}

/// @dev only governance timelock can call
Expand Down
Loading