Required gas, events, reentrancy, go linter (#66)

.golangci.yml

@@ -0,0 +1,31 @@
+# This file configures github.com/golangci/golangci-lint.
+
+run:
+  timeout: 3m
+  tests: true
+  # skip auto-generated files.
+  skip-dirs:
+    - "abi-bindings/go"
+  skip-files:
+    - ".*mock.*"
+
+issues:
+  # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
+  max-same-issues: 0
+
+linters:
+  disable-all: true
+  enable:
+    - goimports
+    - gosimple
+    - govet
+    - ineffassign
+    - misspell
+    - unconvert
+    - unused
+    - whitespace
+    - lll
+
+linters-settings:
+  gofmt:
+    simplify: true

contracts/src/ERC20Destination.sol

@@ -60,7 +60,7 @@ contract ERC20Destination is IERC20Bridge, TeleporterTokenDestination, ERC20 {
      *
      * @dev See {IERC20Bridge-send}
      */
-    function send(SendTokensInput calldata input, uint256 amount) external {
+    function send(SendTokensInput calldata input, uint256 amount) external nonReentrant {
         _send(input, amount);
     }
 

contracts/src/ERC20Source.sol

@@ -46,7 +46,7 @@ contract ERC20Source is IERC20Bridge, TeleporterTokenSource {
     /**
      * @dev See {IERC20Bridge-send}
      */
-    function send(SendTokensInput calldata input, uint256 amount) external {
+    function send(SendTokensInput calldata input, uint256 amount) external nonReentrant {
         _send(input, amount, false);
     }
 

contracts/src/NativeTokenSource.sol

@@ -51,7 +51,7 @@ contract NativeTokenSource is INativeTokenBridge, TeleporterTokenSource {
     /**
      * @dev See {INativeTokenBridge-send}
      */
-    function send(SendTokensInput calldata input) external payable {
+    function send(SendTokensInput calldata input) external payable nonReentrant {
         _send(input, msg.value, false);
     }
 

contracts/src/TeleporterTokenDestination.sol

@@ -34,9 +34,11 @@ abstract contract TeleporterTokenDestination is
     /// @notice The ERC20 token this contract uses to pay for Teleporter fees.
     address public immutable feeTokenAddress;
 
-    // TODO: these are values brought from the example ERC20Bridge contract.
-    // Need to figure out appropriate values.
-    uint256 public constant SEND_TOKENS_REQUIRED_GAS = 300_000;
+    /**
+     * @notice Fixed gas cost for performing a multihop transfer on the `sourceBlockchainID`
+     * , before forwarding to the final destination bridge instance.
+     */
+    uint256 public constant MULTIHOP_REQUIRED_GAS = 220_000;
 
     /**
      * @notice Initializes this destination token bridge instance to receive
@@ -84,24 +86,43 @@ abstract contract TeleporterTokenDestination is
      * - `amount` must be greater than `input.primaryFee`
      */
     function _send(SendTokensInput calldata input, uint256 amount) internal virtual {
+        require(
+            input.destinationBlockchainID != bytes32(0),
+            "TeleporterTokenDestination: zero destination blockchain ID"
+        );
         require(
             input.destinationBridgeAddress != address(0),
             "TeleporterTokenDestination: zero destination bridge address"
         );
         require(input.recipient != address(0), "TeleporterTokenDestination: zero recipient address");
 
-        // If the destination blockchain is the source bridge instance's blockchain,
-        // the destination bridge address must match the token source address.
+        // If the destination blockchain is the source blockchain,
+        // no multihop is needed. Only the required gas limit for the Teleporter message back to
+        // `sourceBlockchainID` is needed, which is provided by `input.requiredGasLimit`.
+        // Else, there will be a multihop transfer to the final destination.
+        // The first hop back to `sourceBlockchainID` requires `MULTIHOP_REQUIRED_GAS`,
+        // and the second hop to the final destination requires `input.requiredGasLimit`.
+        uint256 firstHopRequiredGas = input.requiredGasLimit;
+        uint256 secondHopRequiredGas;
         if (input.destinationBlockchainID == sourceBlockchainID) {
+            // If the destination blockchain is the source bridge instance's blockchain,
+            // the destination bridge address must match the token source address,
+            // and no secondary fee is needed.
             require(
                 input.destinationBridgeAddress == tokenSourceAddress,
                 "TeleporterTokenDestination: invalid destination bridge address"
             );
-        } else if (input.destinationBlockchainID == blockchainID) {
-            require(
-                input.destinationBridgeAddress != address(this),
-                "TeleporterTokenDestination: invalid destination bridge address"
-            );
+            require(input.secondaryFee == 0, "TeleporterTokenDestination: non-zero secondary fee");
+        } else {
+            // Do not allow bridging to the same token bridge instance.
+            if (input.destinationBlockchainID == blockchainID) {
+                require(
+                    input.destinationBridgeAddress != address(this),
+                    "TeleporterTokenDestination: invalid destination bridge address"
+                );
+            }
+            firstHopRequiredGas = MULTIHOP_REQUIRED_GAS;
+            secondHopRequiredGas = input.requiredGasLimit;
         }
 
         // Deposit the funds sent from the user to the bridge,
@@ -120,25 +141,23 @@ abstract contract TeleporterTokenDestination is
                 destinationBlockchainID: sourceBlockchainID,
                 destinationAddress: tokenSourceAddress,
                 feeInfo: TeleporterFeeInfo({feeTokenAddress: feeTokenAddress, amount: input.primaryFee}),
-                // TODO: placeholder value
-                requiredGasLimit: SEND_TOKENS_REQUIRED_GAS,
-                allowedRelayerAddresses: input.allowedRelayerAddresses,
+                requiredGasLimit: firstHopRequiredGas,
+                allowedRelayerAddresses: new address[](0),
                 message: abi.encode(
                     SendTokensInput({
                         destinationBlockchainID: input.destinationBlockchainID,
                         destinationBridgeAddress: input.destinationBridgeAddress,
                         recipient: input.recipient,
                         primaryFee: input.secondaryFee,
                         secondaryFee: 0,
-                        // TODO: Does multihop allowed relayer need to be separate parameter?
-                        allowedRelayerAddresses: input.allowedRelayerAddresses
+                        requiredGasLimit: secondHopRequiredGas
                     }),
                     amount
                     )
             })
         );
 
-        emit SendTokens(messageID, msg.sender, amount);
+        emit SendTokens(messageID, msg.sender, input, amount);
     }
 
     /**
@@ -161,6 +180,7 @@ abstract contract TeleporterTokenDestination is
         );
         (address recipient, uint256 amount) = abi.decode(message, (address, uint256));
 
+        emit WithdrawTokens(recipient, amount);
         _withdraw(recipient, amount);
     }
 

contracts/src/TeleporterTokenSource.sol

@@ -40,10 +40,6 @@ abstract contract TeleporterTokenSource is ITeleporterTokenBridge, TeleporterOwn
             => mapping(address destinationBridgeAddress => uint256 balance)
     ) public bridgedBalances;
 
-    // TODO: these are values brought from the example ERC20Bridge contract.
-    // Need to figure out appropriate values.
-    uint256 public constant SEND_TOKENS_REQUIRED_GAS = 300_000;
-
     /**
      * @notice Initializes this source token bridge instance to send
      * tokens to the specified destination chain and token bridge instance.
@@ -76,6 +72,10 @@ abstract contract TeleporterTokenSource is ITeleporterTokenBridge, TeleporterOwn
         uint256 amount,
         bool isMultihop
     ) internal virtual {
+        require(
+            input.destinationBlockchainID != bytes32(0),
+            "TeleporterTokenSource: zero destination blockchain ID"
+        );
         require(
             input.destinationBlockchainID != blockchainID,
             "TeleporterTokenSource: cannot bridge to same chain"
@@ -85,6 +85,7 @@ abstract contract TeleporterTokenSource is ITeleporterTokenBridge, TeleporterOwn
             "TeleporterTokenSource: zero destination bridge address"
         );
         require(input.recipient != address(0), "TeleporterTokenSource: zero recipient address");
+        require(input.secondaryFee == 0, "TeleporterTokenSource: non-zero secondary fee");
 
         // If this send is not a multihop, deposit the funds sent from the user to the bridge,
         // and set to adjusted amount after deposit. If it is a multihop, the amount is already
@@ -106,14 +107,13 @@ abstract contract TeleporterTokenSource is ITeleporterTokenBridge, TeleporterOwn
                 destinationBlockchainID: input.destinationBlockchainID,
                 destinationAddress: input.destinationBridgeAddress,
                 feeInfo: TeleporterFeeInfo({feeTokenAddress: feeTokenAddress, amount: input.primaryFee}),
-                // TODO: Set requiredGasLimit
-                requiredGasLimit: SEND_TOKENS_REQUIRED_GAS,
-                allowedRelayerAddresses: input.allowedRelayerAddresses,
+                requiredGasLimit: input.requiredGasLimit,
+                allowedRelayerAddresses: new address[](0),
                 message: abi.encode(input.recipient, amount)
             })
         );
 
-        emit SendTokens(messageID, msg.sender, amount);
+        emit SendTokens(messageID, msg.sender, input, amount);
     }
 
     /**
@@ -150,6 +150,7 @@ abstract contract TeleporterTokenSource is ITeleporterTokenBridge, TeleporterOwn
                 input.destinationBridgeAddress == address(this),
                 "TeleporterTokenSource: invalid bridge address"
             );
+            emit WithdrawTokens(input.recipient, amount);
             _withdraw(input.recipient, amount);
             return;
         }

contracts/src/interfaces/ITeleporterTokenBridge.sol

@@ -18,16 +18,18 @@ import {ITeleporterReceiver} from "@teleporter/ITeleporterReceiver.sol";
  * @param destinationBridgeAddress address of the destination token bridge instance
  * @param recipient address of the recipient on the destination chain
  * @param primaryFee amount of tokens to pay for Teleporter fee on the source chain
- * @param secondaryFee amount of tokens to pay for Teleporter fee if a multihop is needed.
- * @param allowedRelayerAddresses addresses of relayers allowed to send the message
+ * @param secondaryFee amount of tokens to pay for Teleporter fee if a multihop is needed
+ * @param requiredGasLimit gas limit requirement for sending to a token bridge.
+ * This is required because the gas requirement varies based on the token bridge instance
+ * specified by `destinationBlockchainID` and `destinationBridgeAddress`.
  */
 struct SendTokensInput {
     bytes32 destinationBlockchainID;
     address destinationBridgeAddress;
     address recipient;
     uint256 primaryFee;
     uint256 secondaryFee;
-    address[] allowedRelayerAddresses;
+    uint256 requiredGasLimit;
 }
 
 /**
@@ -36,7 +38,16 @@ struct SendTokensInput {
 interface ITeleporterTokenBridge is ITeleporterReceiver {
     /**
      * @notice Emitted when tokens are sent to another chain.
-     * TODO: might want to add SendTokensInput as a parameter
      */
-    event SendTokens(bytes32 indexed teleporterMessageID, address indexed sender, uint256 amount);
+    event SendTokens(
+        bytes32 indexed teleporterMessageID,
+        address indexed sender,
+        SendTokensInput input,
+        uint256 amount
+    );
+
+    /**
+     * @notice Emitted when tokens are withdrawn from the token bridge contract.
+     */
+    event WithdrawTokens(address indexed recipient, uint256 amount);
 }

contracts/test/TeleporterTokenBridgeTests.t.sol

@@ -29,6 +29,7 @@ abstract contract TeleporterTokenBridgeTest is Test {
     address public constant TOKEN_SOURCE_ADDRESS = 0xd54e3E251b9b0EEd3ed70A858e927bbC2659587d;
     address public constant DEFAULT_RECIPIENT_ADDRESS = 0xABCDabcdABcDabcDaBCDAbcdABcdAbCdABcDABCd;
     address public constant WARP_PRECOMPILE_ADDRESS = 0x0200000000000000000000000000000000000005;
+    uint256 public constant DEFAULT_REQUIRED_GAS_LIMIT = 100_000;
 
     address public constant MOCK_TELEPORTER_MESSENGER_ADDRESS =
         0x644E5b7c5D4Bc8073732CEa72c66e0BB90dFC00f;
@@ -40,10 +41,24 @@ abstract contract TeleporterTokenBridgeTest is Test {
     ITeleporterTokenBridge public tokenBridge;
     IERC20 public feeToken;
 
-    event SendTokens(bytes32 indexed teleporterMessageID, address indexed sender, uint256 amount);
+    event SendTokens(
+        bytes32 indexed teleporterMessageID,
+        address indexed sender,
+        SendTokensInput input,
+        uint256 amount
+    );
+
+    event WithdrawTokens(address indexed recipient, uint256 amount);
 
     event Transfer(address indexed from, address indexed to, uint256 value);
 
+    function testZeroDestinationBlockchainID() public {
+        SendTokensInput memory input = _createDefaultSendTokensInput();
+        input.destinationBlockchainID = bytes32(0);
+        vm.expectRevert(_formatErrorMessage("zero destination blockchain ID"));
+        _send(input, 0);
+    }
+
     function testZeroDestinationBridge() public {
         SendTokensInput memory input = _createDefaultSendTokensInput();
         input.destinationBridgeAddress = address(0);
@@ -120,7 +135,7 @@ abstract contract TeleporterTokenBridgeTest is Test {
 
         _checkExpectedTeleporterCalls(input, bridgedAmount);
         vm.expectEmit(true, true, true, true, address(tokenBridge));
-        emit SendTokens(_MOCK_MESSAGE_ID, address(this), bridgedAmount);
+        emit SendTokens(_MOCK_MESSAGE_ID, address(this), input, bridgedAmount);
         _send(input, amount);
     }
 
@@ -136,8 +151,8 @@ abstract contract TeleporterTokenBridgeTest is Test {
             destinationBlockchainID: input.destinationBlockchainID,
             destinationAddress: input.destinationBridgeAddress,
             feeInfo: TeleporterFeeInfo({feeTokenAddress: address(feeToken), amount: input.primaryFee}),
-            requiredGasLimit: _requiredGasLimit(),
-            allowedRelayerAddresses: input.allowedRelayerAddresses,
+            requiredGasLimit: input.requiredGasLimit,
+            allowedRelayerAddresses: new address[](0),
             message: _encodeMessage(input, bridgeAmount)
         });
 
@@ -161,8 +176,6 @@ abstract contract TeleporterTokenBridgeTest is Test {
         );
     }
 
-    function _requiredGasLimit() internal view virtual returns (uint256);
-
     function _createDefaultSendTokensInput()
         internal
         pure

contracts/test/TeleporterTokenDestinationTests.t.sol

@@ -33,13 +33,20 @@ abstract contract TeleporterTokenDestinationTest is TeleporterTokenBridgeTest {
         );
     }
 
-    function testInvalidSendingBackToSourceBlockchain() public {
+    function testInvalidSourceBlockchainBridgeAddress() public {
         SendTokensInput memory input = _createDefaultSendTokensInput();
         input.destinationBridgeAddress = address(this);
         vm.expectRevert(_formatErrorMessage("invalid destination bridge address"));
         _send(input, 0);
     }
 
+    function testNonZeroSecondaryFeeToSourceBlockchain() public {
+        SendTokensInput memory input = _createDefaultSendTokensInput();
+        input.secondaryFee = 1;
+        vm.expectRevert(_formatErrorMessage("non-zero secondary fee"));
+        _send(input, 0);
+    }
+
     function testSendingToSameInstance() public {
         SendTokensInput memory input = _createDefaultSendTokensInput();
         input.destinationBlockchainID = tokenDestination.blockchainID();
@@ -85,6 +92,8 @@ abstract contract TeleporterTokenDestinationTest is TeleporterTokenBridgeTest {
     function testReceiveWithdrawSuccess() public {
         uint256 amount = 2;
         vm.prank(MOCK_TELEPORTER_MESSENGER_ADDRESS);
+        vm.expectEmit(true, true, true, true, address(tokenDestination));
+        emit WithdrawTokens(DEFAULT_RECIPIENT_ADDRESS, amount);
         _checkExpectedWithdrawal(DEFAULT_RECIPIENT_ADDRESS, amount);
         tokenDestination.receiveTeleporterMessage(
             DEFAULT_SOURCE_BLOCKCHAIN_ID,
@@ -93,10 +102,6 @@ abstract contract TeleporterTokenDestinationTest is TeleporterTokenBridgeTest {
         );
     }
 
-    function _requiredGasLimit() internal view virtual override returns (uint256) {
-        return tokenDestination.SEND_TOKENS_REQUIRED_GAS();
-    }
-
     function _createDefaultSendTokensInput()
         internal
         pure
@@ -109,7 +114,7 @@ abstract contract TeleporterTokenDestinationTest is TeleporterTokenBridgeTest {
             recipient: DEFAULT_RECIPIENT_ADDRESS,
             primaryFee: 0,
             secondaryFee: 0,
-            allowedRelayerAddresses: new address[](0)
+            requiredGasLimit: 0
         });
     }
 
@@ -133,7 +138,7 @@ abstract contract TeleporterTokenDestinationTest is TeleporterTokenBridgeTest {
                 recipient: input.recipient,
                 primaryFee: input.secondaryFee,
                 secondaryFee: 0,
-                allowedRelayerAddresses: input.allowedRelayerAddresses
+                requiredGasLimit: input.requiredGasLimit
             }),
             amount
         );