OpenZeppelin audited the v4-oracle-hook repository at commit 0575d1a.
In scope were the following files:
src
├── adapters
│ ├── V3OracleAdapter.sol
│ └── V3TruncatedOracleAdapter.sol
├── libraries
│ └── Oracle.sol
└── V3StyleOracleHook.sol
During the fix review, the team renamed and refactored the V3StyleOracleHook contract. The changes did not affect its logic. As a result of the refactoring, the contract was replaced by two new contracts: BaseOracleHook and OracleHookWithV3Adapters. These changes, introduced at commit d441bce, were also audited.
V3StyleOracleHook is a Uniswap V4 hook that mimics the oracle functionality of Uniswap V3. It can be inserted into a Uniswap V4 pool to track price data and expose a Uniswap V3-compatible oracle interface. The hook is designed to run only before swaps.
In Uniswap V3, each pool can serve as a price oracle by maintaining a tick accumulator, which is a time-weighted sum of the pool's active ticks. This accumulator is updated at most once per block—specifically before the first interaction with the pool for the block—and multiple historical snapshots (up to 65,535) are stored. This mechanism allows for the computation of a time-weighted geometric mean price over a specified period.
Uniswap V4 does not natively include oracle support. V3StyleOracleHook introduces this functionality by replicating V3's tick accumulator logic. It updates this tick accumulator right before the first swap in a block. It employs a centralized hook to manage multiple pools via the PoolManager contract, unlike V3’s independent per-pool contracts. In addition to this standard tick accumulator, V3StyleOracleHook maintains a truncated tick accumulator, inspired by Uniswap's TruncGeoOracle hook. The truncated oracle caps tick changes to +/- maxAbsTickDelta to mitigate manipulation. This provides a defense mechanism against an oracle manipulation by smoothing the influence of large tick changes.
The hook is intentionally designed to only execute before swaps, as both Uniswap V4 and V3 only update ticks during swaps. Oracle updates occur before swaps, aligning with V3’s pre-swap updates for accurate TWAPs. However, this choice affects, in particular, the truncated tick accumulator logic because of its non-linear nature. V3StyleOracleHook's results may diverge significantly from similar implementations like the TruncGeoOracle hook, which also runs before liquidity deposits. However, since liquidity deposits are relatively rare compared to swaps, the difference is expected to be minimal in practice.
In Uniswap V4, hooks can also be used to set a dynamic fee and also modify it per swap. However, the V3StyleOracleHook implementation of _afterInitialize function does not have any logic for setting a dynamic fee, and _beforeSwap always returns 0 for lpFeeOverride. Therefore, if a pool is intended to apply a dynamic fee that changes per swap, it should override the _afterInitialize and _beforeSwap functions of V3StyleOracleHook and add this extra logic.
The V3StyleOracleHook contract utilizes a modified version of Uniswap V3's Oracle library for updating and retrieving tick accumulator observations. When a pool with V3StyleOracleHook is deployed, two adapter contracts are also deployed: V3OracleAdaper and V3TruncatedOracleAdapter. These adapters expose Uniswap V3-compatible interfaces for accessing the standard and truncated tick accumulators, respectively.
While the Uniswap V3 oracle also tracks the seconds per liquidity, this is not tracked by V3StyleOracleHook. The observe function returns 0 for secondsPerLiquidityCumulativeX128s, a value integrators should disregard, as V4 does not track this metric. The adapters return a default value of zero for this field, just to maintain interface compatibility. This behavior is clearly documented in the comments, and integrators should be careful and not use this value. The slot0 function returns sqrtPriceX96, tick, observation data, and simplified unlocked and feeProtocol flags in a V3-compatible structure.
There are no privileged roles in the contracts. The V3StyleOracleHook contract is deployed and can be integrated by any Uniswap V4 hook. No changes can be made to the hook contract after deployment. The only parameter set by the deployer is the MAX_ABS_TICK_DELTA, which affects the truncated tick accumulator and is immutable. It is expected that multiple versions of the hook contract will be deployed, each with a different value for this parameter. Pools can then choose the version that best fits their expected profile.
slot0 Ignores the Actual Protocol Fee and Lock State of the PoolThe slot0 function in both the V3OracleAdapter and V3TruncatedOracleAdapter contracts includes a feeProtocol variable and a boolean variable called unlocked for compatibility with the Uniswap V3 interface. However, it always returns the default values for them (0 and true, respectively). This behavior is justified in the comment above the function stating that in the Uniswap V4 protocol, fees are zero, and pools are always considered unlocked. However, this is not entirely accurate.
While Uniswap V4 has been deployed with a zero protocol fee, the protocol includes built-in functionality to configure a protocolFeeController address, which may set a non-zero protocol fee for any pool at any time. Also, the Uniswap V4 PoolManager, and therefore all the pools it controls, is locked during the execution of swaps, liquidity changes, and some other actions, and is unlocked only afterward as a protection against reentrancy.
Consider returning the actual feeProtocol and unlocked status of the PoolManager, allowing the caller of slot0 to determine whether the data they received are safe to use or not.
Update: Acknowledged, not resolved.
In V3StyleOracleHook, the increaseObservationCardinalityNext function can be called by anyone to increase the maximum number of oracle observations the hook can store for a given pool. This value is designed to only increase over time. However, this function can be called even before the pool is created and initialized via the PoolManager contract, since a user can precompute the poolId. In such cases:
observationCardinalityNext has been increased.This creates a misleading history of events, contradicting the intention that the cardinality should only increase, and introduces potential confusion to the users. Also, there is no explicit check that prevents calling V3StyleOracleHook's observe function before the initialization of the pool. In such cases, the returned values will be the default zero, that is not safe to use.
Consider restricting the ability to call increaseObservationCardinalityNext and observe so that they only become callable after the pool has been initialized.
Update: Partially resolved in pull request #1. The team added a check to prevent calling increaseObservationCardinalityNext before the pool is initialized, but decided to leave observe as is.
The documentation for the observe function in line 174 of the V3StyleOracleHook contract contains a misleading statement regarding its second return value. This comment incorrectly identifies the second return value as "seconds per liquidity" and claims that it always returns 0. However, the second @return tag correctly identifies this second array as "Truncated cumulative tick values as of each secondsAgos from the current block timestamp".
Consider addressing the above-mentioned instance of misleading documentation to improve the clarity and maintainability of the codebase.
Update: Resolved in pull request #1.
PoolId Parameter in EventThe V3StyleOracleHook contract is designed to track the prices of different liquidity pools. When this contract emits the IncreaseObservationCardinalityNext event without a PoolId, any off-chain system listening for these events faces ambiguity. It knows an increase happened, but it does not know which pool it applies to.
Consider adding an indexed PoolId parameter to IncreaseObservationCardinalityNext to facilitate proper tracking as V3StyleOracleHook could emit the same event for different pools.
Update: Resolved in pull request #1.
V3StyleOracleHookThe V3StyleOracleHook contract implements some logic related to the Pool Manager that is redundant, as equivalent logic is already present in the inherited ImmutableState contract:
NotManager error, which is equivalent to the NotPoolManager errormanager public immutable variable, which is equivalent to the poolManager public immutable variableonlyByManager modifier, which is equivalent to the onlyPoolManager modifierFurthermore, the V3StyleOracleHook functions that are meant to be called by the Pool Manager perform redundant checks to verify the caller. The entry points afterInitialize and beforeSwap already ensure that these functions can only be called by the Pool Manager.
To improve code clarity and reduce redundancy, consider deleting the duplicated elements and the redundant checks from V3StyleOracleHook and consistently using the inherited poolManager contract throughout the contract.
Update: Resolved in pull request #1.
In the ObservationState struct of the V3StyleOracleHook contract, the word 'observation' is redundant as it is repeated in each of the fields.
To improve code clarity and maintainability, consider keeping the same naming of the fields but removing the 'observation' word.
Update: Resolved in pull request #1.
Pragma directives should be fixed to clearly identify the Solidity version with which the contracts will be compiled.
Throughout the codebase, multiple instances of floating pragma directives were identified:
Oracle.sol has the solidity ^0.8.19 floating pragma directive.V3OracleAdapter.sol has the solidity ^0.8.19 floating pragma directive.V3StyleOracleHook.sol has the solidity ^0.8.19 floating pragma directive.V3TruncatedOracleAdapter.sol has the solidity ^0.8.19 floating pragma directive.Consider using fixed pragma directives.
require StatementsSince Solidity version 0.8.26, custom error support has been added to require statements. While, initially, this feature was only available through the IR pipeline, Solidity 0.8.27 extended support to the legacy pipeline as well.
Throughout the codebase, multiple instances of if-revert statements that could be replaced with require statements were identified:
if (!lte(time, beforeOrAt.blockTimestamp, target)) { revert TargetPredatesOldestObservation(beforeOrAt.blockTimestamp, target); } statement in Oracle.solif (msg.sender != address(manager)) revert NotManager() statement in V3StyleOracleHook.solFor conciseness and gas savings, consider replacing if-revert statements with require statements.
Throughout the codebase, multiple instances of contracts having an inconsistent ordering of functions were identified:
Oracle contract in Oracle.solV3StyleOracleHook contract in V3StyleOracleHook.solTo improve the project's overall legibility, consider standardizing ordering throughout the codebase as recommended by the Solidity Style Guide (Order of Functions).
Providing a specific security contact (such as an email or ENS name) within a smart contract significantly simplifies the process for individuals to communicate if they identify a vulnerability in the code. This practice is quite beneficial as it permits the code owners to dictate the communication channel for vulnerability disclosure, eliminating the risk of miscommunication or failure to report due to a lack of knowledge on how to do so. In addition, if the contract incorporates third-party libraries and a bug surfaces in those, it becomes easier for their maintainers to contact the appropriate person about the problem and provide mitigation instructions.
Throughout the codebase, multiple instances of contracts not having a security contact were identified:
Oracle libraryV3OracleAdapter contractV3StyleOracleHook contractV3TruncatedOracleAdapter contract.Consider adding a NatSpec comment containing a security contact above each contract definition.
Throughout the codebase, multiple instances of unused imports were identified:
import {Oracle} from "../libraries/Oracle.sol"; imports unused alias Oracle in V3OracleAdapter.sol.import {Oracle} from "../libraries/Oracle.sol"; imports unused alias Oracle in V3TruncatedOracleAdapter.sol.import {IHooks} from "v4-core/interfaces/IHooks.sol"; imports unused alias IHooks in V3StyleOracleHook.sol.Consider removing unused imports to improve the overall clarity and readability of the codebase.
Update: Resolved in pull request #1.
Throughout the codebase, multiple instances of incomplete docstrings were identified:
maxAbsTickDelta argument of the write function in Oracle is not present in the NatSpec docstring of the function.maxAbsTickDelta argument of the getSurroundingObservations function in Oracle is not present in the NatSpec docstring of the function.maxAbsTickDelta argument of the observe function in Oracle is not present in the NatSpec docstring of the function._manager argument of the constructor function in V3OracleAdapter is not present in the NatSpec docstring of the function._manager argument of the constructor function in V3TruncatedOracleAdapter is not present in the NatSpec docstring of the function.Consider completing any instances of incomplete docstrings to improve code clarity and maintainability.
Update: Resolved in pull request #1.
@inheritdoc NatSpec TagIn V3StyleOracleHook, the _beforeSwap and _afterInitialize functions use the @inheritdoc BaseHook tag to inherit documentation from the overridden functions in BaseHook. However, the overridden functions do not have NatSpec documentation, so no documentation is inherited.
Consider thoroughly documenting the functions using proper NatSpec documentation instead of inheriting it from BaseHook.
Update: Resolved in pull request #1.
The audited code implements a Uniswap V4 Hook that provides Uniswap V3-style regular and truncated Time-Weighted Average Price (TWAP) oracle functionality for associated pools.
No security vulnerabilities were identified during this review. However, discrepancies were noted between the Hook's interface and the standard Uniswap V3 TWAP oracle. In addition, recommendations were provided to enhance code readability and clarity, aiming to facilitate future audits, integrations, and development.
The Panoptic team is appreciated for being responsive throughout the audit period.