Fix audit issue (#61)

* fix #58 * fix #52 * fix #59
ringecosystem · Oct 26, 2023 · c572abf · c572abf
1 parent 24b3a0c
commit c572abf
Show file tree

Hide file tree

Showing 7 changed files with 12 additions and 89 deletions.
diff --git 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/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/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/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.

diff --git a/src/user/Application.sol b/src/user/Application.sol
@@ -32,16 +32,9 @@ 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;
+    modifier onlyORMP() {
+        require(TRUSTED_ORMP == msg.sender, "!ormp");
+        _;
     }
 
     function _messageId() internal pure returns (bytes32 _msgDataMessageId) {
@@ -58,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)))
         }

diff --git 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
@@ -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 {