feedback

taikoxyz · Feb 12, 2024 · e3a6b97 · e3a6b97
1 parent 7984c2d
commit e3a6b97
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 39 deletions.
diff --git a/packages/protocol/contracts/bridge/Bridge.sol b/packages/protocol/contracts/bridge/Bridge.sol
@@ -358,10 +358,9 @@ contract Bridge is EssentialContract, IBridge {
         whenNotPaused
         sameChain(message.destChainId)
     {
-        // If isLastAttempt is true, the caller must be the message.owner.
-        // TODO(Brecht): why not allow anyone to call when message.gasLimit == 0?
-        // There is no fee to be earned, and there won't be any gas limit anyway.
-        if (isLastAttempt) {
+        // If the gasLimit is set to 0 or isLastAttempt is true, the caller must
+        // be the message.owner.
+        if (message.gasLimit == 0 || isLastAttempt) {
             if (msg.sender != message.owner) revert B_PERMISSION_DENIED();
         }
 
@@ -544,37 +543,13 @@ contract Bridge is EssentialContract, IBridge {
         }
     }
 
-    /// @notice Checks if the signal was received.
-    /// @param signalService The signalService
-    /// @param signal The signal.
-    /// @param chainId The ID of the chain the signal is stored on
-    /// @param proof The merkle inclusion proof.
-    /// @return True if the message was received.
-    function _proveSignalReceived(
-        address signalService,
-        bytes32 signal,
-        uint64 chainId,
-        bytes calldata proof
-    )
-        private
-        view
-        returns (bool)
-    {
-        bytes memory data = abi.encodeCall(
-            ISignalService.proveSignalReceived,
-            (chainId, resolve(chainId, "bridge", false), signal, proof)
-        );
-        (bool success, bytes memory ret) = signalService.staticcall(data);
-        return success ? abi.decode(ret, (bool)) : false;
-    }
-
     /// @notice Resets the call context
-    function _resetContext() internal {
+    function _resetContext() private {
         _storeContext(bytes32(0), address(0), uint64(0));
     }
 
     /// @notice Stores the call context
-    function _storeContext(bytes32 msgHash, address from, uint64 srcChainId) internal {
+    function _storeContext(bytes32 msgHash, address from, uint64 srcChainId) private {
         assembly {
             tstore(_CTX_SLOT, msgHash)
             tstore(add(_CTX_SLOT, 1), from)
@@ -583,7 +558,7 @@ contract Bridge is EssentialContract, IBridge {
     }
 
     /// @notice Loads the call context
-    function _loadContext() internal view returns (Context memory) {
+    function _loadContext() private view returns (Context memory) {
         bytes32 msgHash;
         address from;
         uint64 srcChainId;
@@ -594,4 +569,28 @@ contract Bridge is EssentialContract, IBridge {
         }
         return Context({ msgHash: msgHash, from: from, srcChainId: srcChainId });
     }
+
+    /// @notice Checks if the signal was received.
+    /// @param signalService The signalService
+    /// @param signal The signal.
+    /// @param chainId The ID of the chain the signal is stored on
+    /// @param proof The merkle inclusion proof.
+    /// @return True if the message was received.
+    function _proveSignalReceived(
+        address signalService,
+        bytes32 signal,
+        uint64 chainId,
+        bytes calldata proof
+    )
+        private
+        view
+        returns (bool)
+    {
+        bytes memory data = abi.encodeCall(
+            ISignalService.proveSignalReceived,
+            (chainId, resolve(chainId, "bridge", false), signal, proof)
+        );
+        (bool success, bytes memory ret) = signalService.staticcall(data);
+        return success ? abi.decode(ret, (bool)) : false;
+    }
 }
diff --git a/packages/protocol/contracts/common/OwnerUUPSUpgradable.sol b/packages/protocol/contracts/common/OwnerUUPSUpgradable.sol
@@ -93,14 +93,14 @@ abstract contract OwnerUUPSUpgradable is UUPSUpgradeable, OwnableUpgradeable {
     }
 
     // Stores the reentry lock
-    function _storeReentryLock(uint8 reentry) internal {
+    function _storeReentryLock(uint8 reentry) private {
         assembly {
             tstore(_REENTRY_SLOT, reentry)
         }
     }
 
     // Loads the reentry lock
-    function _loadReentryLock() internal view returns (uint8 reentry) {
+    function _loadReentryLock() private view returns (uint8 reentry) {
         assembly {
             reentry := tload(_REENTRY_SLOT)
         }

diff --git a/packages/protocol/contracts/tokenvault/BridgedERC20Base.sol b/packages/protocol/contracts/tokenvault/BridgedERC20Base.sol
@@ -85,10 +85,10 @@ abstract contract BridgedERC20Base is EssentialContract, IBridgedERC20 {
         return super.owner();
     }
 
+    function _mintToken(address account, uint256 amount) internal virtual;
+    function _burnToken(address from, uint256 amount) internal virtual;
+
     function _isMigratingOut() internal view returns (bool) {
         return migratingAddress != address(0) && !migratingInbound;
     }
-
-    function _mintToken(address account, uint256 amount) internal virtual;
-    function _burnToken(address from, uint256 amount) internal virtual;
 }
diff --git a/packages/protocol/contracts/tokenvault/ERC20Vault.sol b/packages/protocol/contracts/tokenvault/ERC20Vault.sol
@@ -131,9 +131,6 @@ contract ERC20Vault is BaseVault {
 
         btokenOld = canonicalToBridged[ctoken.chainId][ctoken.addr];
 
-        // TODO(Brecht): if the ctoken is on the current chain, should we check the
-        // symbol/name/decimals against the actual token?
-
         if (btokenOld != address(0)) {
             CanonicalERC20 memory _ctoken = bridgedToCanonical[btokenOld];