LoopFi / Penpie #2

Re: [WP-L5] Aggregation oracle should use the earliest timestamp

The updatedAt field in aggregator2 is being ignored.

https://github.com/LoopFi/loop-contracts/blob/a812dd0414a8d2b4fc3d971ad07ef6909844262c/src/oracle/CombinedAggregatorV3Oracle.sol#L40-L51

    /// @notice Return the latest price combined from two Chainlink like oracle and the timestamp from the first aggregator
    function latestRoundData()
        public
        view
        returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound)
    {
        (uint256 value, uint256 timestamp) = getAggregatorData(aggregator, aggregatorScale, aggregatorHeartbeat);
        (uint256 value2, ) = getAggregatorData(aggregator2, aggregator2Scale, aggregator2Heartbeat);
        uint256 price;
        if(isMul) price = wmul(value, value2); else price = wdiv(value, value2);
        return (0, int256(price), 0, timestamp, 0);
    }

[WP-L2] PositionActionPenpie._onDecreaseLever() will unexpectedly revert at L133 when CDPVault.tokenScale() > wad

For example, when CDPVault.tokenScale() is math and subCollateral is 1234567890123456789012345

  • _onDecreaseLever()
    • L131 ICDPVault.withdraw()
      • L258 tokenAmount = wdiv(amount, tokenScale) = 1234567890123456789012345 * 1e18 / 1e24 = 1234567890123456789
      • L260 modifyCollateralAndDebt()
        • L511 amount = wmul(abs(deltaCollateral), tokenScale) = 1234567890123456789 * 1e24 / 1e18 = 1234567890123456789000000
        • L512 only transfers 1234567890123456789000000

Due to precision loss in _onWithdraw() -> ICDPVault.withdraw(), the amount withdrawn is less than subCollateral, causing the transfer at PositionActionPenpie.sol#L133 to revert.

Consider changing L131 to _onWithdraw(leverParams.vault, leverParams.position, pendleToken, subCollateral, 0); and removing penpieHelper.withdrawMarket(pendleToken, subCollateral); at L133. Also, move L132 above L131 so we have pendleToken.

@@ 123,126 @@ /// @notice Hook to decrease lever by withdrawing collateral from the CDPVault /// @param leverParams LeverParams struct /// @param subCollateral Amount of collateral to subtract in CDPVault decimals [wad] /// @return tokenOut Amount of underlying token of Pendle SY [underlying scale]
function _onDecreaseLever( LeverParams memory leverParams, uint256 subCollateral ) internal override returns (uint256 tokenOut) { _onWithdraw(leverParams.vault, leverParams.position, address(0), subCollateral, 0); (address pendleToken, , ) = abi.decode(leverParams.auxAction.args, (address, uint256, address)); penpieHelper.withdrawMarket(pendleToken, subCollateral); if (leverParams.auxAction.args.length != 0) { bytes memory exitData = _delegateCall( address(poolAction), abi.encodeWithSelector(poolAction.exit.selector, leverParams.auxAction) ); tokenOut = abi.decode(exitData, (uint256)); } }
@@ 253,256 @@ /// @notice Withdraws collateral tokens from this contract and decreases a user's collateral balance /// @param to Address of the user to withdraw tokens to /// @param amount Amount of tokens to withdraw [tokenScale] /// @return tokenAmount Amount of tokens withdrawn [wad]
function withdraw(address to, uint256 amount) external returns (uint256 tokenAmount) { tokenAmount = wdiv(amount, tokenScale); int256 deltaCollateral = -toInt256(tokenAmount); modifyCollateralAndDebt({ owner: to, collateralizer: msg.sender, creditor: msg.sender, deltaCollateral: deltaCollateral, deltaDebt: 0 }); }
@@ 406,419 @@ /// @notice Modifies a Position's collateral and debt balances /// @dev Checks that the global debt ceiling and the vault's debt ceiling have not been exceeded via the CDM, /// - that the Position is still safe after the modification, /// - that the msg.sender has the permission of the owner to decrease the collateral-to-debt ratio, /// - that the msg.sender has the permission of the collateralizer to put up new collateral, /// - that the msg.sender has the permission of the creditor to settle debt with their credit, /// - that that the vault debt floor is exceeded /// - that the vault minimum collateralization ratio is met /// @param owner Address of the owner of the position /// @param collateralizer Address of who puts up or receives the collateral delta /// @param creditor Address of who provides or receives the credit delta for the debt delta /// @param deltaCollateral Amount of collateral to put up (+) or to remove (-) from the position [wad] /// @param deltaDebt Amount of normalized debt (gross, before rate is applied) to generate (+) or /// to settle (-) on this position [wad]
function modifyCollateralAndDebt( address owner, address collateralizer, address creditor, int256 deltaCollateral, int256 deltaDebt ) public {
@@ 427,505 @@ if ( // position is either more safe than before or msg.sender has the permission from the owner ((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) || // msg.sender has the permission of the collateralizer to collateralize the position using their cash (deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) || // msg.sender has the permission of the creditor to use their credit to repay the debt (deltaDebt < 0 && !hasPermission(creditor, msg.sender)) ) revert CDPVault__modifyCollateralAndDebt_noPermission(); // if the vault is paused allow only debt decreases if (deltaDebt > 0 || deltaCollateral != 0) { _requireNotPaused(); } Position memory position = positions[owner]; DebtData memory debtData = _calcDebt(position); uint256 newDebt; uint256 newCumulativeIndex; uint256 profit; int256 quotaRevenueChange; if (deltaDebt > 0) { uint256 debtToIncrease = uint256(deltaDebt); // Internal debt calculation remains in 18-decimal precision (newDebt, newCumulativeIndex) = CreditLogic.calcIncrease( debtToIncrease, position.debt, debtData.cumulativeIndexNow, position.cumulativeIndexLastUpdate ); position.cumulativeQuotaInterest = debtData.cumulativeQuotaInterest; position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow; quotaRevenueChange = _calcQuotaRevenueChange(deltaDebt); uint256 scaledDebtIncrease = wmul(debtToIncrease, poolUnderlyingScale); pool.lendCreditAccount(scaledDebtIncrease, creditor); } else if (deltaDebt < 0) { uint256 debtToDecrease = abs(deltaDebt); uint256 maxRepayment = calcTotalDebt(debtData); if (debtToDecrease >= maxRepayment) { debtToDecrease = maxRepayment; deltaDebt = -toInt256(debtToDecrease); } uint256 scaledDebtDecrease = wmul(debtToDecrease, poolUnderlyingScale); poolUnderlying.safeTransferFrom(creditor, address(pool), scaledDebtDecrease); uint128 newCumulativeQuotaInterest; if (debtToDecrease == maxRepayment) { newDebt = 0; newCumulativeIndex = debtData.cumulativeIndexNow; profit = debtData.accruedInterest; newCumulativeQuotaInterest = 0; } else { (newDebt, newCumulativeIndex, profit, newCumulativeQuotaInterest) = calcDecrease( debtToDecrease, position.debt, debtData.cumulativeIndexNow, position.cumulativeIndexLastUpdate, debtData.cumulativeQuotaInterest ); } quotaRevenueChange = _calcQuotaRevenueChange(-int(debtData.debt - newDebt)); uint256 scaledRemainingDebt = wmul(debtData.debt - newDebt, poolUnderlyingScale); uint256 scaledProfit = wmul(profit, poolUnderlyingScale); pool.repayCreditAccount(scaledRemainingDebt, scaledProfit, 0); position.cumulativeQuotaInterest = newCumulativeQuotaInterest; position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow; } else { newDebt = position.debt; newCumulativeIndex = debtData.cumulativeIndexLastUpdate; }
if (deltaCollateral > 0) { uint256 amount = wmul(deltaCollateral.toUint256(), tokenScale); token.safeTransferFrom(collateralizer, address(this), amount); } else if (deltaCollateral < 0) { uint256 amount = wmul(abs(deltaCollateral), tokenScale); token.safeTransfer(collateralizer, amount); }
@@ 515,530 @@ position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, deltaCollateral, totalDebt); VaultConfig memory config = vaultConfig; uint256 spotPrice_ = spotPrice(); uint256 collateralValue = wmul(position.collateral, spotPrice_); if ( (deltaDebt > 0 || deltaCollateral < 0) && !_isCollateralized(calcTotalDebt(_calcDebt(position)), collateralValue, config.liquidationRatio) ) revert CDPVault__modifyCollateralAndDebt_notSafe(); if (quotaRevenueChange != 0) { int256 scaledQuotaRevenueChange = wmul(poolUnderlyingScale, quotaRevenueChange); IPoolV3(pool).updateQuotaRevenue(scaledQuotaRevenueChange); } emit ModifyCollateralAndDebt(owner, collateralizer, creditor, deltaCollateral, deltaDebt);
}

For reference, in PositionActionPenpie._onWithdraw(), the withdrawal amount is based on the return value of vault.withdraw():

https://github.com/LoopFi/loop-contracts/blob/a812dd0414a8d2b4fc3d971ad07ef6909844262c/src/proxy/PositionActionPenpie.sol#L72-L93

    /// @notice Withdraw collateral from the vault
    /// @param vault Address of the vault
    /// @param position Address of the position
    /// @param dst Token the caller expects to receive
    /// @param amount Amount of collateral to withdraw [wad]
    /// @return Amount of collateral withdrawn [CDPVault.tokenScale()]
    function _onWithdraw(
        address vault,
        address position,
        address dst,
        uint256 amount,
        uint256 /*minAmountOut*/
    ) internal override returns (uint256) {
        uint256 scaledCollateralWithdrawn = ICDPVault(vault).withdraw(address(position), amount);
        uint256 collateralWithdrawn = wmul(scaledCollateralWithdrawn, ICDPVault(vault).tokenScale());
        address collateralToken = address(ICDPVault(vault).token());

        if (dst != collateralToken && dst != address(0)) {
            penpieHelper.withdrawMarket(dst, collateralWithdrawn);
        }
        return collateralWithdrawn;
    }
@@ 123,126 @@ /// @notice Hook to decrease lever by withdrawing collateral from the CDPVault /// @param leverParams LeverParams struct /// @param subCollateral Amount of collateral to subtract in CDPVault decimals [wad] /// @return tokenOut Amount of underlying token of Pendle SY [underlying scale]
function _onDecreaseLever( LeverParams memory leverParams, uint256 subCollateral ) internal override returns (uint256 tokenOut) { _onWithdraw(leverParams.vault, leverParams.position, address(0), subCollateral, 0); (address pendleToken, , ) = abi.decode(leverParams.auxAction.args, (address, uint256, address)); penpieHelper.withdrawMarket(pendleToken, subCollateral); if (leverParams.auxAction.args.length != 0) { bytes memory exitData = _delegateCall( address(poolAction), abi.encodeWithSelector(poolAction.exit.selector, leverParams.auxAction) ); tokenOut = abi.decode(exitData, (uint256)); } }
@@ 253,256 @@ /// @notice Withdraws collateral tokens from this contract and decreases a user's collateral balance /// @param to Address of the user to withdraw tokens to /// @param amount Amount of tokens to withdraw [tokenScale] /// @return tokenAmount Amount of tokens withdrawn [wad]
function withdraw(address to, uint256 amount) external returns (uint256 tokenAmount) { tokenAmount = wdiv(amount, tokenScale); int256 deltaCollateral = -toInt256(tokenAmount); modifyCollateralAndDebt({ owner: to, collateralizer: msg.sender, creditor: msg.sender, deltaCollateral: deltaCollateral, deltaDebt: 0 }); }
@@ 406,419 @@ /// @notice Modifies a Position's collateral and debt balances /// @dev Checks that the global debt ceiling and the vault's debt ceiling have not been exceeded via the CDM, /// - that the Position is still safe after the modification, /// - that the msg.sender has the permission of the owner to decrease the collateral-to-debt ratio, /// - that the msg.sender has the permission of the collateralizer to put up new collateral, /// - that the msg.sender has the permission of the creditor to settle debt with their credit, /// - that that the vault debt floor is exceeded /// - that the vault minimum collateralization ratio is met /// @param owner Address of the owner of the position /// @param collateralizer Address of who puts up or receives the collateral delta /// @param creditor Address of who provides or receives the credit delta for the debt delta /// @param deltaCollateral Amount of collateral to put up (+) or to remove (-) from the position [wad] /// @param deltaDebt Amount of normalized debt (gross, before rate is applied) to generate (+) or /// to settle (-) on this position [wad]
function modifyCollateralAndDebt( address owner, address collateralizer, address creditor, int256 deltaCollateral, int256 deltaDebt ) public {
@@ 427,505 @@ if ( // position is either more safe than before or msg.sender has the permission from the owner ((deltaDebt > 0 || deltaCollateral < 0) && !hasPermission(owner, msg.sender)) || // msg.sender has the permission of the collateralizer to collateralize the position using their cash (deltaCollateral > 0 && !hasPermission(collateralizer, msg.sender)) || // msg.sender has the permission of the creditor to use their credit to repay the debt (deltaDebt < 0 && !hasPermission(creditor, msg.sender)) ) revert CDPVault__modifyCollateralAndDebt_noPermission(); // if the vault is paused allow only debt decreases if (deltaDebt > 0 || deltaCollateral != 0) { _requireNotPaused(); } Position memory position = positions[owner]; DebtData memory debtData = _calcDebt(position); uint256 newDebt; uint256 newCumulativeIndex; uint256 profit; int256 quotaRevenueChange; if (deltaDebt > 0) { uint256 debtToIncrease = uint256(deltaDebt); // Internal debt calculation remains in 18-decimal precision (newDebt, newCumulativeIndex) = CreditLogic.calcIncrease( debtToIncrease, position.debt, debtData.cumulativeIndexNow, position.cumulativeIndexLastUpdate ); position.cumulativeQuotaInterest = debtData.cumulativeQuotaInterest; position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow; quotaRevenueChange = _calcQuotaRevenueChange(deltaDebt); uint256 scaledDebtIncrease = wmul(debtToIncrease, poolUnderlyingScale); pool.lendCreditAccount(scaledDebtIncrease, creditor); } else if (deltaDebt < 0) { uint256 debtToDecrease = abs(deltaDebt); uint256 maxRepayment = calcTotalDebt(debtData); if (debtToDecrease >= maxRepayment) { debtToDecrease = maxRepayment; deltaDebt = -toInt256(debtToDecrease); } uint256 scaledDebtDecrease = wmul(debtToDecrease, poolUnderlyingScale); poolUnderlying.safeTransferFrom(creditor, address(pool), scaledDebtDecrease); uint128 newCumulativeQuotaInterest; if (debtToDecrease == maxRepayment) { newDebt = 0; newCumulativeIndex = debtData.cumulativeIndexNow; profit = debtData.accruedInterest; newCumulativeQuotaInterest = 0; } else { (newDebt, newCumulativeIndex, profit, newCumulativeQuotaInterest) = calcDecrease( debtToDecrease, position.debt, debtData.cumulativeIndexNow, position.cumulativeIndexLastUpdate, debtData.cumulativeQuotaInterest ); } quotaRevenueChange = _calcQuotaRevenueChange(-int(debtData.debt - newDebt)); uint256 scaledRemainingDebt = wmul(debtData.debt - newDebt, poolUnderlyingScale); uint256 scaledProfit = wmul(profit, poolUnderlyingScale); pool.repayCreditAccount(scaledRemainingDebt, scaledProfit, 0); position.cumulativeQuotaInterest = newCumulativeQuotaInterest; position.cumulativeQuotaIndexLU = debtData.cumulativeQuotaIndexNow; } else { newDebt = position.debt; newCumulativeIndex = debtData.cumulativeIndexLastUpdate; }
if (deltaCollateral > 0) { uint256 amount = wmul(deltaCollateral.toUint256(), tokenScale); token.safeTransferFrom(collateralizer, address(this), amount); } else if (deltaCollateral < 0) { uint256 amount = wmul(abs(deltaCollateral), tokenScale); token.safeTransfer(collateralizer, amount); }
@@ 515,530 @@ position = _modifyPosition(owner, position, newDebt, newCumulativeIndex, deltaCollateral, totalDebt); VaultConfig memory config = vaultConfig; uint256 spotPrice_ = spotPrice(); uint256 collateralValue = wmul(position.collateral, spotPrice_); if ( (deltaDebt > 0 || deltaCollateral < 0) && !_isCollateralized(calcTotalDebt(_calcDebt(position)), collateralValue, config.liquidationRatio) ) revert CDPVault__modifyCollateralAndDebt_notSafe(); if (quotaRevenueChange != 0) { int256 scaledQuotaRevenueChange = wmul(poolUnderlyingScale, quotaRevenueChange); IPoolV3(pool).updateQuotaRevenue(scaledQuotaRevenueChange); } emit ModifyCollateralAndDebt(owner, collateralizer, creditor, deltaCollateral, deltaDebt);
}

[WP-I3] It is recommended to only use the Pendle LP Oracle for price quotes with verified and trusted LP tokens.

This LP oracle is added after the audit and therefore out of scope for this audit.

The current implementation should work correctly with tETH.

For other LP tokens, it is critical to verify the underlying asset implementation.

During SY transfer hook execution, read-only reentrancy attacks could manipulate the Pendle LP Oracle into reporting incorrect prices.

@@ 90,93 @@ /// @notice Returns the latest price for the asset from Chainlink [WAD] /// @param /*token*/ Token address /// @return price Asset price [WAD] /// @dev reverts if the price is invalid
function spot(address /* token */) external view virtual override returns (uint256 price) { bool isValidPtOracle = _validatePtOracle(); if (!isValidPtOracle) revert PendleLPOracle__validatePtOracle_invalidValue(); return market.getLpToAssetRate(twapWindow); }
@@ 12,17 @@ /** * This function returns the approximated twap rate LP/asset on market, but take into account the current rate of SY This is to account for special cases where underlying asset becomes insolvent and has decreasing exchangeRate * @param market market to get rate from * @param duration twap duration */
function getLpToAssetRate(IPMarket market, uint32 duration) internal view returns (uint256) { (uint256 syIndex, uint256 pyIndex) = PendlePtOracleLib.getSYandPYIndexCurrent(market); uint256 lpToAssetRateRaw = _getLpToAssetRateRaw(market, duration, pyIndex); if (syIndex >= pyIndex) { return lpToAssetRateRaw; } else { return (lpToAssetRateRaw * syIndex) / pyIndex; } } function _getLpToAssetRateRaw( IPMarket market, uint32 duration, uint256 pyIndex ) private view returns (uint256 lpToAssetRateRaw) { MarketState memory state = market.readState(address(0)); int256 totalHypotheticalAsset;
@@ 36,53 @@ if (state.expiry <= block.timestamp) { // 1 PT = 1 Asset post-expiry totalHypotheticalAsset = state.totalPt + PYIndexLib.syToAsset(PYIndex.wrap(pyIndex), state.totalSy); } else { MarketPreCompute memory comp = state.getMarketPreCompute(PYIndex.wrap(pyIndex), block.timestamp); (int256 rateOracle, int256 rateHypTrade) = _getPtRatesRaw(market, state, duration); int256 cParam = LogExpMath.exp(comp.rateScalar.mulDown((rateOracle - comp.rateAnchor))); int256 tradeSize = (cParam.mulDown(comp.totalAsset) - state.totalPt).divDown( PMath.IONE + cParam.divDown(rateHypTrade) ); totalHypotheticalAsset = comp.totalAsset - tradeSize.divDown(rateHypTrade) + (state.totalPt + tradeSize).divDown(rateOracle); }
lpToAssetRateRaw = totalHypotheticalAsset.divDown(state.totalLp).Uint(); }
    /**
     * @notice LP Holders can burn their LP to receive back SY & PT proportionally
     * to their share of the market
     */
    function burn(
        address receiverSy,
        address receiverPt,
        uint256 netLpToBurn
    ) external nonReentrant returns (uint256 netSyOut, uint256 netPtOut) {
        MarketState memory market = readState(msg.sender);

        _burn(address(this), netLpToBurn);

        (netSyOut, netPtOut) = market.removeLiquidity(netLpToBurn);

        if (receiverSy != address(this)) IERC20(SY).safeTransfer(receiverSy, netSyOut);
        if (receiverPt != address(this)) IERC20(PT).safeTransfer(receiverPt, netPtOut);

        _writeState(market);

        emit Burn(receiverSy, receiverPt, netLpToBurn, netSyOut, netPtOut);
    }
    /*///////////////////////////////////////////////////////////////
                            TRANSFER HOOKS
    //////////////////////////////////////////////////////////////*/
    function _beforeTokenTransfer(address from, address to, uint256) internal virtual override whenNotPaused {
        _updateAndDistributeRewardsForTwo(from, to);
    }
@@ 90,93 @@ /// @notice Returns the latest price for the asset from Chainlink [WAD] /// @param /*token*/ Token address /// @return price Asset price [WAD] /// @dev reverts if the price is invalid
function spot(address /* token */) external view virtual override returns (uint256 price) { bool isValidPtOracle = _validatePtOracle(); if (!isValidPtOracle) revert PendleLPOracle__validatePtOracle_invalidValue(); return market.getLpToAssetRate(twapWindow); }
@@ 12,17 @@ /** * This function returns the approximated twap rate LP/asset on market, but take into account the current rate of SY This is to account for special cases where underlying asset becomes insolvent and has decreasing exchangeRate * @param market market to get rate from * @param duration twap duration */
function getLpToAssetRate(IPMarket market, uint32 duration) internal view returns (uint256) { (uint256 syIndex, uint256 pyIndex) = PendlePtOracleLib.getSYandPYIndexCurrent(market); uint256 lpToAssetRateRaw = _getLpToAssetRateRaw(market, duration, pyIndex); if (syIndex >= pyIndex) { return lpToAssetRateRaw; } else { return (lpToAssetRateRaw * syIndex) / pyIndex; } } function _getLpToAssetRateRaw( IPMarket market, uint32 duration, uint256 pyIndex ) private view returns (uint256 lpToAssetRateRaw) { MarketState memory state = market.readState(address(0)); int256 totalHypotheticalAsset;
@@ 36,53 @@ if (state.expiry <= block.timestamp) { // 1 PT = 1 Asset post-expiry totalHypotheticalAsset = state.totalPt + PYIndexLib.syToAsset(PYIndex.wrap(pyIndex), state.totalSy); } else { MarketPreCompute memory comp = state.getMarketPreCompute(PYIndex.wrap(pyIndex), block.timestamp); (int256 rateOracle, int256 rateHypTrade) = _getPtRatesRaw(market, state, duration); int256 cParam = LogExpMath.exp(comp.rateScalar.mulDown((rateOracle - comp.rateAnchor))); int256 tradeSize = (cParam.mulDown(comp.totalAsset) - state.totalPt).divDown( PMath.IONE + cParam.divDown(rateHypTrade) ); totalHypotheticalAsset = comp.totalAsset - tradeSize.divDown(rateHypTrade) + (state.totalPt + tradeSize).divDown(rateOracle); }
lpToAssetRateRaw = totalHypotheticalAsset.divDown(state.totalLp).Uint(); }
    /**
     * @notice LP Holders can burn their LP to receive back SY & PT proportionally
     * to their share of the market
     */
    function burn(
        address receiverSy,
        address receiverPt,
        uint256 netLpToBurn
    ) external nonReentrant returns (uint256 netSyOut, uint256 netPtOut) {
        MarketState memory market = readState(msg.sender);

        _burn(address(this), netLpToBurn);

        (netSyOut, netPtOut) = market.removeLiquidity(netLpToBurn);

        if (receiverSy != address(this)) IERC20(SY).safeTransfer(receiverSy, netSyOut);
        if (receiverPt != address(this)) IERC20(PT).safeTransfer(receiverPt, netPtOut);

        _writeState(market);

        emit Burn(receiverSy, receiverPt, netLpToBurn, netSyOut, netPtOut);
    }
    /*///////////////////////////////////////////////////////////////
                            TRANSFER HOOKS
    //////////////////////////////////////////////////////////////*/
    function _beforeTokenTransfer(address from, address to, uint256) internal virtual override whenNotPaused {
        _updateAndDistributeRewardsForTwo(from, to);
    }

[WP-N4] Only the implementation of PositionActionPenpie._onDeposit() was modified, without updating its NatSpec Documentation.

For reference, the feat/deploy-script-usdc branch modified the NatSpec Documentation for PositionAction._onDeposit() implementations.

The return value of _onDeposit() is not used anywhere in the repo. The NatSpec Documentation serves as the main source for expected behavior and intent.

https://github.com/LoopFi/loop-contracts/blob/a812dd0414a8d2b4fc3d971ad07ef6909844262c/src/proxy/PositionActionPenpie.sol#L47-L70

    /// @notice Deposit collateral into the vault
    /// @param vault Address of the vault
    /// @param amount Amount of collateral to deposit [CDPVault.tokenScale()]
    /// @param src Pendle LP token address
    /// @return Amount of collateral deposited [wad]
    function _onDeposit(
        address vault,
        address position,
        address src,
        uint256 amount
    ) internal override returns (uint256) {
        address collateralToken = address(ICDPVault(vault).token());

        // if the src is not the collateralToken, we need to deposit the underlying into the Penpie staking contract
        if (src != collateralToken) {
            IERC20(src).forceApprove(address(penpieStaking), amount);
            penpieHelper.depositMarketFor(src, address(this), amount);
        }

        IERC20(collateralToken).forceApprove(vault, amount);
        uint256 depositAmount = ICDPVault(vault).deposit(position, amount);
        uint256 scaledAmount = wmul(depositAmount, ICDPVault(vault).tokenScale());
        return scaledAmount;
    }

Additionally, if following the convention where "scaled*Amount refers to the normalized amount in [wad] units, rather than calling the raw token amount (in [token ** token.decimals()] base) as scaled*Amount", it is recommended to name L67 as scaled*Amount instead of L68 to minimize confusion.

For reference, see PositionActionPenpie._onWithdraw():

https://github.com/LoopFi/loop-contracts/blob/a812dd0414a8d2b4fc3d971ad07ef6909844262c/src/proxy/PositionActionPenpie.sol#L72-L93

@@ 72,76 @@ /// @notice Withdraw collateral from the vault /// @param vault Address of the vault /// @param position Address of the position /// @param dst Token the caller expects to receive /// @param amount Amount of collateral to withdraw [wad]
/// @return Amount of collateral withdrawn [CDPVault.tokenScale()] function _onWithdraw( address vault, address position, address dst, uint256 amount, uint256 /*minAmountOut*/ ) internal override returns (uint256) { uint256 scaledCollateralWithdrawn = ICDPVault(vault).withdraw(address(position), amount); uint256 collateralWithdrawn = wmul(scaledCollateralWithdrawn, ICDPVault(vault).tokenScale()); address collateralToken = address(ICDPVault(vault).token()); if (dst != collateralToken && dst != address(0)) { penpieHelper.withdrawMarket(dst, collateralWithdrawn); } return collateralWithdrawn; }

[WP-G5] PositionActionPenpie._onIncreaseLever() L116's balanceOf() query may be redundant

@@ 95,100 @@ /// @notice Hook to increase lever by depositing collateral into the CDPVault /// @param leverParams LeverParams struct /// @param /*upFrontToken*/ the address of the token passed up front /// @param /*upFrontAmount*/ the amount of tokens passed up front [CDPVault.tokenScale()] /// @param /*swapAmountOut*/ the amount of tokens received from the stablecoin flash loan swap [CDPVault.tokenScale()] /// @return addCollateralAmount Amount of collateral added to CDPVault position [CDPVault.tokenScale()]
function _onIncreaseLever( LeverParams memory leverParams, address /*upFrontToken*/, uint256 /*upFrontAmount*/, uint256 /*swapAmountOut*/ ) internal override returns (uint256 addCollateralAmount) { if (leverParams.auxAction.args.length != 0) { _delegateCall(address(poolAction), abi.encodeWithSelector(poolAction.join.selector, leverParams.auxAction)); } address underlying = IPenpieReceiptToken(address(ICDPVault(leverParams.vault).token())).underlying(); addCollateralAmount = IERC20(underlying).balanceOf(address(this)); IERC20(underlying).forceApprove(address(penpieStaking), addCollateralAmount); penpieHelper.depositMarketFor(underlying, address(this), addCollateralAmount); addCollateralAmount = ICDPVault(leverParams.vault).token().balanceOf(address(this)); ICDPVault(leverParams.vault).token().forceApprove(leverParams.vault, addCollateralAmount); // deposit into the CDP Vault return addCollateralAmount; }

For comparison, PositionActionPenpie._onDeposit() does not update amount with collateralToken.balanceOf(address(this)) after calling penpieHelper.depositMarketFor(src, address(this), amount):

https://github.com/LoopFi/loop-contracts/blob/a812dd0414a8d2b4fc3d971ad07ef6909844262c/src/proxy/PositionActionPenpie.sol#L47-L70

    /// @notice Deposit collateral into the vault
    /// @param vault Address of the vault
    /// @param amount Amount of collateral to deposit [CDPVault.tokenScale()]
    /// @param src Pendle LP token address
    /// @return Amount of collateral deposited [wad]
    function _onDeposit(
        address vault,
        address position,
        address src,
        uint256 amount
    ) internal override returns (uint256) {
        address collateralToken = address(ICDPVault(vault).token());

        // if the src is not the collateralToken, we need to deposit the underlying into the Penpie staking contract
        if (src != collateralToken) {
            IERC20(src).forceApprove(address(penpieStaking), amount);
            penpieHelper.depositMarketFor(src, address(this), amount);
        }

        IERC20(collateralToken).forceApprove(vault, amount);
        uint256 depositAmount = ICDPVault(vault).deposit(position, amount);
        uint256 scaledAmount = wmul(depositAmount, ICDPVault(vault).tokenScale());
        return scaledAmount;
    }
@@ 95,100 @@ /// @notice Hook to increase lever by depositing collateral into the CDPVault /// @param leverParams LeverParams struct /// @param /*upFrontToken*/ the address of the token passed up front /// @param /*upFrontAmount*/ the amount of tokens passed up front [CDPVault.tokenScale()] /// @param /*swapAmountOut*/ the amount of tokens received from the stablecoin flash loan swap [CDPVault.tokenScale()] /// @return addCollateralAmount Amount of collateral added to CDPVault position [CDPVault.tokenScale()]
function _onIncreaseLever( LeverParams memory leverParams, address /*upFrontToken*/, uint256 /*upFrontAmount*/, uint256 /*swapAmountOut*/ ) internal override returns (uint256 addCollateralAmount) { if (leverParams.auxAction.args.length != 0) { _delegateCall(address(poolAction), abi.encodeWithSelector(poolAction.join.selector, leverParams.auxAction)); } address underlying = IPenpieReceiptToken(address(ICDPVault(leverParams.vault).token())).underlying(); addCollateralAmount = IERC20(underlying).balanceOf(address(this)); IERC20(underlying).forceApprove(address(penpieStaking), addCollateralAmount); penpieHelper.depositMarketFor(underlying, address(this), addCollateralAmount); addCollateralAmount = ICDPVault(leverParams.vault).token().balanceOf(address(this)); ICDPVault(leverParams.vault).token().forceApprove(leverParams.vault, addCollateralAmount); // deposit into the CDP Vault return addCollateralAmount; }