infiniFi PR 187

Description

The ERC4626 standard includes a decimalOffset parameter that sets the shares decimals to the asset decimals plus the offset:

// From OpenZeppelin ERC4626 Implementationfunction decimals() public view virtual override(IERC20Metadata, ERC20) returns (uint8) {    return _underlyingDecimals + _decimalsOffset();}

Since shares won't always have 18 decimals, we need to calculate the correct decimals for one share instead of using the WAD constant

Recommendation

Consider retrieving the vault decimals directly and using that value instead of WAD.

- uint256 sharePrice = ERC4626(vault).convertToAssets(FixedPointMathLib.WAD);+ uint256 sharePrice = ERC4626(vault).convertToAssets(10**ERC4626(vault).decimals());

InfiniFi: Acknowledge, these checks should be done before deployment.

Cantina Managed: We're acknowledging this finding because the vault should be verified before deployment to ensure there are no differences in decimals between assets and shares, and that the decimalsOffset is set to 0.

Description

The sharePrice value returned from ERC4626.convertToAssets uses the asset's native decimal precision rather than the standard 18 decimals. When an asset has decimals other than 18, a conversion is necessary since the price should be returned in 18 decimals.

Recommendation

Convert the sharePrice to 18 decimals to match the expected return format.

InfiniFi: Acknowledge, these checks should be done before deployment.

Cantina Managed: We're acknowledging this finding because the vault should be verified before deployment to ensure there are no differences in decimals between assets and shares, and that the decimalsOffset is set to 0.

Description

In sGHOFarm, most state‑changing functions are protected with whenNotPaused (unwrapSGHOToGHO, wrapUSDCToGHO, unwrapGHOToUSDC). However, wrapGHOtoSGHO(uint256 _amount) lacks this guard. This allows staking GHO into sGHO even when the farm is paused, which is inconsistent with the contract’s pause semantics.

Recommendation

Consider to add whenNotPaused to wrapGHOtoSGHO for consistency with the rest of the swap/convert surface:

function wrapGHOtoSGHO(uint256 _amount)    public    onlyCoreRole(CoreRoles.FARM_SWAP_CALLER)    whenNotPaused{    ...}

For claimRewards, consider to align it with your desired pause policy, either add whenNotPaused or document why it should remain callable while paused (e.g., to keep rewards collection operational).

InfiniFi: Fixed in commit 39fd2be.

Cantina Managed: Fix verified.

Description

In sGHOFarm.unwrapGHOToUSDC, the code requests a GSM quote and only uses the first return value (asset tokens out), while approvals, slippage checks, and event data are based on the caller‑provided ghoAmount:

(uint256 waEthUSDCTokensOut,,,) = IGsm(GSM).getAssetAmountForBuyAsset(ghoAmount);_approveToken(GHO, GSM, ghoAmount);IGsm(GSM).buyAsset(waEthUSDCTokensOut, address(this));
// ...uint256 expectedUSDC = ghoAmount / 1e12; // used for minUSDCOut and event data

However, getAssetAmountForBuyAsset(maxGhoAmount) returns:

1. assetAmount (waEthUSDC tokens out),
2. ghoRequired = exact GHO that will be pulled (gross + fee, rounded to not exceed maxGhoAmount),
3. gross GHO (without fee),
4. fee.

Because the function may round down the actual ghoRequired below ghoAmount, the current logic:

• Over‑approves GHO to the GSM (approval set to ghoAmount instead of ghoRequired).
• Overestimates expected output by computing expectedUSDC from ghoAmount rather than ghoRequired, making minUSDCOut stricter than necessary and risking false reverts even when the trade is valid.
• Emits misleading data (e.g., logging ghoAmount instead of the actual GHO spent).

Recommendation

Consider to base approvals, slippage checks, and event data on the quoted ghoRequired (the second return value):

• Capture all needed quote values:

  (uint256 waEthUSDCTokensOut, uint256 ghoRequired,,) = IGsm(GSM).getAssetAmountForBuyAsset(ghoAmount);

• Approve exactly what will be spent:

  _approveToken(GHO, GSM, ghoRequired);

• Use ghoRequired to derive expectations and checks:

  uint256 expectedUSDC = ghoRequired / 1e12; // 18 → 6 decimalsuint256 minUSDCOut = expectedUSDC.mulWadDown(maxSlippage);

• Emit events using ghoRequired (actual spent) rather than ghoAmount (cap).

• Optionally, consider to assert the quote respects the cap for clarity:

  require(ghoRequired <= ghoAmount, "Quoted GHO exceeds cap");

This aligns approvals with actual pull behavior, stabilizes slippage checks against rounding/fees, and makes emitted data reflect the true amounts.

InfiniFi: Fixed in commit b3c485e.

Cantina Managed: Fix verified.

Description

When calling wrapUSDCToGHO, it’s necessary to convert USDC to waEthUSDC before converting to GHO. After the initial conversion, the sellAsset function converts a maximum of waEthUSDCBalance to GHO. However, the converted amount may be less than the total waEthUSDCBalance, leaving a small dust amount of waEthUSDC in the contract that isn't accounted for in the assets. These dust assets can be accumulated until withdrawn later when calling the unwrapGHOToUSDC function.

Recommendation

The dust amount is only a few wei, making it not worth converting back to USDC as you would spend more in gas than the value recovered. It's better to leave these small amounts for later conversion. Adding a comment about this behavior in the code would be helpful.

InfiniFi: Acknowledged, won't fix

Cantina Managed: Acknowledged.

Description

The ERC4626Oracle must not be used in Accounting with ERC4626FarmWithMaturity function, otherwise it will calculate an incorrect totalValue.

The price returned in ERC4626Oracle is in terms of USD/Share, while the assets() returned in ERC4626FarmWithMaturity or ERC4626Farm is in terms of assets (USDC):

function assets() public view override returns (uint256) {    uint256 vaultShares = ERC20(vault).balanceOf(address(this));    return ERC4626(vault).convertToAssets(vaultShares);}

When Accounting calculates the totalValue, it uses this formula:

_totalValue += _assets.mulWadDown(assetPrice); //in this case: (USDC) * (USD/Share)

Here's an example demonstrating the issue:

We have an ERC4626Farm with 100 USDC deposit in a vault with ratio 1 USDC : 0.5 stakedUSDC
Price Calculation:assetPrice = 1e18 (1USDC:1$)sharePrice = 2e18 (1 stakedUSDC = 2 USDC)returned price = 2e18
Total assets in farm: 100 USDC

Total value calculation:_totalValue = price * assets_totalValue = 2 * 100 = 200

The totalValue calculated is incorrect as $200, when the actual value of USDC in the vault should be $100.

Recommendation

Not use the ERC4626Oracle with ERC4626Farm as it will provide incorrect result.

InfiniFi: USDC oracle price is 1e30 to account for the 12 decimals of correction, so we will get : assetPrice = 1e30 sharePrice = 2e18 returned price = 2e30

the farm will return assets() = 100e6 and value will be 100e6 * 1e30 / wad = 100e18 (the assetToken of the farm will be vault.asset() so it will be USDC & not the 4626 share token, at no point the 2e30 oracle will be read)

In other places where the share token oracle is used, e.g. a farm swaps 100 USDC to 49.9 stakedUSDC and wants to check for slippage, we will get :

• 100e6 * 1e30 / wad = 100e18 $ in
• 49.9e6 * 2e30 / wad = 99.8e18 $ out
• = 0.2% slippage

it will be used to price ERC4626 external assets inside the Farm assets calculation. Example of this would be syrupUSD https://etherscan.io/address/0x80ac24aa929eaf5013f6436cda2a7ba190f5cc0b#readContract

Cantina Managed: We're acknowledging this finding. The oracle should be used as a slippage check rather than for calculating the total asset value.

Description

FxSaveFarm performs meaningful state transitions without emitting events. In particular, the following external functions modify protocol state or move funds but do not emit any contract‑level events:

• stake(uint256 _amountIn) – wraps fxUSD into fxSave shares.
• beginUnstake(uint256 _amountToUnstake) – initiates fxSave redemption via requestRedeem.
• completeUnstake() – finalizes redemption via claim, moving funds back to the farm.
• signSwapOrder(address _tokenIn, address _tokenOut, uint256 _amountIn, uint256 _minAmountOut) – prepares and signs a CoW Swap order for asset conversion.

The absence of events complicates off‑chain monitoring, accounting reconciliation, incident response, and user‑facing analytics. Other farms in the codebase emit events for similar actions, so this appears to be an oversight rather than an intentional gas optimization.

Recommendation

Consider to add granular events for each state‑changing action, with enough context to reconstruct flows off‑chain. For example:

event FxSaveStaked(address indexed caller, uint256 amountIn, uint256 sharesOut, uint256 timestamp);event FxSaveUnstakeStarted(address indexed caller, uint256 sharesRequested, uint256 timestamp);event FxSaveUnstakeCompleted(address indexed caller, uint256 fxUsdOut, uint256 usdcOut, uint256 timestamp);event FxUsdSwapOrderSigned(    address indexed caller,    address indexed tokenIn,    address indexed tokenOut,    uint256 amountIn,    uint256 minAmountOut,    bytes orderData,    uint256 timestamp);

• Emit FxSaveStaked after a successful depositToFxSave, ideally including computed sharesOut if available (or 0 when not retrievable).
• Emit FxSaveUnstakeStarted after requestRedeem.
• Emit FxSaveUnstakeCompleted after claim, optionally including measured deltas of received fxUSD/USDC if inexpensive to compute.
• Emit FxUsdSwapOrderSigned after _checkSwapApproveAndSignOrder, including the returned order payload (or its hash) for traceability.

If computing exact deltas is costly, consider to emit minimal but actionable fields (caller, inputs, and relevant IDs/hashes). This keeps gas overhead bounded while enabling robust observability.

InfiniFi: Acknowledged, won't fix

Cantina Managed: Acknowledged.

Description

FxSaveFarm.signSwapOrder() only permits {USDC, fxUSD} and rejects fxSave as tokenIn/tokenOut. This prevents using fxSave directly as part of swap flows (e.g., {fxSave ↔ fxUSD} or {fxSave ↔ USDC}), which could be operationally useful when liquidity or pricing favors those paths. Note: IFxSave.requestRedeem() introduces a locking time period (shares are moved to the user’s proxy and become illiquid until claim), so any fxSave → … path inherently includes that delay.

Recommendation

Consider to allow _FXSAVE as a valid tokenIn and tokenOut in signSwapOrder. Document that fxSave → … conversions rely on requestRedeem/claim and therefore include a locking window before funds become available, whereas … → fxSave uses the deposit path. This change keeps behavior explicit and makes these routes available when advantageous.

InfiniFi: Generally against this as pricing mechanism for it is within the farm itself

Cantina Managed: Acknowledged.

Description

Found by Infinifi team.

sGHOFarm inherits MultiAssetFarm, which defines a maxSlippage parameter with a default value. In the current constructor:

constructor(address _core, address _accounting) MultiAssetFarm(_core, USDC, _accounting) {}

No explicit slippage tolerance is set, meaning the contract uses the default from MultiAssetFarm. The client indicated that this farm is intended to operate with a 20 bps tolerance (0.2%), so the absence of explicit configuration can lead to incorrect minimum-out checks during swaps.

Recommendation

Consider to set maxSlippage in the constructor as parameter and assign it the global state variable of this farm.

This makes the farm’s slippage policy explicit and prevents reliance on defaults that may change or differ between farm types.

InfiniFi: Fixed in commit 9abd0d4.

Cantina Managed: Fix verified.

Description

FxSaveFarm.completeUnstake() currently calls only:

IFxSave(_FXSAVE).claim(address(this));

This redeems fxSave shares into USDC and fxUSD after the normal requestRedeem() locking period. The SavingFxUSDFacet contract also exposes two alternative flows:

• redeemFromFxSave redeems and optionally converts all proceeds into a single desired token in one step.
• instantRedeemFromFxSave redeems immediately, bypassing the standard ~1 hour cooldown.

Restricting completeUnstake to claim means the farm always waits for the lock period to end and always receives mixed USDC + fxUSD, even when operational needs might require faster settlement or a single-asset output.

Recommendation

Consider to extend completeUnstake to optionally call redeemFromFxSave or instantRedeemFromFxSave based on parameters. This would:

• Allow bypassing the lock when acceptable (e.g., in emergency exits).
• Support one-asset settlement when operationally useful.

If these options are added, clearly document that instantRedeemFromFxSave bypasses the normal locking period and may have different fee or pricing implications.

InfiniFi: Decided to use only the existing route as the diamond payload requires some effort on our backend side

Cantina: Acknowledged.

Description and Recommendation

In sGHOFarm contract, the assetPrice variable could be defined inside the if(sghoBalance > 0) since it's only used to calculate the total asset value. This would save gas when there is no sghoBalance.

function assets() public view override(MultiAssetFarm, IFarm) returns (uint256) {-    uint256 assetPrice = Accounting(accounting).price(assetToken);    uint256 totalAssetValue = MultiAssetFarm.assets();    // sGHO balance (excluding unvested rewards)    uint256 sghoBalance = _tokenBalance(sGHO) - unvestedRewards();    if (sghoBalance > 0) {+       uint256 assetPrice = Accounting(accounting).price(assetToken);        uint256 ghoPrice = Accounting(accounting).price(sGHO);        totalAssetValue += sghoBalance.mulDivDown(ghoPrice, assetPrice);    }    return totalAssetValue;}

InfiniFi: Fixed in commit 9abd0d4.

Cantina Managed: Fix verified.

Description

In FxSaveFarm contract the stake function the calls to depositToFxSave from SavingFxUSDFacet contract, building a non-empty params.data payload using:

data: abi.encodeWithSignature(              "convert(address,uint256,uint256,uint256[])",              _FXUSD, // _tokenIn              _amountIn, // _amount              0, // _encoding              new uint256[](0) // _routes          ),

However, both tokenIn and tokenOut are _FXUSD, which causes the downstream router LibRouter.sol#L167 to return early without using params.data. This makes the encoding operation redundant and increases calldata size, resulting in unnecessary gas costs.

Recommendation

Consider to replace the encoded payload with an empty byte array using new bytes(0) when no conversion is required. Keep params.target unchanged to satisfy the router’s target approval check. If conversion logic is added in the future for this path, restore the appropriate encoding only when needed.

InfiniFi: Left it as it is to avoid but definitely useful recommendation.

Cantina Managed: Acknowledged.

Description

In sGHOFarm.unwrapSGHOToGHO, the code approves the sGHO token to the sGHO contract right before calling cooldown() and redeem():

_approveToken(sGHO, sGHO, amountToConvert);IAaveStakeToken(sGHO).cooldown();IAaveStakeToken(sGHO).redeem(address(this), amountToConvert);

For Aave‑style stake tokens, redeem burns the caller’s staked balance and does not transfer tokens via transferFrom, so no ERC‑20 allowance is required. As a result, this approval is unnecessary and only increases gas usage (plus emits superfluous Approval events). Note this differs from the wrap path, where approving GHO → sGHO is required for stake().

Recommendation

Consider to remove the approval line in unwrapSGHOToGHO:

// _approveToken(sGHO, sGHO, amountToConvert); // not neededIAaveStakeToken(sGHO).cooldown();IAaveStakeToken(sGHO).redeem(address(this), amountToConvert);

Keep the approval in the wrap path (wrapGHOtoSGHO) where the staking contract needs allowance for GHO.

InfiniFi: Fixed in commit bb163ef.

Cantina Managed: Fix verified.

Description

The following imports are present but unused in the respective files:

• src/integrations/farms/FxSaveFarm.sol:

  import {Farm} from "@integrations/Farm.sol";

• src/governance/MaturedFarmCleaner.sol:

  import {CoreRoles} from "@libraries/CoreRoles.sol";

Keeping unused imports increases bytecode size unnecessarily and may cause confusion for readers or future maintainers.

Recommendation

Consider to remove unused imports to reduce compilation output size and improve code clarity. If these were intended for future functionality, comment them out with an explanatory note instead of leaving them silently unused.

InfiniFi: Fixed in commit f6e0fdd.

Cantina Managed: Fix verified.