S4 gas optimizations (#160)

* fix: use custom error for unsupported ERC20

* fix: reuse parentId

Author: amarinkovic

src/facets/TokenizedVaultIOFacet.sol

@@ -9,7 +9,7 @@ import { LibObject } from "../libs/LibObject.sol";
 import { LibConstants as LC } from "../libs/LibConstants.sol";
 import { LibACL } from "../libs/LibACL.sol";
 import { LibHelpers } from "../libs/LibHelpers.sol";
-import { ExternalWithdrawInvalidReceiver } from "../shared/CustomErrors.sol";
+import { ExternalWithdrawInvalidReceiver, InvalidERC20Token } from "../shared/CustomErrors.sol";
 import { ReentrancyGuard } from "../utils/ReentrancyGuard.sol";
 
 /**
@@ -29,8 +29,10 @@ contract TokenizedVaultIOFacet is Modifiers, ReentrancyGuard {
         address _externalTokenAddress,
         uint256 _amount
     ) external notLocked nonReentrant assertPrivilege(LibObject._getParentFromAddress(msg.sender), LC.GROUP_EXTERNAL_DEPOSIT) {
-        // a user can only deposit an approved external ERC20 token
-        require(LibAdmin._isSupportedExternalTokenAddress(_externalTokenAddress), "extDeposit: invalid ERC20 token");
+        if (!LibAdmin._isSupportedExternalTokenAddress(_externalTokenAddress)) {
+            revert InvalidERC20Token(_externalTokenAddress, "extDeposit");
+        }
+
         // a user can only deposit to their valid entity
         bytes32 entityId = LibObject._getParentFromAddress(msg.sender);
         require(LibEntity._isEntity(entityId), "extDeposit: invalid receiver");

src/facets/ZapFacet.sol

@@ -2,6 +2,7 @@
 pragma solidity 0.8.20;
 
 import { PermitSignature, OnboardingApproval } from "../shared/FreeStructs.sol";
+import { InvalidERC20Token } from "../shared/CustomErrors.sol";
 import { Modifiers } from "../shared/Modifiers.sol";
 import { LibTokenizedVaultIO } from "../libs/LibTokenizedVaultIO.sol";
 import { LibACL } from "../libs/LibACL.sol";
@@ -33,15 +34,16 @@ contract ZapFacet is Modifiers, ReentrancyGuard {
         PermitSignature calldata _permitSignature,
         OnboardingApproval calldata _onboardingApproval
     ) external notLocked nonReentrant {
-        // Check if it's a supported ERC20 token
-        require(LibAdmin._isSupportedExternalTokenAddress(_externalTokenAddress), "zapStake: invalid ERC20 token");
-
-        if (_onboardingApproval.entityId != 0 && LibObject._getParentFromAddress(msg.sender) != _onboardingApproval.entityId) {
-            LibAdmin._onboardUserViaSignature(_onboardingApproval);
+        if (!LibAdmin._isSupportedExternalTokenAddress(_externalTokenAddress)) {
+            revert InvalidERC20Token(_externalTokenAddress, "zapStake");
         }
 
         bytes32 parentId = LibObject._getParentFromAddress(msg.sender);
 
+        if (_onboardingApproval.entityId != 0 && parentId != _onboardingApproval.entityId) {
+            LibAdmin._onboardUserViaSignature(_onboardingApproval);
+        }
+
         // Use permit to set allowance
         IERC20(_externalTokenAddress).permit(msg.sender, address(this), _amountToDeposit, _permitSignature.deadline, _permitSignature.v, _permitSignature.r, _permitSignature.s);
 
@@ -74,8 +76,9 @@ contract ZapFacet is Modifiers, ReentrancyGuard {
         PermitSignature calldata _permitSignature,
         OnboardingApproval calldata _onboardingApproval
     ) external notLocked nonReentrant {
-        // Check if it's a supported ERC20 token
-        require(LibAdmin._isSupportedExternalTokenAddress(_externalTokenAddress), "zapOrder: invalid ERC20 token");
+        if (!LibAdmin._isSupportedExternalTokenAddress(_externalTokenAddress)) {
+            revert InvalidERC20Token(_externalTokenAddress, "zapOrder");
+        }
 
         bytes32 parentId = _onboardingApproval.entityId;
 

src/shared/CustomErrors.sol

@@ -201,3 +201,8 @@ error InvalidTokenId();
 
 /// @dev Cannot stake an amount lower than objectMinimumSell[tokenId].
 error InvalidStakingAmount();
+
+/// @dev Not a supported external ERC 20 token
+/// @param tokenAddress Address of the ERC20 token
+/// @param method Name of the method that threw the error
+error InvalidERC20Token(address tokenAddress, string method);

test/T03TokenizedVault.t.sol

@@ -109,7 +109,7 @@ contract T03TokenizedVaultTest is D03ProtocolDefaults, MockAccounts {
         vm.expectRevert("extDeposit: invalid receiver");
         nayms.externalDeposit(wethAddress, 1);
 
-        vm.expectRevert("extDeposit: invalid ERC20 token");
+        vm.expectRevert(abi.encodeWithSelector(InvalidERC20Token.selector, address(0xBADAAAAAAAAA), "extDeposit"));
         nayms.externalDeposit(address(0xBADAAAAAAAAA), 1);
 
         // deposit to entity1

test/T07Zaps.t.sol

@@ -4,6 +4,7 @@ pragma solidity 0.8.20;
 import { D03ProtocolDefaults, LC, LibHelpers, StdStyle } from "./defaults/D03ProtocolDefaults.sol";
 import { DummyToken } from "test/utils/DummyToken.sol";
 import { StakingConfig, PermitSignature, OnboardingApproval } from "src/shared/FreeStructs.sol";
+import { InvalidERC20Token } from "src/shared/CustomErrors.sol";
 
 import { InvalidSignatureLength } from "src/shared/CustomErrors.sol";
 
@@ -76,7 +77,7 @@ contract ZapFacetTest is D03ProtocolDefaults {
         PermitSignature memory permitSignature = PermitSignature({ deadline: deadline, v: v, r: r, s: s });
         OnboardingApproval memory approval;
 
-        vm.expectRevert("zapStake: invalid ERC20 token");
+        vm.expectRevert(abi.encodeWithSelector(InvalidERC20Token.selector, address(111), "zapStake"));
         nayms.zapStake(address(111), nlf.entityId, stakeAmount, stakeAmount, permitSignature, approval);
 
         nayms.zapStake(address(naymToken), nlf.entityId, stakeAmount, stakeAmount, permitSignature, approval);
@@ -117,7 +118,7 @@ contract ZapFacetTest is D03ProtocolDefaults {
         OnboardingApproval memory onboardingApproval = OnboardingApproval({ entityId: sue.entityId, roleId: cpRoleId, signature: sig });
 
         // verify token address
-        vm.expectRevert("zapOrder: invalid ERC20 token");
+        vm.expectRevert(abi.encodeWithSelector(InvalidERC20Token.selector, address(111), "zapOrder"));
         nayms.zapOrder(address(111), 10 ether, wethId, 1 ether, bob.entityId, 1 ether, permitSignature, onboardingApproval);
 
         OnboardingApproval memory oa = OnboardingApproval ({

test/fixtures/TokenizedVaultFixture.sol

@@ -3,11 +3,13 @@ pragma solidity 0.8.20;
 
 import { LibTokenizedVaultIO } from "src/libs/LibTokenizedVaultIO.sol";
 import { LibAdmin } from "src/libs/LibAdmin.sol";
+import { InvalidERC20Token } from "src/shared/CustomErrors.sol";
 
 contract TokenizedVaultFixture {
     function externalDepositDirect(bytes32 _to, address _externalTokenAddress, uint256 _amount) public {
-        // a user can only deposit an approved external ERC20 token
-        require(LibAdmin._isSupportedExternalTokenAddress(_externalTokenAddress), "extDeposit: invalid ERC20 token");
+        if (!LibAdmin._isSupportedExternalTokenAddress(_externalTokenAddress)) {
+            revert InvalidERC20Token(_externalTokenAddress, "extDeposit");
+        }
         LibTokenizedVaultIO._externalDeposit(_to, _externalTokenAddress, _amount);
     }
 }