From 4bd5926b0e19c3762446f0046c4488371c74321a Mon Sep 17 00:00:00 2001 From: echo Date: Thu, 26 Oct 2023 13:50:04 +0800 Subject: [PATCH 1/3] fix #58 --- src/ORMP.sol | 38 ------------------------------------ src/interfaces/IEndpoint.sol | 10 ---------- src/user/Application.sol | 8 -------- test/ORMP.t.sol | 21 -------------------- test/user/Application.t.sol | 4 ---- 5 files changed, 81 deletions(-) diff --git a/src/ORMP.sol b/src/ORMP.sol index f442d71..15f4f47 100644 --- a/src/ORMP.sol +++ b/src/ORMP.sol @@ -30,17 +30,6 @@ import "./security/ExcessivelySafeCall.sol"; contract ORMP is ReentrancyGuard, Channel { using ExcessivelySafeCall for address; - /// msgHash => isFailed - mapping(bytes32 => bool) public fails; - - /// @dev Notifies an observer that the failed message has been cleared. - /// @param msgHash Hash of the message. - event ClearFailedMessage(bytes32 indexed msgHash); - /// @dev Notifies an observer that the failed message has been retried. - /// @param msgHash Hash of the message. - /// @param dispatchResult Result of the message dispatch. - event RetryFailedMessage(bytes32 indexed msgHash, bool dispatchResult); - constructor(address dao) Channel(dao) {} /// @dev Send a cross-chain message over the endpoint. @@ -126,37 +115,10 @@ contract ORMP is ReentrancyGuard, Channel { { bytes32 msgHash = _recv(message, proof); dispatchResult = _dispatch(message, msgHash, gasLimit); - if (!dispatchResult) { - fails[msgHash] = true; - } // emit dispatched message event. emit MessageDispatched(msgHash, dispatchResult); } - /// @dev Retry failed message. - /// @param message Failed message info. - /// @return dispatchResult Result of the message dispatch. - function retryFailedMessage(Message calldata message) external recvNonReentrant returns (bool dispatchResult) { - bytes32 msgHash = hash(message); - require(fails[msgHash] == true, "!failed"); - dispatchResult = _dispatch(message, msgHash, gasleft()); - if (dispatchResult) { - delete fails[msgHash]; - } - emit RetryFailedMessage(msgHash, dispatchResult); - } - - /// @dev Retry failed message. - /// @notice Only message.to could clear this message. - /// @param message Failed message info. - function clearFailedMessage(Message calldata message) external { - bytes32 msgHash = hash(message); - require(fails[msgHash] == true, "!failed"); - require(message.to == msg.sender, "!auth"); - delete fails[msgHash]; - emit ClearFailedMessage(msgHash); - } - /// @dev Dispatch the cross chain message. function _dispatch(Message memory message, bytes32 msgHash, uint256 gasLimit) private diff --git a/src/interfaces/IEndpoint.sol b/src/interfaces/IEndpoint.sol index 2008491..60628c7 100644 --- a/src/interfaces/IEndpoint.sol +++ b/src/interfaces/IEndpoint.sol @@ -42,16 +42,6 @@ interface IEndpoint { view returns (uint256); - /// @dev Retry failed message. - /// @notice Only message.to could clear this message. - /// @param message Failed message info. - function clearFailedMessage(Message calldata message) external; - - /// @dev Retry failed message. - /// @param message Failed message info. - /// @return dispatchResult Result of the message dispatch. - function retryFailedMessage(Message calldata message) external returns (bool dispatchResult); - /// @dev Recv verified message and dispatch to destination user application address. /// @param message Verified receive message info. /// @param proof Message proof of this message. diff --git a/src/user/Application.sol b/src/user/Application.sol index c0554ac..50d1635 100644 --- a/src/user/Application.sol +++ b/src/user/Application.sol @@ -32,14 +32,6 @@ abstract contract Application { IEndpoint(TRUSTED_ORMP).setAppConfig(oracle, relayer); } - function _clearFailedMessage(Message memory message) internal virtual { - return IEndpoint(TRUSTED_ORMP).clearFailedMessage(message); - } - - function retryFailedMessage(Message calldata message) public virtual returns (bool dispatchResult) { - return IEndpoint(TRUSTED_ORMP).retryFailedMessage(message); - } - function isTrustedORMP(address ormp) public view returns (bool) { return TRUSTED_ORMP == ormp; } diff --git a/test/ORMP.t.sol b/test/ORMP.t.sol index 3e7b595..c3451fd 100644 --- a/test/ORMP.t.sol +++ b/test/ORMP.t.sol @@ -52,27 +52,6 @@ contract ORMPTest is Test, Verifier { assertEq(r, false); } - function test_retry() public { - perform_send(); - bool r = ormp.recv(message, abi.encode(proof), gasleft()); - assertEq(r, false); - r = ormp.retryFailedMessage(message); - assertEq(r, false); - } - - function test_clear() public { - bytes32 msgHash = hash(message); - bool failed = ormp.fails(msgHash); - assertEq(failed, false); - perform_send(); - ormp.recv(message, abi.encode(proof), gasleft()); - failed = ormp.fails(msgHash); - assertEq(failed, true); - ormp.clearFailedMessage(message); - failed = ormp.fails(msgHash); - assertEq(failed, false); - } - function fee(uint256, address) external pure returns (uint256) { return 2; } diff --git a/test/user/Application.t.sol b/test/user/Application.t.sol index 0cb71ed..b5a4037 100644 --- a/test/user/Application.t.sol +++ b/test/user/Application.t.sol @@ -49,10 +49,6 @@ contract ApplicationTest is Test { contract UserApplication is Application { constructor(address ormp) Application(ormp) {} - function clearFailedMessage(Message calldata message) public {} - - function retryFailedMessage(Message calldata message) public override returns (bool dispatchResult) {} - function setAppConfig(address relayer, address oracle) public {} function recv() public view { From bc0114c93aafd14d5fd3adcd8a84a05041280592 Mon Sep 17 00:00:00 2001 From: echo Date: Thu, 26 Oct 2023 13:52:52 +0800 Subject: [PATCH 2/3] fix #52 --- src/Verifier.sol | 5 +---- src/interfaces/IVerifier.sol | 6 ++++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/Verifier.sol b/src/Verifier.sol index ac1b75b..309e999 100644 --- a/src/Verifier.sol +++ b/src/Verifier.sol @@ -30,10 +30,7 @@ abstract contract Verifier is IVerifier { bytes32[32] messageProof; } - /// @notice Fetch message root oracle. - /// @param chainId The destination chain id. - /// @param blockNumber The block number where the message root is located. - /// @return Message root in destination chain. + /// @inheritdoc IVerifier function merkleRoot(uint256 chainId, uint256 blockNumber) public view virtual returns (bytes32); /// @inheritdoc IVerifier diff --git a/src/interfaces/IVerifier.sol b/src/interfaces/IVerifier.sol index 5a6dbd7..5b25f7b 100644 --- a/src/interfaces/IVerifier.sol +++ b/src/interfaces/IVerifier.sol @@ -18,6 +18,12 @@ pragma solidity 0.8.17; interface IVerifier { + /// @notice Fetch message root oracle. + /// @param chainId The destination chain id. + /// @param blockNumber The block number where the message root is located. + /// @return Message root in destination chain. + function merkleRoot(uint256 chainId, uint256 blockNumber) external view returns (bytes32); + /// @notice Verify message proof /// @dev Message proof provided by relayer. Oracle should provide message root of /// source chain, and verify the merkle proof of the message hash. From 18526af91228a41b539caf34e260dc7db2bb51ff Mon Sep 17 00:00:00 2001 From: echo Date: Thu, 26 Oct 2023 14:00:30 +0800 Subject: [PATCH 3/3] fix #59 --- src/user/Application.sol | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/user/Application.sol b/src/user/Application.sol index 50d1635..069b1df 100644 --- a/src/user/Application.sol +++ b/src/user/Application.sol @@ -32,8 +32,9 @@ abstract contract Application { IEndpoint(TRUSTED_ORMP).setAppConfig(oracle, relayer); } - function isTrustedORMP(address ormp) public view returns (bool) { - return TRUSTED_ORMP == ormp; + modifier onlyORMP() { + require(TRUSTED_ORMP == msg.sender, "!ormp"); + _; } function _messageId() internal pure returns (bytes32 _msgDataMessageId) { @@ -50,8 +51,8 @@ abstract contract Application { } } - function _xmsgSender() internal view returns (address payable _from) { - require(msg.data.length >= 20 && isTrustedORMP(msg.sender), "!xmsgSender"); + function _xmsgSender() internal pure returns (address payable _from) { + require(msg.data.length >= 20, "!xmsgSender"); assembly { _from := shr(96, calldataload(sub(calldatasize(), 20))) }