governance minimization and bug fixes

Author: shotaronowhere

contracts/src/bridge/FastBridgeReceiverOnEthereum.sol

@@ -46,36 +46,26 @@ contract FastBridgeReceiverOnEthereum is SafeBridgeReceiverOnEthereum, IFastBrid
     // *             Storage               * //
     // ************************************* //
 
-    uint256 public constant ONE_BASIS_POINT = 1e4; // One basis point, for scaling.
     uint256 public override claimDeposit; // The deposit required to submit a claim.
     uint256 public override challengeDeposit; // The deposit required to submit a challenge.
     uint256 public override challengeDuration; // The duration of the period allowing to challenge a claim.
-    uint256 public override alpha; // Basis point of claim or challenge deposit that are lost when dishonest.
     mapping(uint256 => Ticket) public tickets; // The tickets by ticketID.
-
     /**
      * @dev Constructor.
-     * @param _governor The governor's address.
-     * @param _safeBridgeSender The address of the Safe Bridge sender on Arbitrum.
      * @param _inbox The address of the Arbitrum Inbox contract.
      * @param _claimDeposit The deposit amount to submit a claim in wei.
      * @param _challengeDeposit The deposit amount to submit a challenge in wei.
      * @param _challengeDuration The duration of the period allowing to challenge a claim.
-     * @param _alpha Basis point of claim or challenge deposit that are lost when dishonest.
      */
     constructor(
-        address _governor,
-        address _safeBridgeSender,
         address _inbox,
         uint256 _claimDeposit,
         uint256 _challengeDeposit,
-        uint256 _challengeDuration,
-        uint256 _alpha
-    ) SafeBridgeReceiverOnEthereum(_governor, _safeBridgeSender, _inbox) {
+        uint256 _challengeDuration
+    ) SafeBridgeReceiverOnEthereum(_inbox) {
         claimDeposit = _claimDeposit;
         challengeDeposit = _challengeDeposit;
         challengeDuration = _challengeDuration;
-        alpha = _alpha;
     }
 
     // ************************************* //
@@ -127,18 +117,18 @@ contract FastBridgeReceiverOnEthereum is SafeBridgeReceiverOnEthereum, IFastBrid
     /**
      * @dev Relay the message for this `ticketID` if the challenge period has passed and the claim is unchallenged. The hash computed over `messageData` and the other parameters must match the hash provided by the claim.
      * @param _ticketID The ticket identifier referring to a message going through the bridge.
-     * @param _blockNumber The block number on the cross-domain chain when the message with this ticketID has been sent.
+     * @param _blocknumber The block number on the cross-domain chain when the message with this ticketID has been sent.
      * @param _messageData The data on the cross-domain chain for the message sent with this ticketID.
      */
     function verifyAndRelay(
         uint256 _ticketID,
-        uint256 _blockNumber,
+        uint256 _blocknumber,
         bytes calldata _messageData
     ) external override {
         Ticket storage ticket = tickets[_ticketID];
         require(ticket.claim.bridger != address(0), "Claim does not exist");
         require(
-            ticket.claim.messageHash == keccak256(abi.encode(_ticketID, _blockNumber, _messageData)),
+            ticket.claim.messageHash == keccak256(abi.encode(_ticketID, _blocknumber, _messageData)),
             "Invalid hash"
         );
         require(ticket.claim.claimedAt + challengeDuration < block.timestamp, "Challenge period not over");
@@ -154,12 +144,12 @@ contract FastBridgeReceiverOnEthereum is SafeBridgeReceiverOnEthereum, IFastBrid
      * Note: Access restricted to the Safe Bridge.
      * @dev Relay the message for this `ticketID` as provided by the Safe Bridge. Resolve a challenged claim for this `ticketID` if any.
      * @param _ticketID The ticket identifier referring to a message going through the bridge.
-     * @param _blockNumber The block number on the cross-domain chain when the message with this ticketID has been sent.
+     * @param _blocknumber The block number on the cross-domain chain when the message with this ticketID has been sent.
      * @param _messageData The data on the cross-domain chain for the message sent with this ticketID.
      */
     function verifyAndRelaySafe(
         uint256 _ticketID,
-        uint256 _blockNumber,
+        uint256 _blocknumber,
         bytes calldata _messageData
     ) external override {
         require(isSentBySafeBridge(), "Access not allowed: SafeBridgeSender only.");
@@ -168,7 +158,7 @@ contract FastBridgeReceiverOnEthereum is SafeBridgeReceiverOnEthereum, IFastBrid
         require(ticket.relayed == false, "Message already relayed");
 
         // Claim assessment if any
-        bytes32 messageHash = keccak256(abi.encode(_ticketID, _blockNumber, _messageData));
+        bytes32 messageHash = keccak256(abi.encode(_ticketID, _blocknumber, _messageData));
         if (ticket.claim.bridger != address(0) && ticket.claim.messageHash == messageHash) {
             ticket.claim.verified = true;
         }
@@ -187,7 +177,7 @@ contract FastBridgeReceiverOnEthereum is SafeBridgeReceiverOnEthereum, IFastBrid
         require(ticket.claim.bridger != address(0), "Claim does not exist");
         require(ticket.claim.verified == true, "Claim not verified: deposit forfeited");
 
-        uint256 amount = ticket.claim.claimDeposit + (ticket.challenge.challengeDeposit * alpha) / ONE_BASIS_POINT;
+        uint256 amount = ticket.claim.claimDeposit + ticket.challenge.challengeDeposit / 2;
         ticket.claim.claimDeposit = 0;
         ticket.challenge.challengeDeposit = 0;
         payable(ticket.claim.bridger).send(amount); // Use of send to prevent reverting fallback. User is responsibility for accepting ETH.
@@ -204,7 +194,7 @@ contract FastBridgeReceiverOnEthereum is SafeBridgeReceiverOnEthereum, IFastBrid
         require(ticket.challenge.challenger != address(0), "Challenge does not exist");
         require(ticket.claim.verified == false, "Claim verified: deposit forfeited");
 
-        uint256 amount = ticket.challenge.challengeDeposit + (ticket.claim.claimDeposit * alpha) / ONE_BASIS_POINT;
+        uint256 amount = ticket.challenge.challengeDeposit + ticket.claim.claimDeposit / 2;
         ticket.claim.claimDeposit = 0;
         ticket.challenge.challengeDeposit = 0;
         payable(ticket.challenge.challenger).send(amount); // Use of send to prevent reverting fallback. User is responsibility for accepting ETH.
@@ -229,26 +219,6 @@ contract FastBridgeReceiverOnEthereum is SafeBridgeReceiverOnEthereum, IFastBrid
         return (start, end);
     }
 
-    // ************************ //
-    // *      Governance      * //
-    // ************************ //
-
-    function changeClaimDeposit(uint256 _claimDeposit) external onlyByGovernor {
-        claimDeposit = _claimDeposit;
-    }
-
-    function changeChallengeDeposit(uint256 _challengeDeposit) external onlyByGovernor {
-        challengeDeposit = _challengeDeposit;
-    }
-
-    function changeChallengePeriodDuration(uint256 _challengeDuration) external onlyByGovernor {
-        challengeDuration = _challengeDuration;
-    }
-
-    function changeAlpha(uint256 _alpha) external onlyByGovernor {
-        alpha = _alpha;
-    }
-
     // ************************ //
     // *       Internal       * //
     // ************************ //

contracts/src/bridge/FastBridgeSenderToEthereum.sol

@@ -25,105 +25,88 @@ contract FastBridgeSenderToEthereum is SafeBridgeSenderToEthereum, IFastBridgeSe
 
     struct Ticket {
         bytes32 messageHash;
-        uint256 blockNumber;
+        uint256 blocknumber;
         bool sentSafe;
     }
 
     // ************************************* //
     // *             Storage               * //
     // ************************************* //
 
-    address public governor; // The governor of the contract.
     IFastBridgeReceiver public fastBridgeReceiver; // The address of the Fast Bridge on Ethereum.
-    address public fastBridgeSender; // The address of the Fast Bridge sender on Arbitrum, generally the Home Gateway.
     uint256 public currentTicketID = 1; // Zero means not set, start at 1.
     mapping(uint256 => Ticket) public tickets; // The tickets by ticketID.
 
-    // ************************************* //
-    // *        Function Modifiers         * //
-    // ************************************* //
-
-    modifier onlyByGovernor() {
-        require(governor == msg.sender, "Access not allowed: Governor only.");
-        _;
-    }
-
     /**
      * @dev Constructor.
-     * @param _governor The governor's address.
      * @param _fastBridgeReceiver The address of the Fast Bridge on Ethereum.
-     * @param _fastBridgeSender The address of the Fast Bridge sender on Arbitrum, generally the Home Gateway.
      */
     constructor(
-        address _governor,
-        IFastBridgeReceiver _fastBridgeReceiver,
-        address _fastBridgeSender
+        IFastBridgeReceiver _fastBridgeReceiver
     ) SafeBridgeSenderToEthereum() {
-        governor = _governor;
         fastBridgeReceiver = _fastBridgeReceiver;
-        fastBridgeSender = _fastBridgeSender;
     }
 
     // ************************************* //
     // *         State Modifiers           * //
     // ************************************* //
 
     /**
-     * Note: Access restricted to the `fastSender`, generally the Gateway.
      * @dev Sends an arbitrary message to Ethereum using the Fast Bridge.
      * @param _receiver The address of the contract on Ethereum which receives the calldata.
      * @param _calldata The receiving domain encoded message data.
      * @return ticketID The identifier to provide to sendSafeFallback().
      */
     function sendFast(address _receiver, bytes memory _calldata) external override returns (uint256 ticketID) {
-        require(msg.sender == fastBridgeSender, "Access not allowed: Fast Sender only.");
-
+        require(_calldata.length >= 4, "Malformed calldata: lacks function selector.");
         ticketID = currentTicketID++;
 
-        (bytes32 messageHash, bytes memory messageData) = _encode(ticketID, _receiver, _calldata);
-        emit OutgoingMessage(ticketID, block.number, _receiver, messageHash, messageData);
+        (bytes32 messageHash, ) = _encode(
+            ticketID, 
+            block.number, 
+            msg.sender, 
+            _receiver, 
+            _calldata);
+        emit OutgoingMessage(ticketID, block.number, msg.sender, _receiver, messageHash, _calldata);
 
-        tickets[ticketID] = Ticket({messageHash: messageHash, blockNumber: block.number, sentSafe: false});
+        tickets[ticketID] = Ticket({messageHash: messageHash, blocknumber: block.number, sentSafe: false});
     }
 
+
     /**
      * @dev Sends an arbitrary message to Ethereum using the Safe Bridge, which relies on Arbitrum's canonical bridge. It is unnecessary during normal operations but essential only in case of challenge.
      * @param _ticketID The ticketID as returned by `sendFast()`.
+     * @param _fastMsgSender The msg.sender which called sendFast() to register this ticketID.
      * @param _receiver The address of the contract on Ethereum which receives the calldata.
      * @param _calldata The receiving domain encoded message data.
      */
     function sendSafeFallback(
         uint256 _ticketID,
+        address _fastBridgeReceiver,
+        address _fastMsgSender,
         address _receiver,
         bytes memory _calldata
     ) external payable override {
         // TODO: check if keeping _calldata in storage in sendFast() is cheaper than passing it again as a parameter here
+        // However calldata gas cost-benefit analysis will change with EIP-4488.
         Ticket storage ticket = tickets[_ticketID];
         require(ticket.messageHash != 0, "Ticket does not exist.");
         require(ticket.sentSafe == false, "Ticket already sent safely.");
 
-        (bytes32 messageHash, bytes memory messageData) = _encode(_ticketID, _receiver, _calldata);
+        (bytes32 messageHash, bytes memory messageData) = _encode(_ticketID, ticket.blocknumber, _fastMsgSender, _receiver, _calldata);
         require(ticket.messageHash == messageHash, "Invalid message for ticketID.");
 
         // Safe Bridge message envelope
         bytes4 methodSelector = IFastBridgeReceiver.verifyAndRelaySafe.selector;
         bytes memory safeMessageData = abi.encodeWithSelector(
             methodSelector,
             _ticketID,
-            ticket.blockNumber,
+            ticket.blocknumber,
             messageData
         );
-
+        ticket.sentSafe = true;
         // TODO: how much ETH should be provided for bridging? add an ISafeBridgeSender.bridgingCost() if needed
-        _sendSafe(address(fastBridgeReceiver), safeMessageData);
-    }
-
-    // ************************ //
-    // *      Governance      * //
-    // ************************ //
-
-    function changeFastSender(address _fastBridgeSender) external onlyByGovernor {
-        fastBridgeSender = _fastBridgeSender;
+        _sendSafe(address(_fastBridgeReceiver), safeMessageData);
     }
 
     // ************************ //
@@ -132,13 +115,16 @@ contract FastBridgeSenderToEthereum is SafeBridgeSenderToEthereum, IFastBridgeSe
 
     function _encode(
         uint256 _ticketID,
+        uint256 _blocknumber,
+        address _msgSender,
         address _receiver,
         bytes memory _calldata
-    ) internal view returns (bytes32 messageHash, bytes memory messageData) {
-        // Encode the receiver address with the function signature + arguments i.e calldata
-        messageData = abi.encode(_receiver, _calldata);
+    ) internal pure returns (bytes32 messageHash, bytes memory messageData) {
+        // Encode the receiver address with the function signature + _msgSender as the first argument, then the rest of the args
+        (bytes4 functionSelector, bytes memory _args) = abi.decode(_calldata, (bytes4, bytes));
+        messageData = abi.encode(_receiver, abi.encodeWithSelector(functionSelector, _msgSender, _args));
 
         // Compute the hash over the message header (ticketID, blockNumber) and body (data).
-        messageHash = keccak256(abi.encode(_ticketID, block.number, messageData));
+        messageHash = keccak256(abi.encode(_ticketID, _blocknumber, messageData));
     }
 }

contracts/src/bridge/SafeBridgeReceiverOnEthereum.sol

@@ -23,35 +23,24 @@ contract SafeBridgeReceiverOnEthereum is ISafeBridgeReceiver {
     // *             Storage               * //
     // ************************************* //
 
-    address public governor; // The governor of the contract.
     address public safeBridgeSender; // The address of the Safe Bridge sender on Arbitrum.
     IInbox public inbox; // The address of the Arbitrum Inbox contract.
 
-    // ************************************* //
-    // *        Function Modifiers         * //
-    // ************************************* //
-
-    modifier onlyByGovernor() {
-        require(governor == msg.sender, "Access not allowed: Governor only.");
-        _;
-    }
-
     /**
      * @dev Constructor.
-     * @param _governor The governor's address.
      * @param _safeBridgeSender The address of the Safe Bridge sender on Arbitrum.
      * @param _inbox The address of the Arbitrum Inbox contract.
      */
     constructor(
-        address _governor,
-        address _safeBridgeSender,
         address _inbox
     ) {
-        governor = _governor;
         inbox = IInbox(_inbox);
-        safeBridgeSender = _safeBridgeSender;
     }
 
+    function setSafeBridgeSender(address _safeBridgeSender) external {
+        if (safeBridgeSender == address(0) )
+            safeBridgeSender = _safeBridgeSender;
+    }
     // ************************************* //
     // *              Views                * //
     // ************************************* //
@@ -60,16 +49,4 @@ contract SafeBridgeReceiverOnEthereum is ISafeBridgeReceiver {
         IOutbox outbox = IOutbox(inbox.bridge().activeOutbox());
         return outbox.l2ToL1Sender() == safeBridgeSender;
     }
-
-    // ************************ //
-    // *      Governance      * //
-    // ************************ //
-
-    function setSafeBridgeSender(address _safeBridgeSender) external onlyByGovernor {
-        safeBridgeSender = _safeBridgeSender;
-    }
-
-    function setInbox(address _inbox) external onlyByGovernor {
-        inbox = IInbox(_inbox);
-    }
 }

contracts/src/bridge/interfaces/IFastBridgeReceiver.sol

@@ -109,9 +109,4 @@ interface IFastBridgeReceiver {
      * @return amount The duration of the period allowing to challenge a claim.
      */
     function challengeDuration() external view returns (uint256 amount);
-
-    /**
-     * @return amount Basis point of claim or challenge deposit that are lost when dishonest.
-     */
-    function alpha() external view returns (uint256 amount);
 }

contracts/src/bridge/interfaces/IFastBridgeSender.sol

@@ -11,13 +11,15 @@ interface IFastBridgeSender {
      * @dev The Fast Bridge participants need to watch for these events and relay the messageHash on the FastBridgeReceiverOnEthereum.
      * @param ticketID The ticket identifier referring to a message going through the bridge.
      * @param blockNumber The block number when the message with this ticketID has been created.
+     * @param messageSender The address of the cross-domain receiver of the message, generally the Foreign Gateway.
      * @param target The address of the cross-domain receiver of the message, generally the Foreign Gateway.
      * @param messageHash The hash uniquely identifying this message.
      * @param message The message data.
      */
     event OutgoingMessage(
         uint256 indexed ticketID,
         uint256 blockNumber,
+        address messageSender,
         address target,
         bytes32 indexed messageHash,
         bytes message
@@ -39,11 +41,15 @@ interface IFastBridgeSender {
     /**
      * @dev Sends an arbitrary message across domain using the Safe Bridge, which relies on the chain's canonical bridge. It is unnecessary during normal operations but essential only in case of challenge.
      * @param _ticketID The ticketID as returned by `sendFast()`.
+     * @param _fastBridgeReceiver The address of the fast bridge receiver deployment.
+     * @param _fastMsgSender The msg.sender which called sendFast()
      * @param _receiver The cross-domain contract address which receives the calldata.
      * @param _calldata The receiving domain encoded message data.
      */
     function sendSafeFallback(
         uint256 _ticketID,
+        address _fastBridgeReceiver,
+        address _fastMsgSender,
         address _receiver,
         bytes memory _calldata
     ) external payable;