docs: added NatSpecs for the bridge contracts, moved the events to the interface level

jaybuidl · jaybuidl · commit 4e393e30aecd · 2022-04-07T21:44:39.000+01:00
diff --git a/contracts/deploy/02-home-chain.ts b/contracts/deploy/02-home-chain.ts
@@ -22,7 +22,7 @@ const deployHomeGateway: DeployFunction = async (hre: HardhatRuntimeEnvironment)
       : await hre.companionNetworks.foreign.deployments.get("FastBridgeReceiverOnEthereum");
   const fastBridgeSender = await deploy("FastBridgeSenderToEthereum", {
     from: deployer,
-    args: [deployer, fastBridgeReceiver.address],
+    args: [deployer, fastBridgeReceiver.address, ethers.constants.AddressZero],
     log: true,
   }); // nonce+0
 
@@ -40,7 +40,7 @@ const deployHomeGateway: DeployFunction = async (hre: HardhatRuntimeEnvironment)
 
   const fastSender = await hre.ethers
     .getContractAt("FastBridgeSenderToEthereum", fastBridgeSender.address)
-    .then((contract) => contract.fastSender());
+    .then((contract) => contract.fastBridgeSender());
   if (fastSender === ethers.constants.AddressZero) {
     await execute("FastBridgeSenderToEthereum", { from: deployer, log: true }, "changeFastSender", homeGateway.address);
   }
diff --git a/contracts/src/bridge/FastBridgeReceiverOnEthereum.sol b/contracts/src/bridge/FastBridgeReceiverOnEthereum.sol
@@ -47,19 +47,22 @@ contract FastBridgeReceiverOnEthereum is SafeBridgeReceiverOnEthereum, IFastBrid
     // ************************************* //
 
     uint256 public constant ONE_BASIS_POINT = 1e4; // One basis point, for scaling.
-    uint256 public override claimDeposit;
-    uint256 public override challengeDeposit;
-    uint256 public override challengeDuration;
+    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.
 
-    // ************************************* //
-    // *              Events               * //
-    // ************************************* //
-
-    event ClaimReceived(uint256 indexed _ticketID, bytes32 indexed messageHash, uint256 claimedAt);
-    event ClaimChallenged(uint256 indexed _ticketID, bytes32 indexed _messageHash, uint256 challengedAt);
-
+    /**
+     * @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,
@@ -79,6 +82,11 @@ contract FastBridgeReceiverOnEthereum is SafeBridgeReceiverOnEthereum, IFastBrid
     // *         State Modifiers           * //
     // ************************************* //
 
+    /**
+     * @dev Submit a claim about the `messageHash` for a particular Fast Bridge `ticketID` and submit a deposit. The `messageHash` should match the one on the sending side otherwise the sender will lose his deposit.
+     * @param _ticketID The ticket identifier referring to a message going through the bridge.
+     * @param _messageHash The hash claimed for the ticket.
+     */
     function claim(uint256 _ticketID, bytes32 _messageHash) external payable override {
         Ticket storage ticket = tickets[_ticketID];
         require(ticket.claim.bridger == address(0), "Claim already made");
@@ -96,6 +104,10 @@ contract FastBridgeReceiverOnEthereum is SafeBridgeReceiverOnEthereum, IFastBrid
         emit ClaimReceived(_ticketID, _messageHash, block.timestamp);
     }
 
+    /**
+     * @dev Submit a challenge for a particular Fast Bridge `ticketID` and submit a deposit. The `messageHash` in the claim already made for this `ticketID` should be different from the one on the sending side, otherwise the sender will lose his deposit.
+     * @param _ticketID The ticket identifier referring to a message going through the bridge.
+     */
     function challenge(uint256 _ticketID) external payable override {
         Ticket storage ticket = tickets[_ticketID];
         require(ticket.claim.bridger != address(0), "Claim does not exist");
@@ -109,18 +121,24 @@ contract FastBridgeReceiverOnEthereum is SafeBridgeReceiverOnEthereum, IFastBrid
             challengeDeposit: msg.value
         });
 
-        emit ClaimChallenged(_ticketID, ticket.claim.messageHash, block.timestamp);
+        emit ClaimChallenged(_ticketID, block.timestamp);
     }
 
+    /**
+     * @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 _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");
@@ -132,9 +150,16 @@ contract FastBridgeReceiverOnEthereum is SafeBridgeReceiverOnEthereum, IFastBrid
         require(_relay(_messageData), "Failed to call contract"); // Checks-Effects-Interaction
     }
 
+    /**
+     * 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 _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.");
@@ -143,7 +168,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;
         }
@@ -152,6 +177,10 @@ contract FastBridgeReceiverOnEthereum is SafeBridgeReceiverOnEthereum, IFastBrid
         require(_relay(_messageData), "Failed to call contract"); // Checks-Effects-Interaction
     }
 
+    /**
+     * @dev Sends the deposit back to the Bridger if his claim is not successfully challenged. Includes a portion of the Challenger's deposit if unsuccessfully challenged.
+     * @param _ticketID The ticket identifier referring to a message going through the bridge.
+     */
     function withdrawClaimDeposit(uint256 _ticketID) external override {
         Ticket storage ticket = tickets[_ticketID];
         require(ticket.relayed == true, "Message not relayed yet");
@@ -165,6 +194,10 @@ contract FastBridgeReceiverOnEthereum is SafeBridgeReceiverOnEthereum, IFastBrid
         // Checks-Effects-Interaction
     }
 
+    /**
+     * @dev Sends the deposit back to the Challenger if his challenge is successful. Includes a portion of the Bridger's deposit.
+     * @param _ticketID The ticket identifier referring to a message going through the bridge.
+     */
     function withdrawChallengeDeposit(uint256 _ticketID) external override {
         Ticket storage ticket = tickets[_ticketID];
         require(ticket.relayed == true, "Message not relayed");
@@ -182,7 +215,12 @@ contract FastBridgeReceiverOnEthereum is SafeBridgeReceiverOnEthereum, IFastBrid
     // *           Public Views            * //
     // ************************************* //
 
-    function challengePeriod(uint256 _ticketID) public view returns (uint256 start, uint256 end) {
+    /**
+     * @dev Returns the `start` and `end` time of challenge period for this `ticketID`.
+     * @return start The start time of the challenge period.
+     * @return end The end time of the challenge period.
+     */
+    function challengePeriod(uint256 _ticketID) external view override returns (uint256 start, uint256 end) {
         Ticket storage ticket = tickets[_ticketID];
         require(ticket.claim.bridger != address(0), "Claim does not exist");
 
diff --git a/contracts/src/bridge/FastBridgeSenderToEthereum.sol b/contracts/src/bridge/FastBridgeSenderToEthereum.sol
@@ -33,28 +33,12 @@ contract FastBridgeSenderToEthereum is SafeBridgeSenderToEthereum, IFastBridgeSe
     // *             Storage               * //
     // ************************************* //
 
-    address public governor;
-    IFastBridgeReceiver public fastBridgeReceiver;
-    address public fastSender;
+    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.
 
-    // ************************************* //
-    // *              Events               * //
-    // ************************************* //
-
-    /**
-     * The bridgers need to watch for these events and
-     * relay the messageHash on the FastBridgeReceiverOnEthereum.
-     */
-    event OutgoingMessage(
-        uint256 indexed ticketID,
-        uint256 blockNumber,
-        address target,
-        bytes32 indexed messageHash,
-        bytes message
-    );
-
     // ************************************* //
     // *        Function Modifiers         * //
     // ************************************* //
@@ -64,25 +48,35 @@ contract FastBridgeSenderToEthereum is SafeBridgeSenderToEthereum, IFastBridgeSe
         _;
     }
 
-    constructor(address _governor, IFastBridgeReceiver _fastBridgeReceiver) SafeBridgeSenderToEthereum() {
+    /**
+     * @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
+    ) SafeBridgeSenderToEthereum() {
         governor = _governor;
         fastBridgeReceiver = _fastBridgeReceiver;
+        fastBridgeSender = _fastBridgeSender;
     }
 
     // ************************************* //
     // *         State Modifiers           * //
     // ************************************* //
 
     /**
-     * Sends an arbitrary message from one domain to another
-     * via the fast bridge mechanism
-     *
-     * @param _receiver The L1 contract address who will receive the calldata
+     * 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()
+     * @return ticketID The identifier to provide to sendSafeFallback().
      */
     function sendFast(address _receiver, bytes memory _calldata) external override returns (uint256 ticketID) {
-        require(msg.sender == fastSender, "Access not allowed: Fast Sender only.");
+        require(msg.sender == fastBridgeSender, "Access not allowed: Fast Sender only.");
 
         ticketID = currentTicketID++;
 
@@ -93,25 +87,17 @@ contract FastBridgeSenderToEthereum is SafeBridgeSenderToEthereum, IFastBridgeSe
     }
 
     /**
-     * Sends an arbitrary message from one domain to another
-     * via the safe bridge mechanism, which relies on the chain's native bridge.
-     *
-     * It is unnecessary during normal operations but essential only in case of challenge.
-     *
-     * It may require some ETH (or whichever native token) to pay for the bridging cost,
-     * depending on the underlying safe bridge.
-     *
-     * TODO: check if keeping _calldata in storage in sendFast() is cheaper than passing it again as a parameter here
-     *
-     * @param _ticketID The ticketID as provided by `sendFast()` if any.
-     * @param _receiver The L1 contract address who will receive the calldata
+     * @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 _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 _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
         Ticket storage ticket = tickets[_ticketID];
         require(ticket.messageHash != 0, "Ticket does not exist.");
         require(ticket.sentSafe == false, "Ticket already sent safely.");
@@ -136,9 +122,8 @@ contract FastBridgeSenderToEthereum is SafeBridgeSenderToEthereum, IFastBridgeSe
     // *      Governance      * //
     // ************************ //
 
-    function changeFastSender(address _fastSender) external onlyByGovernor {
-        require(fastSender == address(0));
-        fastSender = _fastSender;
+    function changeFastSender(address _fastBridgeSender) external onlyByGovernor {
+        fastBridgeSender = _fastBridgeSender;
     }
 
     // ************************ //
diff --git a/contracts/src/bridge/SafeBridgeReceiverOnEthereum.sol b/contracts/src/bridge/SafeBridgeReceiverOnEthereum.sol
@@ -23,9 +23,9 @@ contract SafeBridgeReceiverOnEthereum is ISafeBridgeReceiver {
     // *             Storage               * //
     // ************************************* //
 
-    address public governor;
-    address public safeBridgeSender;
-    IInbox public inbox;
+    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         * //
@@ -36,6 +36,12 @@ contract SafeBridgeReceiverOnEthereum is ISafeBridgeReceiver {
         _;
     }
 
+    /**
+     * @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,
diff --git a/contracts/src/bridge/SafeBridgeSenderToEthereum.sol b/contracts/src/bridge/SafeBridgeSenderToEthereum.sol
@@ -20,10 +20,22 @@ import "./interfaces/ISafeBridgeSender.sol";
  * Counterpart of `SafeBridgeReceiverOnEthereum`
  */
 contract SafeBridgeSenderToEthereum is ISafeBridgeSender {
-    IArbSys public constant ARB_SYS = IArbSys(address(100));
+    // ************************************* //
+    // *              Events               * //
+    // ************************************* //
 
     event L2ToL1TxCreated(uint256 indexed withdrawalId);
 
+    // ************************************* //
+    // *             Storage               * //
+    // ************************************* //
+
+    IArbSys public constant ARB_SYS = IArbSys(address(100));
+
+    // ************************************* //
+    // *        Function Modifiers         * //
+    // ************************************* //
+
     function _sendSafe(address _receiver, bytes memory _calldata) internal override returns (uint256) {
         uint256 withdrawalId = ARB_SYS.sendTxToL1(_receiver, _calldata);
 
diff --git a/contracts/src/bridge/interfaces/IFastBridgeReceiver.sol b/contracts/src/bridge/interfaces/IFastBridgeReceiver.sol
diff --git a/contracts/src/bridge/interfaces/IFastBridgeSender.sol b/contracts/src/bridge/interfaces/IFastBridgeSender.sol
