[R4R]-fix: N-01 Unnecessary Boolean Values (#128)

Co-authored-by: Ivan Wang <ivan.wang@mantle.xyz>
mantlenetworkio · Mar 6, 2024 · 5045418 · 5045418
1 parent 9a909a9
commit 5045418
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 10 deletions.
diff --git a/packages/contracts-bedrock/contracts/L1/L1StandardBridge.sol b/packages/contracts-bedrock/contracts/L1/L1StandardBridge.sol
@@ -862,8 +862,9 @@ contract L1StandardBridge is StandardBridge, Semver {
     ) internal override {
         require(msg.value==0, "L1StandardBridge: deposit MNT should not include ETH value.");
         IERC20(L1_MNT_ADDRESS).safeTransferFrom(_from, address(this), _amount);
-        bool success = IERC20(L1_MNT_ADDRESS).approve(address(MESSENGER), _amount);
-        require(success, "L1StandardBridge: approve for L1 MNT failed.");
+        // L1StandardBridge: approve for L1 MNT either reverts or returns true, there is no case in
+        // which its result value is false.
+        IERC20(L1_MNT_ADDRESS).approve(address(MESSENGER), _amount);
 
         // Emit the correct events. By default this will be ERC20BridgeInitiated, but child
         // contracts may override this function in order to emit legacy events as well.

diff --git a/packages/contracts-bedrock/contracts/L1/OptimismPortal.sol b/packages/contracts-bedrock/contracts/L1/OptimismPortal.sol
@@ -415,9 +415,10 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
         //   2. The amount of gas provided to the execution context of the target is at least the
         //      gas limit specified by the user. If there is not enough gas in the current context
         //      to accomplish this, `callWithMinGas` will revert.
-        bool l1mntSuccess = true;
         if (_tx.mntValue>0){
-            l1mntSuccess = IERC20(L1_MNT_ADDRESS).transfer(_tx.target, _tx.mntValue);
+            // The l1mntSuccess variable of transfer is either true or the transfer call reverted.
+            // It will never be false.
+            IERC20(L1_MNT_ADDRESS).transfer(_tx.target, _tx.mntValue);
         }
         require(_tx.target != L1_MNT_ADDRESS, "Directly calling MNT Token is forbidden");
         bool success = SafeCall.callWithMinGas(_tx.target, _tx.gasLimit, _tx.ethValue, _tx.data);
@@ -426,12 +427,12 @@ contract OptimismPortal is Initializable, ResourceMetering, Semver {
 
         // All withdrawals are immediately finalized. Replayability can
         // be achieved through contracts built on top of this contract
-        emit WithdrawalFinalized(withdrawalHash, success && l1mntSuccess);
+        emit WithdrawalFinalized(withdrawalHash, success);
 
         // Reverting here is useful for determining the exact gas cost to successfully execute the
         // sub call to the target contract if the minimum gas limit specified by the user would not
         // be sufficient to execute the sub call.
-        if ((success == false || l1mntSuccess == false) && tx.origin == Constants.ESTIMATION_ADDRESS) {
+        if (success == false && tx.origin == Constants.ESTIMATION_ADDRESS) {
             revert("OptimismPortal: withdrawal failed");
         }
     }

diff --git a/packages/contracts-bedrock/contracts/L2/L2CrossDomainMessenger.sol b/packages/contracts-bedrock/contracts/L2/L2CrossDomainMessenger.sol
@@ -260,18 +260,19 @@ contract L2CrossDomainMessenger is CrossDomainMessenger, Semver {
 
             return;
         }
-        bool ethSuccess = true;
         if (_ethValue != 0) {
-            ethSuccess = IERC20(Predeploys.BVM_ETH).approve(_target, _ethValue);
+            // The ethSuccess variable of approve is either true or the approve function reverted.
+            // It will never be false whenever its value is evaluated.
+            IERC20(Predeploys.BVM_ETH).approve(_target, _ethValue);
         }
         xDomainMsgSender = _sender;
         bool success = SafeCall.call(_target, gasleft() - RELAY_RESERVED_GAS, _mntValue, _message);
         xDomainMsgSender = Constants.DEFAULT_L2_SENDER;
         if (_ethValue != 0) {
-            ethSuccess = IERC20(Predeploys.BVM_ETH).approve(_target, 0);
+            IERC20(Predeploys.BVM_ETH).approve(_target, 0);
         }
 
-        if (success && ethSuccess) {
+        if (success) {
             require(!successfulMessages[versionedHash], "versionedHash has already be marked as successful");
             successfulMessages[versionedHash] = true;
             emit RelayedMessage(versionedHash);