Organization
- @alto-money
Engagement Type
Cantina Solo
Period
-
Researchers
Findings
Low Risk
2 findings
1 fixed
1 acknowledged
Informational
6 findings
3 fixed
3 acknowledged
Gas Optimizations
1 findings
0 fixed
1 acknowledged
Low Risk2 findings
Potential arithmetic overflow in Chainlink price conversion for large input amounts
Severity
- Severity: Low
≈
Likelihood: Low×
Impact: Medium Submitted by
phaze
Description
The
readChainlinkFeed()function inModuleChainlinkperforms multiplication operations that could overflow when processing very large input amounts. While the protocol's typical usage patterns involve small values (around 1 unit of the underlying asset), the function is public and could be used externally with larger amounts, potentially causing unexpected reverts.The
readChainlinkFeed()function converts prices between different decimal representations by performing multiplications before divisions:function readChainlinkFeed(uint256 quoteAmount, ChainlinkNode memory config) public view returns (uint256) { // ... validation ... uint256 price = uint256(ratio); uint256 powFeed = 10 ** config.feed.decimals(); uint256 powIn = 10 ** config.inDecimals; uint256 powOut = 10 ** config.outDecimals; if (config.direction == ConversionDirection.IN_TO_OUT) { // quoteAmount = quoteAmount * ratio / 10^feedDecimals * 10^outDecimals / 10^inDecimals quoteAmount = FixedPointMath.divideWithRounding(quoteAmount, price * powOut, powFeed * powIn, false); } else { // quoteAmount = quoteAmount * 10^feedDecimals / ratio * 10^outDecimals / 10^inDecimals quoteAmount = FixedPointMath.divideWithRounding(quoteAmount, powFeed * powOut, price * powIn, false); } return quoteAmount;}The
divideWithRounding()function uses standard Solidity multiplication and division, which can overflow at 2^256. For a typical ETH oracle configuration:- Feed decimals: 8
- Output decimals (USD precision): 36
- Input amount: ~1e18 (1 ETH)
- Price: ~1e11 (at $3,000 per ETH)
The numerator calculation becomes:
quoteAmount * price * powOut = 1e18 * 1e11 * 1e36 = 1e65This approaches the uint256 maximum of ~1e77, leaving a margin of only 1e12. The function would overflow when the input amount reaches approximately 1e12 ether (depending on the current ETH price), or similar values for other assets.
While the protocol's oracles typically call this function with values around 1 unit of the underlying asset (as seen in
AltoMultiChainlinkLendingOracle.getPrice()andAltoERC4626RatioChainlinkOracle.getPrice()), the function is public and could be used by external contracts or integrators with larger amounts.Additionally, the parameter is named
quoteAmount, which is misleading. In standard trading terminology, the "quote" is the output amount (e.g., USD), while the input should be called the "base" or "asset" amount.Recommendation
Consider implementing one of the following approaches to make the function more robust:
Option 1: Use 512-bit precision arithmetic
The codebase already includes Uniswap's
FullMathlibrary, which handles intermediate overflows using 512-bit precision. Consider using it for the price conversion:function readChainlinkFeed(uint256 quoteAmount, ChainlinkNode memory config) public view returns (uint256) { // ... validation ... uint256 price = uint256(ratio); uint256 powFeed = 10 ** config.feed.decimals(); uint256 powIn = 10 ** config.inDecimals; uint256 powOut = 10 ** config.outDecimals; if (config.direction == ConversionDirection.IN_TO_OUT) {- quoteAmount = FixedPointMath.divideWithRounding(quoteAmount, price * powOut, powFeed * powIn, false);+ quoteAmount = _mulDiv(quoteAmount * price, powOut, powFeed * powIn); } else {- quoteAmount = FixedPointMath.divideWithRounding(quoteAmount, powFeed * powOut, price * powIn, false);+ quoteAmount = _mulDiv(quoteAmount * powFeed, powOut, price * powIn); } return quoteAmount;}Option 2: Pre-calculate decimal adjustments
Reduce the numerator size by pre-calculating the net decimal adjustment:
function readChainlinkFeed(uint256 baseAmount, ChainlinkNode memory config) public view returns (uint256) { // ... validation ... uint256 price = uint256(ratio); uint256 feedDecimals = config.feed.decimals(); // Calculate net decimal adjustment (uint256 numeratorPow, uint256 denominatorPow) = _calculateDecimalAdjustment(config.outDecimals, config.inDecimals, feedDecimals); if (config.direction == ConversionDirection.IN_TO_OUT) { baseAmount = baseAmount * price * numeratorPow / denominatorPow; } else { baseAmount = baseAmount * numeratorPow / (price * denominatorPow); } return baseAmount;} function _calculateDecimalAdjustment( uint256 outDecimals, uint256 inDecimals, uint256 feedDecimals) internal pure returns (uint256 numeratorPow, uint256 denominatorPow) { int256 exponentDiff; if (direction == ConversionDirection.IN_TO_OUT) { // For IN_TO_OUT: outDecimals / (feedDecimals + inDecimals) // Net exponent: outDecimals - feedDecimals - inDecimals exponentDiff = int256(outDecimals) - int256(feedDecimals) - int256(inDecimals); } else { // For OUT_TO_IN: (feedDecimals + outDecimals) / inDecimals // Net exponent: feedDecimals + outDecimals - inDecimals exponentDiff = int256(feedDecimals) + int256(outDecimals) - int256(inDecimals); } numeratorPow = 1; denominatorPow = 1; if (exponentDiff > 0) { numeratorPow = 10 ** uint256(exponentDiff); } else if (exponentDiff < 0) { denominatorPow = 10 ** uint256(-exponentDiff); }}This approach would reduce the numerator for ETH from 1e65 to approximately 1e36, providing significantly more headroom before overflow (up to ~1e41 ether).
Oracle price feeds assume 1:1 peg for wrapped Bitcoin assets
State
- Acknowledged
Severity
- Severity: Low
≈
Likelihood: Low×
Impact: High Submitted by
phaze
Summary
The oracle deployments for cbBTC and tBTC assume a 1:1 exchange rate with Bitcoin without accounting for potential depegging. Both assets have direct Chainlink USD price feeds available that would eliminate this assumption and provide accurate market-based pricing. This contrasts with the WBTC oracle which properly uses a dedicated WBTC/BTC ratio feed to account for potential deviations from the peg.
Description
The protocol deploys oracles for three wrapped Bitcoin assets: WBTC, cbBTC, and tBTC. However, these deployments handle the wrapped asset to BTC relationship inconsistently.
WBTC Oracle (Proper Implementation): The
deployWbtcOracle()function properly accounts for the WBTC/BTC ratio by chaining two Chainlink feeds:function deployWbtcOracle(address owner) public returns (address) { ChainlinkNode[] memory chainlinkNodes = new ChainlinkNode[](2); // First feed: WBTC/BTC ratio chainlinkNodes[0] = ChainlinkNode({ feed: AggregatorV3Interface(CHAINLINK_WBTC_BTC_RATIO_FEED), direction: ConversionDirection.IN_TO_OUT, stalePeriod: 26 hours, inDecimals: 8, outDecimals: 8 }); // Second feed: BTC/USD price chainlinkNodes[1] = ChainlinkNode({ feed: AggregatorV3Interface(CHAINLINK_BTC_USD_PRICE_FEED), direction: ConversionDirection.IN_TO_OUT, stalePeriod: 2 hours, inDecimals: 8, outDecimals: 46 }); return address(new AltoMultiChainlinkLendingOracle(owner, chainlinkNodes));}This approach correctly prices WBTC as:
WBTC Price = (WBTC/BTC ratio) × (BTC/USD price)cbBTC and tBTC Oracles (Incomplete Implementation): Both
deployCbbtcOracle()anddeployTbtcOracle()assume a 1:1 ratio with BTC by using only the BTC/USD price feed:function deployCbbtcOracle(address owner) public returns (address) { ChainlinkNode[] memory chainlinkNodes = new ChainlinkNode[](1); chainlinkNodes[0] = ChainlinkNode({ feed: AggregatorV3Interface(CHAINLINK_BTC_USD_PRICE_FEED), direction: ConversionDirection.IN_TO_OUT, stalePeriod: 2 hours, inDecimals: 8, outDecimals: 46 }); return address(new AltoMultiChainlinkLendingOracle(owner, chainlinkNodes));} function deployTbtcOracle(address owner) public returns (address) { ChainlinkNode[] memory chainlinkNodes = new ChainlinkNode[](1); chainlinkNodes[0] = ChainlinkNode({ feed: AggregatorV3Interface(CHAINLINK_BTC_USD_PRICE_FEED), direction: ConversionDirection.IN_TO_OUT, stalePeriod: 2 hours, inDecimals: 18, outDecimals: 36 }); return address(new AltoMultiChainlinkLendingOracle(owner, chainlinkNodes));}Both implementations effectively price the assets as:
Asset Price = BTC/USD price(assuming 1:1 peg)However, Chainlink provides direct USD price feeds for both assets that reflect their actual market values:
- cbBTC/USD feed:
0x2665701293fCbEB223D11A08D826563EDcCE423A - tBTC/USD feed:
0x8350b7De6a6a2C1368E7D4Bd968190e13E354297
Risk of 1:1 Peg Assumption: Wrapped Bitcoin assets have historically maintained close to 1:1 pegs with Bitcoin, but depegging events can occur due to:
- Custodial issues with the backing reserves
- Smart contract vulnerabilities
- Regulatory actions affecting the issuer
- Market liquidity problems
When a wrapped asset depegs, using only the BTC/USD price would result in either overvaluing or undervaluing the collateral.
In a lending protocol, overvalued collateral is particularly dangerous as it allows users to borrow more than their collateral is actually worth, potentially leading to bad debt accumulation.
Impact Explanation
The impact is high. If cbBTC or tBTC were to depeg from Bitcoin, the lending protocol would incorrectly value the collateral, potentially allowing:
- Users to over-borrow against depegged assets
- Liquidations to fail due to overvalued collateral
- Accumulation of bad debt in the protocol
Likelihood Explanation
The likelihood is low. While wrapped Bitcoin assets have generally maintained strong pegs historically, depegging events have occurred in crypto markets (though not specifically with these assets). The risk increases during market stress or if issues arise with the asset issuers.
Recommendation
Use the direct Chainlink USD price feeds for both cbBTC and tBTC instead of assuming a 1:1 peg with Bitcoin:
For cbBTC:
function deployCbbtcOracle(address owner) public returns (address) { ChainlinkNode[] memory chainlinkNodes = new ChainlinkNode[](1); chainlinkNodes[0] = ChainlinkNode({ feed: AggregatorV3Interface(CHAINLINK_CBBTC_USD_PRICE_FEED), // cbBTC/USD feed direction: ConversionDirection.IN_TO_OUT, stalePeriod: 2 hours, inDecimals: 8, outDecimals: 46 }); return address(new AltoMultiChainlinkLendingOracle(owner, chainlinkNodes));}For tBTC:
function deployTbtcOracle(address owner) public returns (address) { ChainlinkNode[] memory chainlinkNodes = new ChainlinkNode[](1); chainlinkNodes[0] = ChainlinkNode({ feed: AggregatorV3Interface(CHAINLINK_TBTC_USD_PRICE_FEED), // tBTC/USD feed direction: ConversionDirection.IN_TO_OUT, stalePeriod: 2 hours, inDecimals: 18, outDecimals: 36 }); return address(new AltoMultiChainlinkLendingOracle(owner, chainlinkNodes));}These direct price feeds provide accurate market-based pricing that reflects actual trading values rather than assuming maintained pegs with Bitcoin.
Informational6 findings
User-supplied initial parameters increase risk of misconfiguration in ratio tracking
State
- Acknowledged
Severity
- Severity: Informational
≈
Likelihood: Low×
Impact: Medium Submitted by
phaze
Description
The
ModuleERC4626Ratiocontract relies on user-suppliedinitialRatioandinitialTimestampparameters during initialization to establish the baseline for tracking ratio growth over time. The contract validates these parameters in_validateERC4626RatioParams(), checking thatinitialRatiois non-zero andinitialTimestampis not in the future. However, there is no validation that these values accurately reflect the actual on-chain state at the time of initialization.The
_computeMaxRatio()function uses these parameters to calculate the maximum allowed ratio growth:function _computeMaxRatio(ERC4626RatioNode memory _erc4626RatioNode) internal view returns (uint256) { uint256 timePassed = block.timestamp - _erc4626RatioNode.initialTimestamp; uint256 maxGrowthPerYear = _erc4626RatioNode.initialRatio * _erc4626RatioNode.maxYearlyGrowthRatioWad; uint256 maxAllowedGrowthRatio = maxGrowthPerYear * timePassed / MATH_PRECISION / SECONDS_PER_YEAR; return _erc4626RatioNode.initialRatio + maxAllowedGrowthRatio;}If the
initialRatioorinitialTimestampare set incorrectly, aninitialRatioset too low or an initialTimestamp` in the past would allow the actual ratio to grow beyond intended limits before the cap takes effect. Conversely, if setting it too low would unnecessarily restrict the reported ratio.Since the contract already implements
_getRatioOfWrappedAssetToUnderlyingAsset()to fetch the current ratio from the underlying vault, these values could be determined programmatically at initialization time rather than relying on manual input.Recommendation
Consider modifying the initialization process to automatically fetch the initial parameters rather than accepting them as user input. This would involve moving the
_getRatioOfWrappedAssetToUnderlyingAsset()function from the child contracts to theModuleERC4626Ratiocontract and calling it during initialization:function _validateERC4626RatioParams(ERC4626RatioNode memory _erc4626RatioNode) internal view {- if (_erc4626RatioNode.initialRatio == 0) {- revert ModuleERC4626RatioInvalidInput();- }- if (_erc4626RatioNode.initialTimestamp > block.timestamp) {- revert ModuleERC4626RatioInvalidInput();- } if (_erc4626RatioNode.maxYearlyGrowthRatioWad == 0) { revert ModuleERC4626RatioInvalidInput(); }} +function _initializeERC4626RatioParams(+ uint256 maxYearlyGrowthRatioWad+) internal {+ erc4626RatioNode = ERC4626RatioNode({+ initialRatio: _getRatioOfWrappedAssetToUnderlyingAsset(),+ initialTimestamp: block.timestamp,+ maxYearlyGrowthRatioWad: maxYearlyGrowthRatioWad+ });+}This approach would reduce the risk of misconfiguration through human errors.
Manual initial price in TWAP initialization increases risk of misconfiguration
State
- Acknowledged
Severity
- Severity: Informational
≈
Likelihood: Low×
Impact: Medium Submitted by
phaze
Description
The
AltoRewardsOraclecontract'sstartTwapPeriod()function requires the owner to manually provide an initial price when starting the TWAP period. However, the contract already contains the necessary infrastructure to fetch the current price from Uniswap and Chainlink oracles in theupdatePrice()function.The
startTwapPeriod()function currently accepts a user-supplied_initialPrice:function startTwapPeriod(uint256 _initialPrice) external onlyOwner { if (lastPriceUpdateTimestamp != 0) { revert AltoRewardsOracleAlreadyStartedTwapPeriod(); } lastPriceUpdateTimestamp = block.timestamp; cachedPrice = _initialPrice; emit PriceUpdated(block.timestamp, cachedPrice, 0);}Meanwhile, the
updatePrice()function demonstrates that the oracle can fetch the current price programmatically:function updatePrice() external { // ... validation checks ... // reads the amount of out currency for 1 full unit of in-currency uint256 newPrice = readUniswapPool(inTokenOneUnit, uniswapNode); // converts the price from the out-currency to chainlinkNode.outDecimals decimals cachedPrice = readChainlinkFeed(newPrice, chainlinkNode); lastPriceUpdateTimestamp = block.timestamp; emit PriceUpdated(block.timestamp, newPrice, oldPrice);}Requiring manual input of the initial price creates an opportunity for human error. If the owner provides an incorrect initial price, it could affect the TWAP calculation used in the AltoRewardsDistributor for pricing call options.
Recommendation
Consider refactoring the price fetching logic into an internal function that can be shared between
startTwapPeriod()andupdatePrice():function startTwapPeriod() external onlyOwner { if (lastPriceUpdateTimestamp != 0) { revert AltoRewardsOracleAlreadyStartedTwapPeriod(); } _updatePrice();} function updatePrice() external { if (lastPriceUpdateTimestamp == 0) { revert AltoRewardsOracleNotInitiated(); } if (block.timestamp - lastPriceUpdateTimestamp <= priceUpdateCooldownPeriod) { revert AltoRewardsOracleTooEarlyToUpdatePrice(); } _updatePrice();} function _updatePrice() internal { uint256 oldPrice = cachedPrice; // reads the amount of out currency for 1 full unit of in-currency uint256 newPrice = readUniswapPool(inTokenOneUnit, uniswapNode); // converts the price from the out-currency to chainlinkNode.outDecimals decimals cachedPrice = readChainlinkFeed(newPrice, chainlinkNode); lastPriceUpdateTimestamp = block.timestamp; emit PriceUpdated(block.timestamp, newPrice, oldPrice);}This approach eliminates the need for manual price input, reducing the risk of configuration errors while ensuring consistency in how prices are fetched throughout the contract's lifecycle.
Missing validation in Uniswap direction configuration could result in inverted price oracle
Severity
- Severity: Informational
≈
Likelihood: Low×
Impact: Medium Submitted by
phaze
Description
The
AltoRewardsOraclecontract lacks validation to ensure that the Uniswap pool direction is configured correctly. This could result in the oracle returning inverted prices (e.g., USDC/ALTO instead of ALTO/USDC), which would break the pricing mechanism in theAltoRewardsDistributorcontract where users exercise call options to purchase ALTO tokens.The
_updateInTokenOneUnitFromUniswapNode()function determines which token from the Uniswap pool is the "in token" (ALTO) based on the configured direction:function _updateInTokenOneUnitFromUniswapNode() internal { address inToken = uniswapNode.direction == ConversionDirection.IN_TO_OUT ? IUniswapV3Pool(uniswapNode.pool).token0() : IUniswapV3Pool(uniswapNode.pool).token1(); inTokenOneUnit = 10 ** IERC20Metadata(inToken).decimals();}This function is called during initialization and whenever the Uniswap node configuration is updated. However, there is no validation to ensure that the determined
inTokenis actually the ALTO token that the oracle is intended to price.If the direction is configured incorrectly, the oracle will:
- Use the wrong token's decimals to calculate
inTokenOneUnit - Request a price quote for the wrong token from the Uniswap pool
- Return an inverted price (e.g., how much ALTO you can buy for 1 USDC instead of how much USDC you need to buy 1 ALTO)
The oracle's
getPrice()function is used by theAltoRewardsDistributorto calculate payment amounts:// In AltoRewardsDistributor._calculatePaymentAmount()uint256 rewardPriceValue = IUsdOracle(rewardTokenUsdOracle).getPrice();// ... price calculation logic ...An inverted price would cause users to either pay drastically too much or too little when exercising their call options, depending on which direction the inversion occurs.
Recommendation
Add validation in
_updateInTokenOneUnitFromUniswapNode()to ensure the determined token matches the expected ALTO token address:function _updateInTokenOneUnitFromUniswapNode() internal { address token0 = IUniswapV3Pool(uniswapNode.pool).token0(); address token1 = IUniswapV3Pool(uniswapNode.pool).token1(); address inToken = uniswapNode.direction == ConversionDirection.IN_TO_OUT ? token0 : token1; address outToken = uniswapNode.direction == ConversionDirection.IN_TO_OUT ? token1 : token0; // Validate that inToken appears to be ALTO and outToken appears to be a stablecoin // This could check token symbols, known addresses, or other heuristics if (!_isAltoToken(inToken) || !_isEther(outToken)) { revert AltoRewardsOracleInvalidDirection(); } inTokenOneUnit = 10 ** IERC20Metadata(inToken).decimals();}Mutable oracle configurations increase risk of misconfiguration
State
- Acknowledged
Severity
- Severity: Informational
Submitted by
phaze
Description
The oracle contracts expose owner-controlled functions that allow updating critical configuration parameters after deployment. Both
AltoMultiChainlinkLendingOracleandAltoRewardsOracleinclude functions to modify their Chainlink and Uniswap data sources:In AltoMultiChainlinkLendingOracle:
function updateChainlinkNodes(ChainlinkNode[] memory _chainlinkNodes) external onlyOwner { if (_chainlinkNodes.length == 0) { revert AltoMultiChainlinkLendingOracleInvalidInput(); } emit SetChainlinkNodes(chainlinkNodes, _chainlinkNodes); delete chainlinkNodes; for (uint256 i = 0; i < _chainlinkNodes.length; i++) { _validateChainlinkParams(_chainlinkNodes[i]); chainlinkNodes.push(_chainlinkNodes[i]); }}In AltoRewardsOracle:
function updateChainlinkNode(ChainlinkNode memory _chainlinkNode) external onlyOwner { _validateChainlinkParams(_chainlinkNode); emit SetChainlinkNode(chainlinkNode, _chainlinkNode); chainlinkNode = _chainlinkNode;} function updateUniswapNode(UniswapNode memory _uniswapNode) external onlyOwner { _validateUniswapParams(_uniswapNode); emit SetUniswapNode(uniswapNode, _uniswapNode); uniswapNode = _uniswapNode; _updateInTokenOneUnitFromUniswapNode();}While these functions include parameter validation, they allow the owner to fundamentally change the oracle's data sources at any time. This creates several considerations:
Configuration Complexity: Oracle configurations are referenced in multiple places (the oracle contracts themselves, consumer contracts like
AltoRewardsDistributor, and deployment scripts), making it harder to track what data sources are actually being used.Immediate Price Changes: Changing oracle configurations causes immediate price updates. Even with proper validation, these changes happen atomically and any downstream contracts consuming the oracle will immediately see different prices.
Multi-Contract Coordination: If multiple oracles need coordinated updates, there's no atomic way to update them all simultaneously, potentially leaving the protocol in an inconsistent state between updates.
Recommendation
Consider removing update functions from oracle contracts and handling configuration changes by deploying new oracle contracts and updating references at the consumer contract level, e.g. in AltoRewardsDistributor or similar consumer contracts. This approach provides a single source of configuration in consumer contracts, makes oracle changes more explicit through new deployments, and allows new oracles to be tested before activation.
Uniswap oracle rounds down when calculating prices used for user payments
Description
The
_getQuoteAtTick()function inOracleMath.soluses floor division when calculating price quotes from Uniswap tick data. When these prices are used to determine payment amounts that users must transfer to the protocol (as inAltoRewardsDistributor.claimRewardTokens()), rounding down results in users paying slightly less than they should.The function currently implements:
function _getQuoteAtTick(int24 tick, uint256 baseAmount, ConversionDirection direction) internal pure returns (uint256 quoteAmount){ uint256 ratio = _getRatioAtTick(tick); quoteAmount = (direction == ConversionDirection.IN_TO_OUT) ? _mulDiv(ratio, baseAmount, 1e18) : _mulDiv(1e18, baseAmount, ratio);}The
_mulDiv()function fromFullMathperforms floor division, which rounds down to the nearest integer. This is called through the following chain:AltoRewardsDistributor.claimRewardTokens()calls_calculatePaymentAmount()_calculatePaymentAmount()retrieves the ALTO token price viaIUsdOracle(rewardTokenUsdOracle).getPrice()AltoRewardsOracle.getPrice()callsreadUniswapPool(inTokenOneUnit, uniswapNode)readUniswapPool()calls_getQuoteAtTick()with the time-weighted average tick
When the ALTO price is calculated with floor division, it returns a slightly lower value than the true price. Subsequently, when
_calculatePaymentAmount()computes how much users need to pay, this undervalued price results in a lower payment amount.While
_calculatePaymentAmount()does round up in its own calculations to avoid zero payments, this doesn't compensate for the initial rounding down in the oracle price itself:// In _calculatePaymentAmount()uint256 rewardPriceValue = IUsdOracle(rewardTokenUsdOracle).getPrice(); // Already rounded downuint256 tokenPrice = IUsdOracle(paymentTokenConfig.oracle).getPrice(); uint256 rewardToPurchaseCostUsd = rewardsPurchaseAmount.divideWithRounding( rewardPriceValue, // Uses the already-rounded-down price 10 ** REWARD_TOKEN.decimals, true // round up to avoid zero payment);In DeFi protocols, the standard practice is to round in favor of the protocol when users are making payments (round up) and in favor of users when they are receiving funds (round down). While individual rounding losses are small (typically 1 wei or a few wei per transaction), they can be exacerbated under certain circumstances.
Recommendation
Consider modifying the oracle calculation to support rounding up when the price will be used for user payments. This could be implemented by:
Option 1: Add a rounding parameter to
_getQuoteAtTick():function _getQuoteAtTick( int24 tick, uint256 baseAmount, ConversionDirection direction, bool roundUp) internal pure returns (uint256 quoteAmount){ uint256 ratio = _getRatioAtTick(tick); if (direction == ConversionDirection.IN_TO_OUT) { quoteAmount = roundUp ? _mulDivRoundingUp(ratio, baseAmount, 1e18) : _mulDiv(ratio, baseAmount, 1e18); } else { quoteAmount = roundUp ? _mulDivRoundingUp(1e18, baseAmount, ratio) : _mulDiv(1e18, baseAmount, ratio); }} function _mulDivRoundingUp(uint256 a, uint256 b, uint256 denominator) internal pure returns (uint256 result) { result = _mulDiv(a, b, denominator); if (mulmod(a, b, denominator) > 0) { result++; }}Option 2: Round up consistently in oracle operations:
Since the oracle is primarily used for pricing assets in the rewards distributor (where users pay), consider always rounding up in
_getQuoteAtTick():function _getQuoteAtTick(int24 tick, uint256 baseAmount, ConversionDirection direction) internal pure returns (uint256 quoteAmount){ uint256 ratio = _getRatioAtTick(tick); if (direction == ConversionDirection.IN_TO_OUT) { quoteAmount = _mulDiv(ratio, baseAmount, 1e18); if (mulmod(ratio, baseAmount, 1e18) > 0) { quoteAmount++; } } else { quoteAmount = _mulDiv(1e18, baseAmount, ratio); if (mulmod(1e18, baseAmount, ratio) > 0) { quoteAmount++; } }}The first option provides more flexibility if the oracle is used in different contexts with different rounding requirements, while the second option is simpler if the oracle's primary use case is for pricing in payment calculations.
Protocol assumptions about ratio growth and rounding directions could be documented explicitly
Severity
- Severity: Informational
Submitted by
phaze
Description
The protocol makes important assumptions about the behavior of underlying assets and rounding directions that are not explicitly documented in the code. These implicit assumptions affect system safety and correctness, and their absence could lead to integration errors or protocol misuse.
Assumption 1: Monotonically Increasing Ratios
The ERC4626 ratio capping mechanism in
ModuleERC4626Ratio.solassumes that wrapped asset ratios (like wstETH/stETH, rETH/ETH, sUSDe/USDe) always increase over time and never decrease:function readLatestRatio(uint256 latestRatio, ERC4626RatioNode memory _erc4626RatioNode) public view returns (uint256){ uint256 currentRatio = latestRatio; if (currentRatio == 0) return 0; uint256 maxRatio = _computeMaxRatio(_erc4626RatioNode); if (currentRatio > maxRatio) { return maxRatio; // Caps growth but doesn't enforce a minimum } else { return currentRatio; }}The function caps the maximum ratio growth to prevent manipulation, but it does not enforce a minimum ratio. This design implicitly assumes that ratios never decrease because:
- For liquid staking tokens (wstETH, rETH, mETH), the ratio represents accumulated staking rewards, which only grows
- For yield-bearing tokens (sUSDe), the ratio represents accrued interest, which only increases
- Decreasing ratios would indicate slashing events or protocol losses
However, this assumption is nowhere documented in the code or comments.
Assumption 2: Protocol-Favorable Rounding Directions
The protocol makes different rounding decisions based on whether users are depositing or borrowing, but these decisions are not consistently documented:
In lending/collateral scenarios (protocol should round down):
- Users deposit collateral and borrow against it
- Oracle prices should round down to avoid overvaluing collateral
- This keeps the protocol solvent by being conservative about borrowing capacity
- Example:
ModuleERC4626Ratio.readLatestRatio()returns capped values that prevent over-valuation
In payment scenarios (protocol should round up):
- Users pay to acquire protocol tokens (like in AltoRewardsDistributor)
- Prices should round up to ensure users pay enough
- This prevents value leakage from accumulated rounding losses
- Current behavior: The Uniswap oracle rounds down, which is incorrect for this use case
Assumption 3: Directional Usage of Collateral
The protocol assumes users only:
- Deposit collateral → Borrow DUSD (not the reverse)
- Pay DUSD/stablecoins → Acquire ALTO tokens (not the reverse)
If the protocol were to support the opposite flow (depositing DUSD to borrow underlying collateral), the rounding directions would need to be inverted. For example:
- If users could deposit DUSD and borrow wstETH, the oracle should round up the collateral value to prevent under-collateralization
- If users could sell ALTO tokens for stablecoins, the oracle should round down the ALTO price to protect the protocol
The current implementation doesn't document these usage patterns, making it easy for future integrators to misuse the oracle functions with incorrect rounding.
Recommendation
Add thorough documentation to make protocol assumptions explicit:
1. Document ratio monotonicity assumptions:
/// @title Module ERC4626 Ratio/// @notice Utility contract used to read the ratio of ERC4626 vaults and LSTs with growth caps/// @dev IMPORTANT ASSUMPTION: This module assumes that all underlying asset ratios are /// monotonically increasing (never decrease). This holds true for:/// - Liquid staking tokens (wstETH, rETH, mETH) where ratios represent accumulated rewards/// - Yield-bearing tokens (sUSDe) where ratios represent accrued interest/// @dev If an asset experiences slashing or losses causing the ratio to decrease, this /// module will report the decreased value without safeguards. Such assets should either:/// 1. Not be integrated into the protocol, or/// 2. Use a different oracle mechanism that includes minimum ratio enforcementcontract ModuleERC4626Ratio { // ... /// @notice Returns the current capped ratio /// @dev ROUNDING: Always returns floor value (rounds down) to avoid overvaluing collateral /// @dev This is appropriate for lending scenarios where users borrow against this collateral function readLatestRatio(uint256 latestRatio, ERC4626RatioNode memory _erc4626RatioNode) public view returns (uint256) { // ... }}2. Document rounding directions:
/// @notice Gets a quote for an amount of input currency and returns the output currency/// @param quoteAmount The input currency amount to get the price of in output currency/// @param config Uniswap configuration/// @return The value of the `quoteAmount` in output currency/// @dev ROUNDING: This function uses floor division (rounds down)/// @dev APPROPRIATE USES:/// - Pricing collateral in lending protocols (conservative valuation)/// - Calculating amounts when USERS RECEIVE tokens/// @dev INAPPROPRIATE USES:/// - Pricing assets when users make payments (use ceiling division instead)/// - Any scenario where protocol should receive more rather than lessfunction readUniswapPool(uint256 quoteAmount, UniswapNode memory config) public view returns (uint256){ // ...}3. Add usage guidelines:
/// @title Alto Protocol Oracle Usage Guidelines/// @notice This document describes the correct usage of oracle functions based on rounding requirements////// LENDING/COLLATERAL SCENARIOS (round DOWN):/// - When valuing user collateral deposits/// - When calculating maximum borrowing capacity/// - Goal: Be conservative to keep protocol solvent/// - Use: AltoMultiChainlinkLendingOracle, ModuleERC4626Ratio (as-is)////// PAYMENT SCENARIOS (round UP):/// - When calculating payment amounts users must transfer/// - When pricing tokens users are purchasing/// - Goal: Prevent value leakage from accumulated rounding/// - Use: Modify oracle to support ceiling division, or round up at consumer level////// REVERSE FLOWS (not currently supported):/// - If protocol adds: deposit DUSD → borrow collateral/// Then: Oracle must round UP collateral value/// - If protocol adds: sell ALTO → receive stablecoins /// Then: Oracle must round DOWN ALTO price4. Add invariant documentation:
/// @notice Validates and stores the ERC4626 ratio configuration/// @dev INVARIANT: initialRatio must be ≤ current on-chain ratio/// @dev INVARIANT: Over time, ratio should only increase (monotonicity assumption)/// @dev SECURITY: If ratio decreases, it indicates asset losses/slashing - should trigger alertsfunction _validateERC4626RatioParams(ERC4626RatioNode memory _erc4626RatioNode) internal view { // ...}This documentation would make the protocol's assumptions explicit, helping auditors verify correctness and preventing future integrators from misusing oracle functions with incorrect rounding directions.
Gas Optimizations1 finding
Chainlink feed decimals could be cached to reduce external calls
State
- Acknowledged
Severity
- Severity: Gas optimization
Submitted by
phaze
Description
The
readChainlinkFeed()function inModuleChainlink.solcallsconfig.feed.decimals()on every invocation to retrieve the Chainlink feed's decimal precision:function readChainlinkFeed(uint256 quoteAmount, ChainlinkNode memory config) public view returns (uint256) { (uint80 roundId, int256 ratio,, uint256 updatedAt, uint80 answeredInRound) = config.feed.latestRoundData(); if (ratio <= 0 || roundId > answeredInRound || block.timestamp - updatedAt > config.stalePeriod) { revert ModuleChainlinkInvalidRate(); } // External call to get decimals uint256 price = uint256(ratio); uint256 powFeed = 10 ** config.feed.decimals(); // <-- External call on every read uint256 powIn = 10 ** config.inDecimals; uint256 powOut = 10 ** config.outDecimals; // ... price calculation ...}This external call to retrieve the feed's decimals occurs on every price read operation. Since Chainlink feed decimals are immutable and never change after deployment, this value could be cached in the
ChainlinkNodeconfiguration struct and validated once during initialization.Recommendation
Store the feed decimals directly in the
ChainlinkNodestruct and validate it during configuration setup:Update the ChainlinkNode struct:
struct ChainlinkNode { AggregatorV3Interface feed; ConversionDirection direction; uint32 stalePeriod; uint8 inDecimals; uint8 outDecimals; uint8 feedDecimals; // Add cached feed decimals}Validate the cached value matches the actual feed:
function _validateChainlinkParams(ChainlinkNode memory config) internal view { if (address(config.feed) == address(0) || config.stalePeriod == 0) { revert ModuleChainlinkInvalidInput(); } // Validate that cached feedDecimals matches actual feed decimals if (config.feedDecimals != config.feed.decimals()) { revert ModuleChainlinkInvalidInput(); } // Bound decimals to prevent overflow and DoS via extreme values if (config.inDecimals + config.outDecimals > 54) { revert ModuleChainlinkInvalidInput(); }}Use the cached value in readChainlinkFeed:
function readChainlinkFeed(uint256 quoteAmount, ChainlinkNode memory config) public view returns (uint256) { (uint80 roundId, int256 ratio,, uint256 updatedAt, uint80 answeredInRound) = config.feed.latestRoundData(); if (ratio <= 0 || roundId > answeredInRound || block.timestamp - updatedAt > config.stalePeriod) { revert ModuleChainlinkInvalidRate(); } uint256 price = uint256(ratio); uint256 powFeed = 10 ** config.feedDecimals; // Use cached value instead of external call uint256 powIn = 10 ** config.inDecimals; uint256 powOut = 10 ** config.outDecimals; // ... rest of function ...}This optimization eliminates one external call per oracle read, reducing gas costs for all contracts that consume these oracle prices. The one-time validation during initialization ensures the cached value remains accurate.