Skip to content

Commit

Permalink
Remove ambiguity of execution behaviour in MMR (#100)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ermyas
ermyas authored Oct 6, 2023
1 parent e614b4c commit b5ca127
Show file tree
Hide file tree
Showing 11 changed files with 59 additions and 53 deletions.
28 changes: 16 additions & 12 deletions src/MultiBridgeMessageReceiver.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ contract MultiBridgeMessageReceiver is IMultiBridgeMessageReceiver, ExecutorAwar
/// @notice the data required for executing a message
mapping(bytes32 msgId => ExecutionData execData) public msgExecData;

/// @notice whether a message has been validated and sent to the governance timelock for execution
mapping(bytes32 msgId => bool executed) public isExecuted;
/// @notice whether a message has been sent to the governance timelock for execution
mapping(bytes32 msgId => bool scheduled) public isExecutionScheduled;

/*/////////////////////////////////////////////////////////////////
MODIFIERS
Expand Down Expand Up @@ -113,8 +113,9 @@ contract MultiBridgeMessageReceiver is IMultiBridgeMessageReceiver, ExecutorAwar
revert Error.DUPLICATE_MESSAGE_DELIVERY_BY_ADAPTER();
}

if (isExecuted[msgId]) {
revert Error.MSG_ID_ALREADY_EXECUTED();
/// @dev checks if msgId was already sent to the timelock for eventual execution
if (isExecutionScheduled[msgId]) {
revert Error.MSG_ID_ALREADY_SCHEDULED();
}

msgDeliveries[msgId][msg.sender] = true;
Expand All @@ -137,20 +138,20 @@ contract MultiBridgeMessageReceiver is IMultiBridgeMessageReceiver, ExecutorAwar
}

/// @inheritdoc IMultiBridgeMessageReceiver
function executeMessage(bytes32 _msgId) external override {
function scheduleMessageExecution(bytes32 _msgId) external override {
ExecutionData memory _execData = msgExecData[_msgId];

/// @dev validates if msg execution is not beyond expiration
if (block.timestamp > _execData.expiration) {
revert Error.MSG_EXECUTION_PASSED_DEADLINE();
}

/// @dev validates if msgId is already executed
if (isExecuted[_msgId]) {
revert Error.MSG_ID_ALREADY_EXECUTED();
/// @dev checks if msgId was already sent to the timelock for eventual execution
if (isExecutionScheduled[_msgId]) {
revert Error.MSG_ID_ALREADY_SCHEDULED();
}

isExecuted[_msgId] = true;
isExecutionScheduled[_msgId] = true;

/// @dev validates message quorum
if (msgDeliveryCount[_msgId] < quorum) {
Expand All @@ -162,7 +163,7 @@ contract MultiBridgeMessageReceiver is IMultiBridgeMessageReceiver, ExecutorAwar
_execData.target, _execData.value, _execData.callData
);

emit MessageExecuted(_msgId, _execData.target, _execData.value, _execData.nonce, _execData.callData);
emit MessageExecutionScheduled(_msgId, _execData.target, _execData.value, _execData.nonce, _execData.callData);
}

/// @notice update the governance timelock contract.
Expand Down Expand Up @@ -207,7 +208,10 @@ contract MultiBridgeMessageReceiver is IMultiBridgeMessageReceiver, ExecutorAwar
VIEW/READ-ONLY FUNCTIONS
////////////////////////////////////////////////////////////////*/

/// @notice view message info, return (executed, msgPower, delivered adapters)
/// @notice View message info
/// @return isExecutionScheduled is true if the message has been sent to the timelock for execution
/// @return msgCurrentVotes is the number of bridges that have delivered the message
/// @return successfulBridge is the list of bridges that have delivered the message
function getMessageInfo(bytes32 _msgId) public view returns (bool, uint256, string[] memory) {
uint256 msgCurrentVotes = msgDeliveryCount[_msgId];
string[] memory successfulBridge = new string[](msgCurrentVotes);
Expand All @@ -227,7 +231,7 @@ contract MultiBridgeMessageReceiver is IMultiBridgeMessageReceiver, ExecutorAwar
}
}

return (isExecuted[_msgId], msgCurrentVotes, successfulBridge);
return (isExecutionScheduled[_msgId], msgCurrentVotes, successfulBridge);
}

/*/////////////////////////////////////////////////////////////////
Expand Down
4 changes: 2 additions & 2 deletions src/interfaces/IMultiBridgeMessageReceiver.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ interface IMultiBridgeMessageReceiver {
/// @param nativeValue is the value that will be passed to the target address through low-level call
/// @param nonce is the nonce of the message
/// @param callData is the data that will be passed to the target address through low-level call
event MessageExecuted(
event MessageExecutionScheduled(
bytes32 indexed msgId, address indexed target, uint256 nativeValue, uint256 nonce, bytes callData
);

Expand All @@ -55,7 +55,7 @@ interface IMultiBridgeMessageReceiver {

/// @notice Sends a message, that has achieved quorum and has not yet expired, to the governance timelock for eventual execution.
/// @param _msgId is the unique identifier of the message
function executeMessage(bytes32 _msgId) external;
function scheduleMessageExecution(bytes32 _msgId) external;

/// @notice adds or removes bridge receiver adapters.
/// @param _receiverAdapters the list of receiver adapters to add or remove
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/Error.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ library Error {
error NO_SENDER_ADAPTER_FOUND();

/// @dev is thrown if msg id is already executed
error MSG_ID_ALREADY_EXECUTED();
error MSG_ID_ALREADY_SCHEDULED();

/// @dev is thrown if bridge adapter already delivered the message to multi message receiver
error DUPLICATE_MESSAGE_DELIVERY_BY_ADAPTER();
Expand Down
4 changes: 2 additions & 2 deletions test/integration-tests/GracePeriodExpiry.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ contract GracePeriodExpiryTest is Setup {

vm.selectFork(fork[DST_CHAIN_ID]);
vm.recordLogs();
/// execute the message and move it to governance timelock contract
MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).executeMessage(msgId);
/// schedule the message for execution by moving it to governance timelock contract
MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution(msgId);
(uint256 txId, address finalTarget, uint256 value, bytes memory data, uint256 eta) =
_getExecParams(vm.getRecordedLogs());

Expand Down
4 changes: 2 additions & 2 deletions test/integration-tests/MultiMessageAggregation.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ contract MultiBridgeMessageAggregationTest is Setup {

vm.selectFork(fork[DST_CHAIN_ID]);
vm.recordLogs();
/// execute the message and move it to governance timelock contract
MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).executeMessage(msgId);
/// schedule the message for execution by moving it to governance timelock contract
MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution(msgId);
(uint256 txId, address finalTarget, uint256 value, bytes memory data, uint256 eta) =
_getExecParams(vm.getRecordedLogs());

Expand Down
4 changes: 2 additions & 2 deletions test/integration-tests/RemoteAdapterAdd.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ contract RemoteAdapterAdd is Setup {

vm.selectFork(fork[DST_CHAIN_ID]);
vm.recordLogs();
/// execute the message and move it to governance timelock contract
MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).executeMessage(msgId);
/// schedule the message for execution by moving it to governance timelock contract
MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution(msgId);
(uint256 txId, address finalTarget, uint256 value, bytes memory data, uint256 eta) =
_getExecParams(vm.getRecordedLogs());

Expand Down
4 changes: 2 additions & 2 deletions test/integration-tests/RemoteAdapterRemove.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ contract RemoteAdapterRemove is Setup {

vm.selectFork(fork[DST_CHAIN_ID]);
vm.recordLogs();
/// execute the message and move it to governance timelock contract
MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).executeMessage(msgId);
/// schedule the message for execution by moving it to governance timelock contract
MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution(msgId);
(uint256 txId, address finalTarget, uint256 value, bytes memory data, uint256 eta) =
_getExecParams(vm.getRecordedLogs());

Expand Down
4 changes: 2 additions & 2 deletions test/integration-tests/RemoteSetQuorum.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ contract RemoteQuorumUpdate is Setup {

vm.selectFork(fork[DST_CHAIN_ID]);
vm.recordLogs();
/// execute the message and move it to governance timelock contract
MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).executeMessage(msgId);
/// schedule the message for execution by moving it to governance timelock contract
MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution(msgId);
(uint256 txId, address finalTarget, uint256 value, bytes memory data, uint256 eta) =
_getExecParams(vm.getRecordedLogs());

Expand Down
6 changes: 4 additions & 2 deletions test/integration-tests/RemoteTimelockUpdate.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,10 @@ contract RemoteTimelockUpdate is Setup {

vm.selectFork(fork[POLYGON_CHAIN_ID]);
vm.recordLogs();
/// execute the message and move it to governance timelock contract
MultiBridgeMessageReceiver(contractAddress[POLYGON_CHAIN_ID][bytes("MMA_RECEIVER")]).executeMessage(msgId);
/// schedule the message for execution by moving it to governance timelock contract
MultiBridgeMessageReceiver(contractAddress[POLYGON_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution(
msgId
);
(uint256 txId, address finalTarget, uint256 value, bytes memory data, uint256 eta) =
_getExecParams(vm.getRecordedLogs());

Expand Down
4 changes: 2 additions & 2 deletions test/integration-tests/TimelockCheck.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ contract TimelockCheckTest is Setup {

vm.selectFork(fork[DST_CHAIN_ID]);
vm.recordLogs();
/// execute the message and move it to governance timelock contract
MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).executeMessage(msgId);
/// schedule the message for execution by moving it to governance timelock contract
MultiBridgeMessageReceiver(contractAddress[DST_CHAIN_ID][bytes("MMA_RECEIVER")]).scheduleMessageExecution(msgId);
(uint256 txId, address finalTarget, uint256 value, bytes memory data, uint256 eta) =
_getExecParams(vm.getRecordedLogs());

Expand Down
48 changes: 24 additions & 24 deletions test/unit-tests/MultiBridgeMessageReceiver.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ contract MultiBridgeMessageReceiverTest is Setup {
event BridgeMessageReceived(
bytes32 indexed msgId, string indexed bridgeName, uint256 nonce, address receiverAdapter
);
event MessageExecuted(
event MessageExecutionScheduled(
bytes32 indexed msgId, address indexed target, uint256 nativeValue, uint256 nonce, bytes callData
);

Expand Down Expand Up @@ -69,7 +69,7 @@ contract MultiBridgeMessageReceiverTest is Setup {

receiver.receiveMessage(message);

assertFalse(receiver.isExecuted(msgId));
assertFalse(receiver.isExecutionScheduled(msgId));

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

Expand Down Expand Up @@ -199,8 +199,8 @@ contract MultiBridgeMessageReceiverTest is Setup {
receiver.receiveMessage(message);
}

/// @dev executed message should be rejected
function test_receiver_message_msg_id_already_executed() public {
/// @dev scheduled message should be rejected
function test_receiver_message_msg_id_already_scheduled() public {
MessageLibrary.Message memory message = MessageLibrary.Message({
srcChainId: SRC_CHAIN_ID,
dstChainId: DST_CHAIN_ID,
Expand All @@ -219,15 +219,15 @@ contract MultiBridgeMessageReceiverTest is Setup {
vm.startPrank(wormholeAdapterAddr);
receiver.receiveMessage(message);

receiver.executeMessage(msgId);
receiver.scheduleMessageExecution(msgId);

vm.startPrank(axelarAdapterAddr);
vm.expectRevert(Error.MSG_ID_ALREADY_EXECUTED.selector);
vm.expectRevert(Error.MSG_ID_ALREADY_SCHEDULED.selector);
receiver.receiveMessage(message);
}

/// @dev executes message delivered by two adapters
function test_execute_message() public {
/// @dev schedules message delivered by two adapters
function test_schedule_message_execution() public {
vm.startPrank(wormholeAdapterAddr);

MessageLibrary.Message memory message = MessageLibrary.Message({
Expand All @@ -247,15 +247,15 @@ contract MultiBridgeMessageReceiverTest is Setup {
receiver.receiveMessage(message);

vm.expectEmit(true, true, true, true, address(receiver));
emit MessageExecuted(msgId, address(42), 0, 42, bytes("42"));
emit MessageExecutionScheduled(msgId, address(42), 0, 42, bytes("42"));

receiver.executeMessage(msgId);
receiver.scheduleMessageExecution(msgId);

assertTrue(receiver.isExecuted(msgId));
assertTrue(receiver.isExecutionScheduled(msgId));
}

/// @dev cannot execute message past deadline
function test_execute_message_passed_deadline() public {
/// @dev cannot schedule execution of message past deadline
function test_schedule_message_execution_passed_deadline() public {
vm.startPrank(wormholeAdapterAddr);

MessageLibrary.Message memory message = MessageLibrary.Message({
Expand All @@ -272,11 +272,11 @@ contract MultiBridgeMessageReceiverTest is Setup {
receiver.receiveMessage(message);

vm.expectRevert(Error.MSG_EXECUTION_PASSED_DEADLINE.selector);
receiver.executeMessage(msgId);
receiver.scheduleMessageExecution(msgId);
}

/// @dev cannot executed message that has already been executed
function test_execute_message_msg_id_already_executed() public {
/// @dev cannot schedule execution of message that has already been scheduled
function test_schedule_message_execution_already_scheduled() public {
vm.startPrank(wormholeAdapterAddr);

MessageLibrary.Message memory message = MessageLibrary.Message({
Expand All @@ -295,14 +295,14 @@ contract MultiBridgeMessageReceiverTest is Setup {
vm.startPrank(axelarAdapterAddr);
receiver.receiveMessage(message);

receiver.executeMessage(msgId);
receiver.scheduleMessageExecution(msgId);

vm.expectRevert(Error.MSG_ID_ALREADY_EXECUTED.selector);
receiver.executeMessage(msgId);
vm.expectRevert(Error.MSG_ID_ALREADY_SCHEDULED.selector);
receiver.scheduleMessageExecution(msgId);
}

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

MessageLibrary.Message memory message = MessageLibrary.Message({
Expand All @@ -319,7 +319,7 @@ contract MultiBridgeMessageReceiverTest is Setup {
receiver.receiveMessage(message);

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

/// @dev adds one receiver adapter
Expand Down Expand Up @@ -486,8 +486,8 @@ contract MultiBridgeMessageReceiverTest is Setup {

receiver.receiveMessage(message);

(bool isExecuted, uint256 msgCurrentVotes, string[] memory successfulBridge) = receiver.getMessageInfo(msgId);
assertFalse(isExecuted);
(bool isScheduled, uint256 msgCurrentVotes, string[] memory successfulBridge) = receiver.getMessageInfo(msgId);
assertFalse(isScheduled);
assertEq(msgCurrentVotes, 1);
assertEq(successfulBridge.length, 1);
assertEq(successfulBridge[0], WormholeReceiverAdapter(wormholeAdapterAddr).name());
Expand Down

0 comments on commit b5ca127

Please sign in to comment.