From 6324b355bc240116351ce2d74bb0f08913b12175 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Thu, 19 Sep 2024 11:56:13 +0100 Subject: [PATCH 1/7] feat: scaffold Manager --- contracts/messaging/MessageBusManager.sol | 46 +++++++++++++++++++ .../messaging/interfaces/IManageable.sol | 22 +++++++++ contracts/messaging/interfaces/IManager.sol | 10 ++++ 3 files changed, 78 insertions(+) create mode 100644 contracts/messaging/MessageBusManager.sol create mode 100644 contracts/messaging/interfaces/IManageable.sol create mode 100644 contracts/messaging/interfaces/IManager.sol diff --git a/contracts/messaging/MessageBusManager.sol b/contracts/messaging/MessageBusManager.sol new file mode 100644 index 000000000..4df272841 --- /dev/null +++ b/contracts/messaging/MessageBusManager.sol @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.13; + +import {IManager} from "./interfaces/IManager.sol"; + +import {Ownable} from "@openzeppelin/contracts-4.5.0/access/Ownable.sol"; + +contract MessageBusManager is IManager, Ownable { + constructor(address messageBus_, address owner_) { + // TODO: implement + } + + function resetFailedMessages(bytes32[] calldata messageIds) external { + // TODO: implement + } + + // ═════════════════════════════════════════════ GENERIC MANAGING ══════════════════════════════════════════════════ + + function updateMessageStatus(bytes32 messageId, TxStatus status) external { + // TODO: implement + } + + function updateAuthVerifier(address authVerifier) external { + // TODO: implement + } + + function withdrawGasFees(address payable to) external { + // TODO: implement + } + + function rescueGas(address payable to) external { + // TODO: implement + } + + function updateGasFeePricing(address gasFeePricing) external { + // TODO: implement + } + + function transferMessageBusOwnership(address newOwner) external { + // TODO: implement + } + + function getExecutedMessage(bytes32 messageId) public view returns (TxStatus) { + // TODO: implement + } +} diff --git a/contracts/messaging/interfaces/IManageable.sol b/contracts/messaging/interfaces/IManageable.sol new file mode 100644 index 000000000..595919701 --- /dev/null +++ b/contracts/messaging/interfaces/IManageable.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +interface IManageable { + enum TxStatus { + Null, + Success, + Fail + } + + function updateMessageStatus(bytes32 messageId, TxStatus status) external; + + function updateAuthVerifier(address authVerifier) external; + + function withdrawGasFees(address payable to) external; + + function rescueGas(address payable to) external; + + function updateGasFeePricing(address gasFeePricing) external; + + function getExecutedMessage(bytes32 messageId) external view returns (TxStatus); +} diff --git a/contracts/messaging/interfaces/IManager.sol b/contracts/messaging/interfaces/IManager.sol new file mode 100644 index 000000000..6dd8d1a2f --- /dev/null +++ b/contracts/messaging/interfaces/IManager.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import {IManageable} from "./IManageable.sol"; + +interface IManager is IManageable { + function resetFailedMessages(bytes32[] calldata messageIds) external; + + function transferMessageBusOwnership(address newOwner) external; +} From e82d9219622a1cced1284d92287851c1fffc0b28 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Thu, 19 Sep 2024 12:06:58 +0100 Subject: [PATCH 2/7] test: add unit tests for Manager --- test/messaging/MessageBusHarness.sol | 17 ++ test/messaging/MessageBusManager.t.sol | 213 +++++++++++++++++++++++++ 2 files changed, 230 insertions(+) create mode 100644 test/messaging/MessageBusHarness.sol create mode 100644 test/messaging/MessageBusManager.t.sol diff --git a/test/messaging/MessageBusHarness.sol b/test/messaging/MessageBusHarness.sol new file mode 100644 index 000000000..1c3adb442 --- /dev/null +++ b/test/messaging/MessageBusHarness.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.13; + +import {MessageBus} from "../../contracts/messaging/MessageBus.sol"; + +// DO NOT USE THIS CONTRACT IN PRODUCTION +contract MessageBusHarness is MessageBus { + constructor(address _gasFeePricing, address _authVerifier) MessageBus(_gasFeePricing, _authVerifier) {} + + function setMessageStatus(bytes32 messageId, TxStatus status) external { + executedMessages[messageId] = status; + } + + function setFees(uint256 fees_) external { + fees = fees_; + } +} diff --git a/test/messaging/MessageBusManager.t.sol b/test/messaging/MessageBusManager.t.sol new file mode 100644 index 000000000..2415a2ffe --- /dev/null +++ b/test/messaging/MessageBusManager.t.sol @@ -0,0 +1,213 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.13; + +import {IManageable} from "../../contracts/messaging/interfaces/IManageable.sol"; +import {MessageBusManager} from "../../contracts/messaging/MessageBusManager.sol"; +import {MessageBusReceiver} from "../../contracts/messaging/MessageBusReceiver.sol"; + +import {MessageBusHarness} from "./MessageBusHarness.sol"; +import {Test} from "forge-std/Test.sol"; + +// solhint-disable func-name-mixedcase +contract MessageBusManagerTest is Test { + bytes32 public constant MESSAGE_ID = bytes32("Test"); + + MessageBusHarness public messageBus; + MessageBusManager public manager; + + address public authVerifier = makeAddr("authVerifier"); + address public gasFeePricing = makeAddr("gasFeePricing"); + address public owner = makeAddr("owner"); + + address payable public gasRecipient = payable(makeAddr("gasRecipient")); + + function setUp() public { + messageBus = new MessageBusHarness({_gasFeePricing: gasFeePricing, _authVerifier: authVerifier}); + manager = new MessageBusManager({messageBus_: address(messageBus), owner_: owner}); + messageBus.transferOwnership(address(manager)); + } + + function assertEq(MessageBusReceiver.TxStatus status, IManageable.TxStatus expected) internal { + assertEq(uint8(status), uint8(expected)); + } + + function assertEq(IManageable.TxStatus status, IManageable.TxStatus expected) internal { + assertEq(uint8(status), uint8(expected)); + } + + function toArray( + bytes32 a, + bytes32 b, + bytes32 c + ) public pure returns (bytes32[] memory arr) { + arr = new bytes32[](3); + arr[0] = a; + arr[1] = b; + arr[2] = c; + } + + function test_updateMessageStatus_success() public { + vm.prank(owner); + manager.updateMessageStatus(MESSAGE_ID, IManageable.TxStatus.Success); + assertEq(messageBus.getExecutedMessage(MESSAGE_ID), IManageable.TxStatus.Success); + } + + function test_updateMessageStatus_fail() public { + vm.prank(owner); + manager.updateMessageStatus(MESSAGE_ID, IManageable.TxStatus.Fail); + assertEq(messageBus.getExecutedMessage(MESSAGE_ID), IManageable.TxStatus.Fail); + } + + function test_updateMessageStatus_null() public { + messageBus.setMessageStatus(MESSAGE_ID, MessageBusReceiver.TxStatus.Fail); + vm.prank(owner); + manager.updateMessageStatus(MESSAGE_ID, IManageable.TxStatus.Null); + assertEq(messageBus.getExecutedMessage(MESSAGE_ID), IManageable.TxStatus.Null); + } + + function test_updateMessageStatus_revert_callerNotOwner(address caller) public { + vm.assume(caller != owner); + vm.expectRevert(); + vm.prank(caller); + manager.updateMessageStatus(MESSAGE_ID, IManageable.TxStatus.Success); + } + + function test_updateAuthVerifier() public { + vm.prank(owner); + manager.updateAuthVerifier(address(1)); + assertEq(messageBus.authVerifier(), address(1)); + } + + function test_updateAuthVerifier_revert_callerNotOwner(address caller) public { + vm.assume(caller != owner); + vm.expectRevert(); + vm.prank(caller); + manager.updateAuthVerifier(address(1)); + } + + function test_withdrawGasFees() public { + deal(address(messageBus), 123456); + messageBus.setFees(123456); + vm.prank(owner); + manager.withdrawGasFees(gasRecipient); + assertEq(address(messageBus).balance, 0); + assertEq(gasRecipient.balance, 123456); + } + + function test_withdrawGasFees_revert_callerNotOwner(address caller) public { + vm.assume(caller != owner); + deal(address(messageBus), 123456); + messageBus.setFees(123456); + vm.expectRevert(); + vm.prank(caller); + manager.withdrawGasFees(gasRecipient); + } + + function test_rescueGas() public { + deal(address(messageBus), 123456); + vm.prank(owner); + manager.rescueGas(gasRecipient); + assertEq(address(messageBus).balance, 0); + assertEq(gasRecipient.balance, 123456); + } + + function test_rescueGas_revert_callerNotOwner(address caller) public { + vm.assume(caller != owner); + deal(address(messageBus), 123456); + vm.expectRevert(); + vm.prank(caller); + manager.rescueGas(gasRecipient); + } + + function test_updateGasFeePricing() public { + vm.prank(owner); + manager.updateGasFeePricing(address(1)); + assertEq(messageBus.gasFeePricing(), address(1)); + } + + function test_updateGasFeePricing_revert_callerNotOwner(address caller) public { + vm.assume(caller != owner); + vm.expectRevert(); + vm.prank(caller); + manager.updateGasFeePricing(address(1)); + } + + function test_transferOwnership() public { + vm.prank(owner); + manager.transferOwnership(address(1)); + assertEq(manager.owner(), address(1)); + } + + function test_transferOwnership_revert_callerNotOwner(address caller) public { + vm.assume(caller != owner); + vm.expectRevert(); + vm.prank(caller); + manager.transferOwnership(address(1)); + } + + function test_transferMessageBusOwnership() public { + vm.prank(owner); + manager.transferMessageBusOwnership(address(1)); + assertEq(messageBus.owner(), address(1)); + } + + function test_transferMessageBusOwnership_revert_callerNotOwner(address caller) public { + vm.assume(caller != owner); + vm.expectRevert(); + vm.prank(caller); + manager.transferMessageBusOwnership(address(1)); + } + + function test_getExecutedMessage() public { + messageBus.setMessageStatus(MESSAGE_ID, MessageBusReceiver.TxStatus.Success); + assertEq(manager.getExecutedMessage(MESSAGE_ID), IManageable.TxStatus.Success); + messageBus.setMessageStatus(MESSAGE_ID, MessageBusReceiver.TxStatus.Fail); + assertEq(manager.getExecutedMessage(MESSAGE_ID), IManageable.TxStatus.Fail); + messageBus.setMessageStatus(MESSAGE_ID, MessageBusReceiver.TxStatus.Null); + assertEq(manager.getExecutedMessage(MESSAGE_ID), IManageable.TxStatus.Null); + } + + function test_resetFailedMessages() public { + bytes32[] memory messageIds = toArray("Test1", "Test2", "Test3"); + for (uint256 i = 0; i < messageIds.length; i++) { + messageBus.setMessageStatus(messageIds[i], MessageBusReceiver.TxStatus.Fail); + } + vm.prank(owner); + manager.resetFailedMessages(messageIds); + for (uint256 i = 0; i < messageIds.length; i++) { + assertEq(manager.getExecutedMessage(messageIds[i]), IManageable.TxStatus.Null); + assertEq(messageBus.getExecutedMessage(messageIds[i]), IManageable.TxStatus.Null); + } + } + + function test_resetFailedMessages_revert_hasNullMessage() public { + bytes32[] memory messageIds = toArray("Test1", "Test2", "Test3"); + messageBus.setMessageStatus(messageIds[0], MessageBusReceiver.TxStatus.Fail); + messageBus.setMessageStatus(messageIds[1], MessageBusReceiver.TxStatus.Null); + messageBus.setMessageStatus(messageIds[2], MessageBusReceiver.TxStatus.Fail); + vm.expectRevert(); + vm.prank(owner); + manager.resetFailedMessages(messageIds); + } + + function test_resetFailedMessages_revert_hasSuccessMessage() public { + bytes32[] memory messageIds = toArray("Test1", "Test2", "Test3"); + messageBus.setMessageStatus(messageIds[0], MessageBusReceiver.TxStatus.Fail); + messageBus.setMessageStatus(messageIds[1], MessageBusReceiver.TxStatus.Fail); + messageBus.setMessageStatus(messageIds[2], MessageBusReceiver.TxStatus.Success); + vm.expectRevert(); + vm.prank(owner); + manager.resetFailedMessages(messageIds); + } + + function test_resetFailedMessages_revert_callerNotOwner(address caller) public { + vm.assume(caller != owner); + bytes32[] memory messageIds = toArray("Test1", "Test2", "Test3"); + for (uint256 i = 0; i < messageIds.length; i++) { + messageBus.setMessageStatus(messageIds[i], MessageBusReceiver.TxStatus.Fail); + } + vm.expectRevert(); + vm.prank(caller); + manager.resetFailedMessages(messageIds); + } +} From 1cbffb7db5e487ff252581e13126f62904b08b4b Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Thu, 19 Sep 2024 12:07:46 +0100 Subject: [PATCH 3/7] feat: basic managing --- contracts/messaging/MessageBusManager.sol | 33 ++++++++++++----------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/contracts/messaging/MessageBusManager.sol b/contracts/messaging/MessageBusManager.sol index 4df272841..4d6fb8b6f 100644 --- a/contracts/messaging/MessageBusManager.sol +++ b/contracts/messaging/MessageBusManager.sol @@ -1,13 +1,16 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.13; -import {IManager} from "./interfaces/IManager.sol"; +import {IManager, IManageable} from "./interfaces/IManager.sol"; import {Ownable} from "@openzeppelin/contracts-4.5.0/access/Ownable.sol"; contract MessageBusManager is IManager, Ownable { + address public immutable MESSAGE_BUS; + constructor(address messageBus_, address owner_) { - // TODO: implement + MESSAGE_BUS = messageBus_; + transferOwnership(owner_); } function resetFailedMessages(bytes32[] calldata messageIds) external { @@ -16,31 +19,31 @@ contract MessageBusManager is IManager, Ownable { // ═════════════════════════════════════════════ GENERIC MANAGING ══════════════════════════════════════════════════ - function updateMessageStatus(bytes32 messageId, TxStatus status) external { - // TODO: implement + function updateMessageStatus(bytes32 messageId, TxStatus status) external onlyOwner { + IManageable(MESSAGE_BUS).updateMessageStatus(messageId, status); } - function updateAuthVerifier(address authVerifier) external { - // TODO: implement + function updateAuthVerifier(address authVerifier) external onlyOwner { + IManageable(MESSAGE_BUS).updateAuthVerifier(authVerifier); } - function withdrawGasFees(address payable to) external { - // TODO: implement + function withdrawGasFees(address payable to) external onlyOwner { + IManageable(MESSAGE_BUS).withdrawGasFees(to); } - function rescueGas(address payable to) external { - // TODO: implement + function rescueGas(address payable to) external onlyOwner { + IManageable(MESSAGE_BUS).rescueGas(to); } - function updateGasFeePricing(address gasFeePricing) external { - // TODO: implement + function updateGasFeePricing(address gasFeePricing) external onlyOwner { + IManageable(MESSAGE_BUS).updateGasFeePricing(gasFeePricing); } - function transferMessageBusOwnership(address newOwner) external { - // TODO: implement + function transferMessageBusOwnership(address newOwner) external onlyOwner { + Ownable(MESSAGE_BUS).transferOwnership(newOwner); } function getExecutedMessage(bytes32 messageId) public view returns (TxStatus) { - // TODO: implement + return IManageable(MESSAGE_BUS).getExecutedMessage(messageId); } } From 4f76cec665acfe69d52a630c780a59ac83da208f Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Thu, 19 Sep 2024 12:11:45 +0100 Subject: [PATCH 4/7] test: expect precise error --- contracts/messaging/MessageBusManager.sol | 2 ++ test/messaging/MessageBusManager.t.sol | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/contracts/messaging/MessageBusManager.sol b/contracts/messaging/MessageBusManager.sol index 4d6fb8b6f..347e714dd 100644 --- a/contracts/messaging/MessageBusManager.sol +++ b/contracts/messaging/MessageBusManager.sol @@ -8,6 +8,8 @@ import {Ownable} from "@openzeppelin/contracts-4.5.0/access/Ownable.sol"; contract MessageBusManager is IManager, Ownable { address public immutable MESSAGE_BUS; + error MessageBusManager__NotFailed(bytes32 messageId); + constructor(address messageBus_, address owner_) { MESSAGE_BUS = messageBus_; transferOwnership(owner_); diff --git a/test/messaging/MessageBusManager.t.sol b/test/messaging/MessageBusManager.t.sol index 2415a2ffe..96b69e929 100644 --- a/test/messaging/MessageBusManager.t.sol +++ b/test/messaging/MessageBusManager.t.sol @@ -185,7 +185,7 @@ contract MessageBusManagerTest is Test { messageBus.setMessageStatus(messageIds[0], MessageBusReceiver.TxStatus.Fail); messageBus.setMessageStatus(messageIds[1], MessageBusReceiver.TxStatus.Null); messageBus.setMessageStatus(messageIds[2], MessageBusReceiver.TxStatus.Fail); - vm.expectRevert(); + vm.expectRevert(abi.encodeWithSelector(MessageBusManager.MessageBusManager__NotFailed.selector, messageIds[1])); vm.prank(owner); manager.resetFailedMessages(messageIds); } @@ -195,7 +195,7 @@ contract MessageBusManagerTest is Test { messageBus.setMessageStatus(messageIds[0], MessageBusReceiver.TxStatus.Fail); messageBus.setMessageStatus(messageIds[1], MessageBusReceiver.TxStatus.Fail); messageBus.setMessageStatus(messageIds[2], MessageBusReceiver.TxStatus.Success); - vm.expectRevert(); + vm.expectRevert(abi.encodeWithSelector(MessageBusManager.MessageBusManager__NotFailed.selector, messageIds[2])); vm.prank(owner); manager.resetFailedMessages(messageIds); } From bc149c5897bb22de82b5dd3271ceef965ffb04c3 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Thu, 19 Sep 2024 12:15:52 +0100 Subject: [PATCH 5/7] feat: resetFailedMessages --- contracts/messaging/MessageBusManager.sol | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/contracts/messaging/MessageBusManager.sol b/contracts/messaging/MessageBusManager.sol index 347e714dd..7692f5dff 100644 --- a/contracts/messaging/MessageBusManager.sol +++ b/contracts/messaging/MessageBusManager.sol @@ -15,8 +15,14 @@ contract MessageBusManager is IManager, Ownable { transferOwnership(owner_); } - function resetFailedMessages(bytes32[] calldata messageIds) external { - // TODO: implement + function resetFailedMessages(bytes32[] calldata messageIds) external onlyOwner { + for (uint256 i = 0; i < messageIds.length; i++) { + bytes32 messageId = messageIds[i]; + if (getExecutedMessage(messageId) != IManageable.TxStatus.Fail) { + revert MessageBusManager__NotFailed(messageId); + } + IManageable(MESSAGE_BUS).updateMessageStatus(messageId, IManageable.TxStatus.Null); + } } // ═════════════════════════════════════════════ GENERIC MANAGING ══════════════════════════════════════════════════ From 45ac4b98f1ddd2c101fbb2dab4bca92b40ba9c40 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Thu, 19 Sep 2024 12:22:24 +0100 Subject: [PATCH 6/7] test: should not allow zero addresses in constructor --- contracts/messaging/MessageBusManager.sol | 1 + test/messaging/MessageBusManager.t.sol | 15 +++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/contracts/messaging/MessageBusManager.sol b/contracts/messaging/MessageBusManager.sol index 7692f5dff..f290cd4dc 100644 --- a/contracts/messaging/MessageBusManager.sol +++ b/contracts/messaging/MessageBusManager.sol @@ -9,6 +9,7 @@ contract MessageBusManager is IManager, Ownable { address public immutable MESSAGE_BUS; error MessageBusManager__NotFailed(bytes32 messageId); + error MessageBusManager__ZeroAddress(); constructor(address messageBus_, address owner_) { MESSAGE_BUS = messageBus_; diff --git a/test/messaging/MessageBusManager.t.sol b/test/messaging/MessageBusManager.t.sol index 96b69e929..a996dc59e 100644 --- a/test/messaging/MessageBusManager.t.sol +++ b/test/messaging/MessageBusManager.t.sol @@ -46,6 +46,21 @@ contract MessageBusManagerTest is Test { arr[2] = c; } + function test_constructor() public { + assertEq(manager.MESSAGE_BUS(), address(messageBus)); + assertEq(manager.owner(), owner); + } + + function test_constructor_revert_zeroMessageBus() public { + vm.expectRevert(MessageBusManager.MessageBusManager__ZeroAddress.selector); + new MessageBusManager({messageBus_: address(0), owner_: owner}); + } + + function test_constructor_revert_zeroOwner() public { + vm.expectRevert(MessageBusManager.MessageBusManager__ZeroAddress.selector); + new MessageBusManager({messageBus_: address(messageBus), owner_: address(0)}); + } + function test_updateMessageStatus_success() public { vm.prank(owner); manager.updateMessageStatus(MESSAGE_ID, IManageable.TxStatus.Success); From 6d71a7edfa2245e45d00ccedc154209ffc94dbb0 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Thu, 19 Sep 2024 12:23:37 +0100 Subject: [PATCH 7/7] feat: address check in constructor --- contracts/messaging/MessageBusManager.sol | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contracts/messaging/MessageBusManager.sol b/contracts/messaging/MessageBusManager.sol index f290cd4dc..7dadf6335 100644 --- a/contracts/messaging/MessageBusManager.sol +++ b/contracts/messaging/MessageBusManager.sol @@ -12,6 +12,9 @@ contract MessageBusManager is IManager, Ownable { error MessageBusManager__ZeroAddress(); constructor(address messageBus_, address owner_) { + if (messageBus_ == address(0) || owner_ == address(0)) { + revert MessageBusManager__ZeroAddress(); + } MESSAGE_BUS = messageBus_; transferOwnership(owner_); }