Fix Immunefi reports (#131)

* fix #119

* fix

* Fix #120

* Simply

* keep the same interface

* rename assign event

* fix ci

src/ORMP.sol

@@ -30,6 +30,10 @@ import "./security/ExcessivelySafeCall.sol";
 contract ORMP is ReentrancyGuard, Channel {
     using ExcessivelySafeCall for address;
 
+    event MessageAssigned(
+        bytes32 indexed msgHash, address indexed oracle, address indexed relayer, uint256 oracleFee, uint256 relayerFee
+    );
+
     constructor(address dao) Channel(dao) {}
 
     /// @dev Send a cross-chain message over the endpoint.
@@ -39,22 +43,21 @@ contract ORMP is ReentrancyGuard, Channel {
     /// @param gasLimit Gas limit for destination user application used.
     /// @param encoded The calldata which encoded by ABI Encoding.
     /// @param refund Return extra fee to refund address.
-    /// @param params General extensibility for relayer to custom functionality.
     function send(
         uint256 toChainId,
         address to,
         uint256 gasLimit,
         bytes calldata encoded,
         address refund,
-        bytes calldata params
+        bytes calldata
     ) external payable sendNonReentrant returns (bytes32) {
         // user application address.
         address ua = msg.sender;
         // send message by channel, return the hash of the message as id.
         bytes32 msgHash = _send(ua, toChainId, to, gasLimit, encoded);
 
         // handle fee
-        _handleFee(ua, refund, msgHash, toChainId, gasLimit, encoded, params);
+        _handleFee(ua, refund, msgHash, toChainId, gasLimit, encoded);
 
         return msgHash;
     }
@@ -65,21 +68,21 @@ contract ORMP is ReentrancyGuard, Channel {
         bytes32 msgHash,
         uint256 toChainId,
         uint256 gasLimit,
-        bytes calldata encoded,
-        bytes calldata params
+        bytes calldata encoded
     ) internal {
         // fetch user application's config.
         UC memory uc = getAppConfig(ua);
         // handle relayer fee
-        uint256 relayerFee = _handleRelayer(uc.relayer, msgHash, toChainId, ua, gasLimit, encoded, params);
+        uint256 relayerFee = _handleRelayer(uc.relayer, toChainId, ua, gasLimit, encoded);
         // handle oracle fee
-        uint256 oracleFee = _handleOracle(uc.oracle, msgHash, toChainId, ua);
+        uint256 oracleFee = _handleOracle(uc.oracle, toChainId, ua);
+
+        emit MessageAssigned(msgHash, uc.oracle, uc.relayer, oracleFee, relayerFee);
 
         // refund
         if (msg.value > relayerFee + oracleFee) {
             uint256 refundFee = msg.value - (relayerFee + oracleFee);
-            (bool success,) = refund.call{value: refundFee}("");
-            require(success, "!refund");
+            _sendValue(refund, refundFee);
         }
     }
 
@@ -88,35 +91,29 @@ contract ORMP is ReentrancyGuard, Channel {
     //  @param ua User application contract address which send the message.
     /// @param gasLimit Gas limit for destination user application used.
     /// @param encoded The calldata which encoded by ABI Encoding.
-    /// @param params General extensibility for relayer to custom functionality.
-    function fee(uint256 toChainId, address ua, uint256 gasLimit, bytes calldata encoded, bytes calldata params)
+    function fee(uint256 toChainId, address ua, uint256 gasLimit, bytes calldata encoded, bytes calldata)
         external
         view
         returns (uint256)
     {
         UC memory uc = getAppConfig(ua);
-        uint256 relayerFee = IRelayer(uc.relayer).fee(toChainId, ua, gasLimit, encoded, params);
+        uint256 relayerFee = IRelayer(uc.relayer).fee(toChainId, ua, gasLimit, encoded);
         uint256 oracleFee = IOracle(uc.oracle).fee(toChainId, ua);
         return relayerFee + oracleFee;
     }
 
-    function _handleRelayer(
-        address relayer,
-        bytes32 msgHash,
-        uint256 toChainId,
-        address ua,
-        uint256 gasLimit,
-        bytes calldata encoded,
-        bytes calldata params
-    ) internal returns (uint256) {
-        uint256 relayerFee = IRelayer(relayer).fee(toChainId, ua, gasLimit, encoded, params);
-        IRelayer(relayer).assign{value: relayerFee}(msgHash, params);
+    function _handleRelayer(address relayer, uint256 toChainId, address ua, uint256 gasLimit, bytes calldata encoded)
+        internal
+        returns (uint256)
+    {
+        uint256 relayerFee = IRelayer(relayer).fee(toChainId, ua, gasLimit, encoded);
+        _sendValue(relayer, relayerFee);
         return relayerFee;
     }
 
-    function _handleOracle(address oracle, bytes32 msgHash, uint256 toChainId, address ua) internal returns (uint256) {
+    function _handleOracle(address oracle, uint256 toChainId, address ua) internal returns (uint256) {
         uint256 oracleFee = IOracle(oracle).fee(toChainId, ua);
-        IOracle(oracle).assign{value: oracleFee}(msgHash);
+        _sendValue(oracle, oracleFee);
         return oracleFee;
     }
 
@@ -147,4 +144,11 @@ contract ORMP is ReentrancyGuard, Channel {
             abi.encodePacked(message.encoded, msgHash, message.fromChainId, message.from)
         );
     }
+
+    /// @dev Replacement for Solidity's `transfer`: sends `amount` wei to
+    /// `recipient`, forwarding all available gas and reverting on errors.
+    function _sendValue(address recipient, uint256 amount) internal {
+        (bool success,) = recipient.call{value: amount}("");
+        require(success, "!send");
+    }
 }

src/UserConfig.sol

@@ -40,6 +40,10 @@ contract UserConfig {
     /// @param oracle Oracle which the user application choose.
     /// @param relayer Relayer which the user application choose.
     event AppConfigUpdated(address indexed ua, address oracle, address relayer);
+    /// @dev Notifies an observer that the setter is changed.
+    /// @param oldSetter Old setter address.
+    /// @param newSetter New setter address.
+    event SetterChanged(address indexed oldSetter, address indexed newSetter);
 
     modifier onlySetter() {
         require(msg.sender == setter, "!auth");
@@ -52,9 +56,11 @@ contract UserConfig {
 
     /// @dev Change setter.
     /// @notice Only current setter could call.
-    /// @param setter_ New setter.
-    function changeSetter(address setter_) external onlySetter {
-        setter = setter_;
+    /// @param newSetter New setter.
+    function changeSetter(address newSetter) external onlySetter {
+        address oldSetter = setter;
+        setter = newSetter;
+        emit SetterChanged(oldSetter, newSetter);
     }
 
     /// @dev Set default user config for all user application.

src/eco/ORMPOracle.sol

@@ -20,7 +20,6 @@ pragma solidity 0.8.17;
 import "../Verifier.sol";
 
 contract ORMPOracle is Verifier {
-    event Assigned(bytes32 indexed msgHash, uint256 fee);
     event SetFee(uint256 indexed chainId, uint256 fee);
     event SetApproved(address operator, bool approve);
     event Withdrawal(address indexed to, uint256 amt);
@@ -96,11 +95,6 @@ contract ORMPOracle is Verifier {
         return f;
     }
 
-    function assign(bytes32 msgHash) external payable {
-        require(msg.sender == PROTOCOL, "!auth");
-        emit Assigned(msgHash, msg.value);
-    }
-
     function merkleRoot(uint256 chainId, uint256 blockNumber) public view override returns (bytes32) {
         return rootOf[chainId][blockNumber];
     }

src/eco/Oracle.sol

@@ -21,7 +21,6 @@ import "../Verifier.sol";
 import "../interfaces/IFeedOracle.sol";
 
 contract Oracle is Verifier {
-    event Assigned(bytes32 indexed msgHash, uint256 fee);
     event SetFee(uint256 indexed chainId, uint256 fee);
     event SetApproved(address operator, bool approve);
 
@@ -79,11 +78,6 @@ contract Oracle is Verifier {
         return feeOf[toChainId];
     }
 
-    function assign(bytes32 msgHash) external payable {
-        require(msg.sender == PROTOCOL, "!auth");
-        emit Assigned(msgHash, msg.value);
-    }
-
     function merkleRoot(uint256 chainId, uint256 /*blockNumber*/ ) public view override returns (bytes32) {
         return IFeedOracle(SUBAPI).messageRootOf(chainId);
     }

src/eco/Relayer.sol

@@ -20,10 +20,10 @@ pragma solidity 0.8.17;
 import "../interfaces/IORMP.sol";
 
 contract Relayer {
-    event Assigned(bytes32 indexed msgHash, uint256 fee, bytes params, bytes32[32] proof);
     event SetDstPrice(uint256 indexed chainId, uint128 dstPriceRatio, uint128 dstGasPriceInWei);
     event SetDstConfig(uint256 indexed chainId, uint64 baseGas, uint64 gasPerByte);
     event SetApproved(address operator, bool approve);
+    event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);
 
     struct DstPrice {
         uint128 dstPriceRatio; // dstPrice / localPrice * 10^10
@@ -69,8 +69,10 @@ contract Relayer {
         return approvedOf[operator];
     }
 
-    function changeOwner(address owner_) external onlyOwner {
-        owner = owner_;
+    function changeOwner(address newOwner) external onlyOwner {
+        address oldOwner = owner;
+        owner = newOwner;
+        emit OwnershipTransferred(oldOwner, newOwner);
     }
 
     function setApproved(address operator, bool approve) public onlyOwner {
@@ -89,13 +91,11 @@ contract Relayer {
     }
 
     // extraGas = gasLimit
-    function fee(
-        uint256 toChainId,
-        address, /*ua*/
-        uint256 gasLimit,
-        bytes calldata encoded,
-        bytes calldata /*params*/
-    ) public view returns (uint256) {
+    function fee(uint256 toChainId, address, /*ua*/ uint256 gasLimit, bytes calldata encoded)
+        public
+        view
+        returns (uint256)
+    {
         uint256 size = encoded.length;
         uint256 extraGas = gasLimit;
         DstPrice memory p = priceOf[toChainId];
@@ -111,11 +111,6 @@ contract Relayer {
         return sourceToken + payloadToken;
     }
 
-    function assign(bytes32 msgHash, bytes calldata params) external payable {
-        require(msg.sender == PROTOCOL, "!ormp");
-        emit Assigned(msgHash, msg.value, params, IORMP(PROTOCOL).prove());
-    }
-
     function relay(Message calldata message, bytes calldata proof) external onlyApproved {
         IORMP(PROTOCOL).recv(message, proof);
     }

src/interfaces/IORMP.sol

@@ -27,8 +27,8 @@ interface IORMP {
     /// @param gasLimit Gas limit for destination user application used.
     /// @param encoded The calldata which encoded by ABI Encoding.
     /// @param refund Return extra fee to refund address.
-    /// @param params General extensibility for relayer to custom functionality.
     /// @return Return the hash of the message as message id.
+    /// @param params General extensibility for relayer to custom functionality.
     function send(
         uint256 toChainId,
         address to,

src/interfaces/IOracle.sol

@@ -25,8 +25,4 @@ interface IOracle is IVerifier {
     /// @param ua The user application which send the message.
     /// @return Oracle price in source native gas.
     function fee(uint256 toChainId, address ua) external view returns (uint256);
-
-    /// @notice Assign the relay message root task to oracle maintainer.
-    /// @param msgHash Hash of the message.
-    function assign(bytes32 msgHash) external payable;
 }

src/interfaces/IRelayer.sol

@@ -23,15 +23,9 @@ interface IRelayer {
     /// @param ua The user application which send the message.
     /// @param gasLimit Gas limit for destination user application used.
     /// @param encoded The calldata which encoded by ABI Encoding.
-    /// @param params General extensibility for relayer to custom functionality.
     /// @return Relayer price in source native gas.
-    function fee(uint256 toChainId, address ua, uint256 gasLimit, bytes calldata encoded, bytes calldata params)
+    function fee(uint256 toChainId, address ua, uint256 gasLimit, bytes calldata encoded)
         external
         view
         returns (uint256);
-
-    /// @notice Assign the relay message task to relayer maintainer.
-    /// @param msgHash Hash of the message.
-    /// @param params General extensibility for relayer to custom functionality.
-    function assign(bytes32 msgHash, bytes calldata params) external payable;
 }

test/ORMP.t.sol

@@ -27,6 +27,8 @@ contract ORMPTest is Test, Verifier {
     Proof proof;
     address immutable self = address(this);
 
+    receive() external payable {}
+
     function setUp() public {
         vm.chainId(1);
         ormp = new ORMP(self);
@@ -108,10 +110,7 @@ contract ORMPTest is Test, Verifier {
         return 2;
     }
 
-    function assign(bytes32) external payable {}
-    function assign(bytes32, bytes calldata) external payable {}
-
-    function fee(uint256, address, uint256, bytes calldata, bytes calldata) external pure returns (uint256) {
+    function fee(uint256, address, uint256, bytes calldata) external pure returns (uint256) {
         return 1;
     }
 

test/bench/ORMP.b.sol

@@ -85,7 +85,7 @@ contract ORMPBenchmarkTest is Test {
 
     function perform_send(uint256 fromChainId, uint256 toChainId, bytes calldata encoded) public {
         vm.createSelectFork(fromChainId.toChainName());
-        uint256 f = ormp.fee(toChainId, self, 0, encoded, abi.encode(uint256(0)));
-        ormp.send{value: f}(toChainId, self, 0, encoded, self, abi.encode(uint256(0)));
+        uint256 f = ormp.fee(toChainId, self, 0, encoded, "");
+        ormp.send{value: f}(toChainId, self, 0, encoded, self, "");
     }
 }

test/eco/Oracle.t.sol

@@ -66,11 +66,6 @@ contract OracleTest is Test {
         oracle.setFee(1, 1);
     }
 
-    function test_assign() public {
-        oracle.setFee(1, 1);
-        oracle.assign{value: 1}(bytes32(0));
-    }
-
     function test_merkleRoot() public {
         bytes32 r = oracle.merkleRoot(1, 1);
         assertEq(r, bytes32(uint256(1)));

test/eco/Relayer.t.sol

@@ -78,18 +78,10 @@ contract RelayerTest is Test {
     function test_setPrice() public {
         relayer.setDstPrice(1, 10 ** 10, 1);
         relayer.setDstConfig(1, 1, 1);
-        uint256 f = relayer.fee(1, self, 1, hex"00", abi.encode(uint256(1)));
+        uint256 f = relayer.fee(1, self, 1, hex"00");
         assertEq(f, 3);
     }
 
-    function test_assign() public {
-        relayer.setDstPrice(1, 10 ** 10, 1);
-        relayer.setDstConfig(1, 1, 1);
-        uint256 v = relayer.fee(1, address(1), 1, hex"00", abi.encode(uint256(1)));
-        relayer.assign{value: v}(bytes32(0), abi.encode(uint256(1)));
-        assertEq(v, 3);
-    }
-
     function test_relay() public {
         Message memory message = Message({
             channel: address(0xc),