fix: audit fixes

src/L1Deployer.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: AGPL-3.0
 pragma solidity ^0.8.20;
 
 import {RoleManager} from "./RoleManager.sol";
 import {DeployerBase} from "./DeployerBase.sol";
@@ -66,10 +66,10 @@
     //////////////////////////////////////////////////////////////*/
 
     /// @notice Yearn STB Role Manager.
     RoleManager public immutable roleManager;
 
     /// @notice Polygon CDK Rollup Manager.
     IPolygonRollupManager public immutable rollupManager;
 
     /*//////////////////////////////////////////////////////////////
                            STORAGE
@@ -163,6 +163,7 @@
         address _l2Deployer
     ) external virtual {
         require(getRollupContract(_rollupID) == address(0), "registered");
+        require(_l1EscrowManager != address(0), "ZERO ADDRESS");
         require(_l2Deployer != address(0), "ZERO ADDRESS");
 
         IPolygonRollupContract _rollupContract = rollupManager
@@ -218,6 +219,8 @@
 
     /**
      * @notice Creates a new custom vault and escrow for a specific asset on the specified rollup.
+     * @dev If the L1 escrow already exists the Rollup admin
+     *  will need to update the vault manually on the escrow.
      * @param _rollupID The ID of the rollup.
      * @param _asset The address of the asset for which the vault and escrow are created.
      * @return _l1Escrow The address of the L1 escrow.
@@ -233,7 +236,11 @@
         returns (address _l1Escrow, address _vault)
     {
         _vault = roleManager.newVault(_rollupID, _asset);
-        _l1Escrow = _newCustomVault(_rollupID, _asset, _vault);
+        // Deploy an L1 escrow if it does not already exist.
+        _l1Escrow = getEscrow(_rollupID, _asset);
+        if (_l1Escrow == address(0)) {
+            _l1Escrow = _deployL1Escrow(_rollupID, _asset, _vault);
+        }
     }
 
     /**
@@ -250,23 +257,7 @@
     ) external virtual onlyRollupAdmin(_rollupID) returns (address _l1Escrow) {
         // Make sure the vault has been registered.
         require(roleManager.isVaultsRoleManager(_vault), "!role manager");
-        _l1Escrow = _newCustomVault(_rollupID, _asset, _vault);
-    }
-
-    /**
-     * @dev Deploys an L1 Escrow for a custom vault if one does not exist.
-     *  Will store all relevant information as well.
-     */
-    function _newCustomVault(
-        uint32 _rollupID,
-        address _asset,
-        address _vault
-    ) internal virtual returns (address _l1Escrow) {
-        _l1Escrow = getEscrow(_rollupID, _asset);
-
-        if (_l1Escrow == address(0)) {
-            _l1Escrow = _deployL1Escrow(_rollupID, _asset, _vault);
-        }
+        _l1Escrow = _deployL1Escrow(_rollupID, _asset, _vault);
     }
 
     /*//////////////////////////////////////////////////////////////
@@ -277,7 +268,7 @@
      * @dev Deploys a new L1 Escrow and send a message to the bridge to
      *   tell the L2 deployer to deploy the needed contract on the L2
      */
     function _deployL1Escrow(
         uint32 _rollupID,
         address _asset,
         address _vault
@@ -366,7 +357,7 @@
      */
     function getEscrowManager(
         uint32 _rollupID
-    ) public view virtual returns (address) {
+    ) external view virtual returns (address) {
         return _chainConfig[_rollupID].escrowManager;
     }
 

src/L1YearnEscrow.sol

@@ -97,7 +97,7 @@ contract L1YearnEscrow is L1Escrow {
         );
 
         // Max approve the vault
-        originTokenAddress().forceApprove(_vaultAddress, 2 ** 256 - 1);
+        IERC20(_originTokenAddress).forceApprove(_vaultAddress, 2 ** 256 - 1);
         // Set the vault variable
         VaultStorage storage $ = _getVaultStorage();
         $.vaultAddress = IVault(_vaultAddress);
@@ -114,19 +114,14 @@ contract L1YearnEscrow is L1Escrow {
     function _receiveTokens(
         uint256 amount
     ) internal virtual override whenNotPaused {
-        originTokenAddress().safeTransferFrom(
-            msg.sender,
-            address(this),
-            amount
-        );
+        IERC20 originToken = originTokenAddress();
+        originToken.safeTransferFrom(msg.sender, address(this), amount);
 
         VaultStorage storage $ = _getVaultStorage();
         uint256 _minimumBuffer = $.minimumBuffer;
         // Deposit to the vault if above buffer
         if (_minimumBuffer != 0) {
-            uint256 underlyingBalance = originTokenAddress().balanceOf(
-                address(this)
-            );
+            uint256 underlyingBalance = originToken.balanceOf(address(this));
 
             if (underlyingBalance <= _minimumBuffer) return;
 
@@ -181,7 +176,10 @@ contract L1YearnEscrow is L1Escrow {
             // Check again to account for if there was loose underlying
             if (amount > maxWithdraw) {
                 // Send an equivalent amount of shares for the difference.
-                uint256 shares = _vault.convertToShares(amount - maxWithdraw);
+                uint256 shares;
+                unchecked {
+                    shares = _vault.convertToShares(amount - maxWithdraw);
+                }
                 _vault.transfer(destinationAddress, shares);
                 if (maxWithdraw == 0) return;
                 amount = maxWithdraw;
@@ -220,6 +218,8 @@ contract L1YearnEscrow is L1Escrow {
     /**
      * @dev Update the vault to deploy funds into.
      *      Will fully withdraw from the old vault.
+     *      The current vault must be completely liquid for this to succeed.
+     *
      * @param _vaultAddress Address of the new vault to use.
      */
     function updateVault(
@@ -249,11 +249,14 @@ contract L1YearnEscrow is L1Escrow {
             // Deposit any loose funds over minimum buffer
             uint256 balance = originToken.balanceOf(address(this));
             uint256 _minimumBuffer = $.minimumBuffer;
-            if (balance > _minimumBuffer)
-                IVault(_vaultAddress).deposit(
-                    balance - _minimumBuffer,
-                    address(this)
-                );
+            if (balance > _minimumBuffer) {
+                unchecked {
+                    IVault(_vaultAddress).deposit(
+                        balance - _minimumBuffer,
+                        address(this)
+                    );
+                }
+            }
         }
 
         // Update Storage
@@ -285,10 +288,15 @@ contract L1YearnEscrow is L1Escrow {
 
         if (balance > _minimumBuffer) {
             // Deposit the difference.
-            $.vaultAddress.deposit(balance - _minimumBuffer, address(this));
+            unchecked {
+                $.vaultAddress.deposit(balance - _minimumBuffer, address(this));
+            }
         } else if (balance < _minimumBuffer) {
             // Withdraw the difference
-            uint256 diff = _minimumBuffer - balance;
+            uint256 diff;
+            unchecked {
+                diff = _minimumBuffer - balance;
+            }
             uint256 available = $.vaultAddress.maxWithdraw(address(this));
 
             // Withdraw the min between the difference or what is available.

src/RoleManager.sol

@@ -42,6 +42,17 @@ contract RoleManager is Positions {
         uint96 index;
     }
 
+    /// @notice Make sure the vault has been added to the role manager.
+    modifier vaultIsAdded(address _vault) {
+        _vaultIsAdded(_vault);
+        _;
+    }
+
+    /// @notice Check if the vault is added to the Role Manager.
+    function _vaultIsAdded(address _vault) internal view virtual {
+        require(vaultConfig[_vault].asset != address(0), "vault not added");
+    }
+
     /// @notice ID to use for the L1
     uint32 internal constant ORIGIN_NETWORK_ID = 0;
 
@@ -185,15 +196,21 @@ contract RoleManager is Positions {
         // Append the rollup ID for the name and symbol of custom vaults.
         string memory _id = _rollupID == ORIGIN_NETWORK_ID
             ? ""
-            : string(abi.encodePacked("-", Strings.toString(_rollupID)));
+            : string.concat("-", Strings.toString(_rollupID));
 
         // Name is "{SYMBOL}-STB yVault"
-        string memory _name = string(
-            abi.encodePacked(ERC20(_asset).symbol(), "-STB", _id, " yVault")
+        string memory _name = string.concat(
+            ERC20(_asset).symbol(),
+            "-STB",
+            _id,
+            " yVault"
         );
+
         // Symbol is "stb{SYMBOL}".
-        string memory _symbol = string(
-            abi.encodePacked("stb", ERC20(_asset).symbol(), _id)
+        string memory _symbol = string.concat(
+            "stb",
+            ERC20(_asset).symbol(),
+            _id
         );
 
         // Deploy through the registry so it is automatically endorsed.
@@ -378,10 +395,7 @@ contract RoleManager is Positions {
     function updateDebtAllocator(
         address _vault,
         address _debtAllocator
-    ) public virtual onlyPositionHolder(MANAGEMENT) {
-        // Make sure the vault has been added to the role manager.
-        require(vaultConfig[_vault].asset != address(0), "vault not added");
-
+    ) public virtual vaultIsAdded(_vault) onlyPositionHolder(MANAGEMENT) {
         // Remove the roles from the old allocator.
         _setRole(_vault, Position(vaultConfig[_vault].debtAllocator, 0));
 
@@ -406,10 +420,7 @@ contract RoleManager is Positions {
     function updateKeeper(
         address _vault,
         address _keeper
-    ) external virtual onlyPositionHolder(MANAGEMENT) {
-        // Make sure the vault has been added to the role manager.
-        require(vaultConfig[_vault].asset != address(0), "vault not added");
-
+    ) external virtual vaultIsAdded(_vault) onlyPositionHolder(MANAGEMENT) {
         // Remove the roles from the old keeper if active.
         address defaultKeeper = getPositionHolder(KEEPER);
         if (
@@ -429,18 +440,16 @@ contract RoleManager is Positions {
      */
     function removeVault(
         address _vault
-    ) external virtual onlyPositionHolder(CZAR) {
-        // Get the vault specific config.
-        VaultConfig memory config = vaultConfig[_vault];
-        // Make sure the vault has been added to the role manager.
-        require(config.asset != address(0), "vault not added");
-
+    ) external virtual vaultIsAdded(_vault) onlyPositionHolder(CZAR) {
         // Transfer the role manager position.
         IVault(_vault).transfer_role_manager(chad);
 
         // Address of the vault to replace it with.
         address vaultToMove = vaults[vaults.length - 1];
 
+        // Get the vault specific config.
+        VaultConfig memory config = vaultConfig[_vault];
+
         // Move the last vault to the index of `_vault`
         vaults[config.index] = vaultToMove;
         vaultConfig[vaultToMove].index = config.index;
@@ -470,7 +479,7 @@ contract RoleManager is Positions {
         uint256 _role
     ) external virtual onlyPositionHolder(CZAR) {
         address _vault;
-        for (uint256 i = 0; i < _vaults.length; ++i) {
+        for (uint256 i; i < _vaults.length; ++i) {
             _vault = _vaults[i];
             // Make sure the vault is added to this Role Manager.
             require(vaultConfig[_vault].asset != address(0), "vault not added");
@@ -523,6 +532,8 @@ contract RoleManager is Positions {
     function setDefaultProfitMaxUnlock(
         uint256 _newDefaultProfitMaxUnlock
     ) external virtual onlyPositionHolder(GOVERNATOR) {
+        require(_newDefaultProfitMaxUnlock != 0, "too short");
+        require(_newDefaultProfitMaxUnlock <= 31_556_952, "too long");
         defaultProfitMaxUnlock = _newDefaultProfitMaxUnlock;
 
         emit UpdateDefaultProfitMaxUnlock(_newDefaultProfitMaxUnlock);
@@ -568,7 +579,7 @@ contract RoleManager is Positions {
      * @param _asset The underlying asset used.
      * @return The default vault for the specified `_asset`.
      */
-    function getVault(address _asset) public view virtual returns (address) {
+    function getVault(address _asset) external view virtual returns (address) {
         return getVault(_asset, ORIGIN_NETWORK_ID);
     }
 
@@ -600,7 +611,7 @@ contract RoleManager is Positions {
      */
     function isVaultsRoleManager(
         address _vault
-    ) public view virtual returns (bool) {
+    ) external view virtual returns (bool) {
         return vaultConfig[_vault].asset != address(0);
     }
 

test/L1Deployer.t.sol

@@ -161,6 +161,8 @@ contract L1DeployerTest is Setup {
 
         IVault vault = IVault(_vault);
 
+        assertEq(vault.name(), "DAI-STB yVault");
+        assertEq(vault.symbol(), "stbDAI");
         assertEq(vault.accountant(), address(accountant));
         assertEq(vault.asset(), address(asset));
         assertEq(vault.deposit_limit(), 2 ** 256 - 1);