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.

Sign up for GitHub

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

Jump to bottom

Remove ambiguity of message execution behaviour in MMR #100

Merged
merged 1 commit into from
Oct 6, 2023
Merged

Remove ambiguity of message execution behaviour in MMR #100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 16 additions & 12 deletions src/MultiBridgeMessageReceiver.sol
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading