Skip to content

Commit

Permalink
feat: add more integration tests (#57)
Browse files Browse the repository at this point in the history 
* chore: add new integration test scenarios

* chore: add more integration tests

* chore: revert foundry changes

* fix: failing tests

* feat: add user specified expiration time

* chore: merge main

* fix: commenting & refund fee test

* fix: remove deadline check test

* fix: formatter

* chore: fix nit suggested by ermyas

* chore: add more integration tests

* chore: add convenience helpers for receiver updates

* chore: add unit tests for the new function

* fix: linter
  • Loading branch information
@sujithsomraaj
sujithsomraaj authored Sep 12, 2023
1 parent 9aa65fe commit dc99b0f
Show file tree
Hide file tree
Showing 15 changed files with 579 additions and 136 deletions.
100 changes: 58 additions & 42 deletions src/MultiMessageReceiver.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,27 +58,14 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ
////////////////////////////////////////////////////////////////*/

/// @notice sets the initial parameters
function initialize(address[] calldata _receiverAdapters, uint64 _quorum, address _governanceTimelock)
external
initializer
{
uint256 len = _receiverAdapters.length;

if (len == 0) {
revert Error.ZERO_RECEIVER_ADAPTER();
}

for (uint256 i; i < len;) {
if (_receiverAdapters[i] == address(0)) {
revert Error.ZERO_ADDRESS_INPUT();
}

_updateReceiverAdapter(_receiverAdapters[i], true);

unchecked {
++i;
}
}
function initialize(
address[] calldata _receiverAdapters,
bool[] calldata _operations,
uint64 _quorum,
address _governanceTimelock
) external initializer {
/// @dev adds the new receiver adapters before setting quorum and validations
_updateReceiverAdapters(_receiverAdapters, _operations);

if (_quorum > trustedExecutor.length || _quorum == 0) {
revert Error.INVALID_QUORUM_THRESHOLD();
Expand Down Expand Up @@ -180,35 +167,30 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ

/// @notice Update bridge receiver adapters.
/// @dev called by admin to update receiver bridge adapters on all other chains
function updateReceiverAdapter(address[] calldata _receiverAdapters, bool[] calldata _operations)
function updateReceiverAdapters(address[] calldata _receiverAdapters, bool[] calldata _operations)
external
onlyGovernanceTimelock
{
uint256 len = _receiverAdapters.length;

if (len != _operations.length) {
revert Error.ARRAY_LENGTH_MISMATCHED();
}

for (uint256 i; i < len;) {
_updateReceiverAdapter(_receiverAdapters[i], _operations[i]);
_updateReceiverAdapters(_receiverAdapters, _operations);
}

unchecked {
++i;
}
}
/// @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,
address[] calldata _receiverAdapters,
bool[] calldata _operations
) external onlyGovernanceTimelock {
/// @dev updates quorum here
_updateQuorum(_newQuorum);

/// @dev updates receiver adapter here
_updateReceiverAdapters(_receiverAdapters, _operations);
}

/// @notice Update power quorum threshold of message execution.
function updateQuorum(uint64 _quorum) external onlyGovernanceTimelock {
/// NOTE: should check 2/3 ?
if (_quorum > trustedExecutor.length || _quorum == 0) {
revert Error.INVALID_QUORUM_THRESHOLD();
}
uint64 oldValue = quorum;

quorum = _quorum;
emit QuorumUpdated(oldValue, _quorum);
_updateQuorum(_quorum);
}

/*/////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -241,6 +223,40 @@ contract MultiMessageReceiver is IMultiMessageReceiver, ExecutorAware, Initializ
PRIVATE/INTERNAL FUNCTIONS
////////////////////////////////////////////////////////////////*/

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

quorum = _quorum;
emit QuorumUpdated(oldValue, _quorum);
}

function _updateReceiverAdapters(address[] memory _receiverAdapters, bool[] memory _operations) internal {
uint256 len = _receiverAdapters.length;

if (len == 0) {
revert Error.ZERO_RECEIVER_ADAPTER();
}

if (len != _operations.length) {
revert Error.ARRAY_LENGTH_MISMATCHED();
}

for (uint256 i; i < len;) {
if (_receiverAdapters[i] == address(0)) {
revert Error.ZERO_ADDRESS_INPUT();
}

_updateReceiverAdapter(_receiverAdapters[i], _operations[i]);

unchecked {
++i;
}
}
}

function _updateReceiverAdapter(address _receiverAdapter, bool _add) private {
if (_add) {
_addTrustedExecutor(_receiverAdapter);
Expand Down
45 changes: 33 additions & 12 deletions src/MultiMessageSender.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,14 @@ import "./libraries/Error.sol";
/// @title MultiMessageSender
/// @dev handles the routing of message from external sender to bridge adapters
contract MultiMessageSender {
/*///////////////////////////////////////////////////////////////
CONSTANTS
//////////////////////////////////////////////////////////////*/
IGAC public immutable gac;

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

/*/////////////////////////////////////////////////////////////////
STATE VARIABLES
////////////////////////////////////////////////////////////////*/
Expand Down Expand Up @@ -70,6 +76,18 @@ 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 > MAXIMUM_EXPIRATION) {
revert Error.INVALID_EXPIRATION_MAX();
}
_;
}

/*/////////////////////////////////////////////////////////////////
CONSTRUCTOR
////////////////////////////////////////////////////////////////*/
Expand Down Expand Up @@ -98,28 +116,33 @@ contract MultiMessageSender {
/// @param _target is the target execution point on dst 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))
function remoteCall(uint256 _dstChainId, address _target, bytes calldata _callData, uint256 _nativeValue)
external
payable
onlyCaller
{
/// @param _expiration refers to the number of days that a message remains valid before it is considered stale and can no longer be executed.
function remoteCall(
uint256 _dstChainId,
address _target,
bytes calldata _callData,
uint256 _nativeValue,
uint256 _expiration
) external payable onlyCaller {
address[] memory excludedAdapters;
_remoteCall(_dstChainId, _target, _callData, _nativeValue, excludedAdapters);
_remoteCall(_dstChainId, _target, _callData, _nativeValue, _expiration, excludedAdapters);
}

/// @param _dstChainId is the destination chainId
/// @param _target is the target execution point on dst 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 _excludedAdapters are the sender adapters to be excluded from relaying the message
/// @param _expiration refers to the number of days that a message remains valid before it is considered stale and can no longer be executed.
function remoteCall(
uint256 _dstChainId,
address _target,
bytes calldata _callData,
uint256 _nativeValue,
uint256 _expiration,
address[] calldata _excludedAdapters
) external payable onlyCaller {
_remoteCall(_dstChainId, _target, _callData, _nativeValue, _excludedAdapters);
_remoteCall(_dstChainId, _target, _callData, _nativeValue, _expiration, _excludedAdapters);
}

/// @notice Add bridge sender adapters
Expand Down Expand Up @@ -183,7 +206,6 @@ contract MultiMessageSender {
struct LocalCallVars {
address[] adapters;
uint256 adapterLength;
uint256 msgExpiration;
bool[] adapterSuccess;
bytes32 msgId;
}
Expand All @@ -193,6 +215,7 @@ contract MultiMessageSender {
address _target,
bytes calldata _callData,
uint256 _nativeValue,
uint256 _expiration,
address[] memory _excludedAdapters
) private {
LocalCallVars memory v;
Expand Down Expand Up @@ -247,10 +270,8 @@ contract MultiMessageSender {
/// @dev increments nonce
++nonce;

v.msgExpiration = block.timestamp + gac.getMsgExpiryTime();

MessageLibrary.Message memory message =
MessageLibrary.Message(block.chainid, _dstChainId, _target, nonce, _callData, _nativeValue, v.msgExpiration);
MessageLibrary.Message(block.chainid, _dstChainId, _target, nonce, _callData, _nativeValue, _expiration);

v.adapterSuccess = new bool[](v.adapterLength);

Expand Down Expand Up @@ -284,7 +305,7 @@ contract MultiMessageSender {
}

emit MultiMessageMsgSent(
v.msgId, nonce, _dstChainId, _target, _callData, _nativeValue, v.msgExpiration, v.adapters, v.adapterSuccess
v.msgId, nonce, _dstChainId, _target, _callData, _nativeValue, _expiration, v.adapters, v.adapterSuccess
);
}

Expand Down
15 changes: 0 additions & 15 deletions src/controllers/GAC.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ contract GAC is IGAC, Ownable {
STATE VARIABLES
//////////////////////////////////////////////////////////////*/
uint256 public dstGasLimit;
uint256 public msgExpiration;

/// @dev is the address to receive value refunds from remoteCall
address public refundAddress;
Expand Down Expand Up @@ -83,15 +82,6 @@ contract GAC is IGAC, Ownable {
emit DstGasLimitUpdated(oldLimit, _gasLimit);
}

/// @inheritdoc IGAC
function setMsgExpiryTime(uint256 _timeInSeconds) external override onlyOwner {
if (_timeInSeconds == 0) {
revert Error.ZERO_EXPIRATION_TIME();
}

msgExpiration = _timeInSeconds;
}

/// @inheritdoc IGAC
function setRefundAddress(address _refundAddress) external override onlyOwner {
if (_refundAddress == address(0)) {
Expand Down Expand Up @@ -124,11 +114,6 @@ contract GAC is IGAC, Ownable {
_gasLimit = dstGasLimit;
}

/// @inheritdoc IGAC
function getMsgExpiryTime() external view override returns (uint256 _expiration) {
_expiration = msgExpiration;
}

/// @inheritdoc IGAC
function getRefundAddress() external view override returns (address _refundAddress) {
_refundAddress = refundAddress;
Expand Down
7 changes: 0 additions & 7 deletions src/interfaces/IGAC.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ interface IGAC {
/// @param _gasLimit is the limit to be set
function setGlobalMsgDeliveryGasLimit(uint256 _gasLimit) external;

/// @dev sets the message expiry time
/// @param _timeInSeconds is the expiry time for the message on dst chain
function setMsgExpiryTime(uint256 _timeInSeconds) external;

/// @dev sets the refund address for gas refunds
/// @param _refundAddress is the address to receive refunds from MMA sender
function setRefundAddress(address _refundAddress) external;
Expand Down Expand Up @@ -70,9 +66,6 @@ interface IGAC {
/// @dev returns the multi message receiver on the chain
function getMultiMessageReceiver(uint256 _chainId) external view returns (address _mmaReceiver);

/// @dev returns the expiry time of message from the time of dispatch
function getMsgExpiryTime() external view returns (uint256 _expiration);

/// @dev returns the refund address
function getRefundAddress() external view returns (address _refundAddress);
}
12 changes: 9 additions & 3 deletions src/libraries/Error.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ library Error {
/// @dev is thrown when input is zero address
error ZERO_ADDRESS_INPUT();

/// @dev is thrown when quorum is zero
error ZERO_QUORUM();

/// @dev is thrown if the length of two arrays are mismatched
error ARRAY_LENGTH_MISMATCHED();

Expand All @@ -35,9 +38,6 @@ library Error {
/// @dev is thrown if bridge adapter already delivered the message to multi message receiver
error DUPLICATE_MESSAGE_DELIVERY_BY_ADAPTER();

/// @dev is thrown if expiration time is zero
error ZERO_EXPIRATION_TIME();

/// @dev is thrown if quorum threshold is greater than receiver adapters
error INVALID_QUORUM_THRESHOLD();

Expand All @@ -62,6 +62,12 @@ 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();

/*/////////////////////////////////////////////////////////////////
ADAPTER ERRORS
////////////////////////////////////////////////////////////////*/
Expand Down
19 changes: 12 additions & 7 deletions test/Setup.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ import {WormholeReceiverAdapter} from "src/adapters/Wormhole/WormholeReceiverAda
import {AxelarSenderAdapter} from "src/adapters/axelar/AxelarSenderAdapter.sol";
import {AxelarReceiverAdapter} from "src/adapters/axelar/AxelarReceiverAdapter.sol";

import {GAC} from "../src/controllers/GAC.sol";
import {GovernanceTimelock} from "../src/controllers/GovernanceTimelock.sol";
import {GAC} from "src/controllers/GAC.sol";
import {GovernanceTimelock} from "src/controllers/GovernanceTimelock.sol";

import {MultiMessageSender} from "../src/MultiMessageSender.sol";
import {MultiMessageReceiver} from "../src/MultiMessageReceiver.sol";
import {MultiMessageSender} from "src/MultiMessageSender.sol";
import {MultiMessageReceiver} from "src/MultiMessageReceiver.sol";

/// @dev can inherit the setup in tests
abstract contract Setup is Test {
Expand All @@ -32,6 +32,8 @@ abstract contract Setup is Test {
/// @dev simulated caller
address constant caller = address(10);
address constant owner = address(420);
address constant refundAddress = address(420420);
uint256 constant EXPIRATION_CONSTANT = 5 days;

/// @dev constants for axelar
address constant ETH_GATEWAY = 0x4F4495243837681061C4743b74B3eEdf548D56A5;
Expand Down Expand Up @@ -123,7 +125,6 @@ abstract contract Setup is Test {
vm.selectFork(fork[chainId]);

GAC gac = new GAC{salt: _salt}();
gac.setMsgExpiryTime(2 days);
gac.setMultiMessageCaller(caller);
contractAddress[chainId][bytes("GAC")] = address(gac);

Expand Down Expand Up @@ -280,8 +281,12 @@ abstract contract Setup is Test {
_recieverAdapters[0] = contractAddress[chainId][bytes("WORMHOLE_RECEIVER_ADAPTER")];
_recieverAdapters[1] = contractAddress[chainId][bytes("AXELAR_RECEIVER_ADAPTER")];

bool[] memory _operations = new bool[](2);
_operations[0] = true;
_operations[1] = true;

MultiMessageReceiver(contractAddress[DST_CHAINS[i]][bytes("MMA_RECEIVER")]).initialize(
_recieverAdapters, 2, contractAddress[chainId]["TIMELOCK"]
_recieverAdapters, _operations, 2, contractAddress[chainId]["TIMELOCK"]
);

unchecked {
Expand All @@ -308,7 +313,7 @@ abstract contract Setup is Test {
GAC(contractAddress[chainId][bytes("GAC")]).setMultiMessageReceiver(
ALL_CHAINS[j], contractAddress[ALL_CHAINS[j]][bytes("MMA_RECEIVER")]
);
GAC(contractAddress[chainId][bytes("GAC")]).setRefundAddress(caller);
GAC(contractAddress[chainId][bytes("GAC")]).setRefundAddress(refundAddress);
}

unchecked {
Expand Down
Loading

0 comments on commit dc99b0f

Please sign in to comment.