fix: enforce at the code level to not support hooks

[supercontracts]

Summary by CodeRabbit

• New Features

  • Pool key retrieval functionality is now publicly available, allowing direct access to configuration data for Uniswap V4 pools and supporting enhanced integration scenarios.

• Improvements

  • Introduced validation checks that prevent configuration of pools containing hooks, restricting pool setup to standard configurations only for improved system stability.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[octane-security-app]

Summary by Octane

New Contracts

No new contracts were added.

Updated Contracts

• MainnetController.sol: Added Uniswap pool key validation with no hooks requirement in the MainnetController smart contract.
• UniswapV4Lib.sol: The function getPoolKeyFromPoolId is now public, replacing the internal _getPoolKeyFromPoolId.

---

🔗 Commit Hash: dffca50

[coderabbitai]

Walkthrough

The changes introduce a new PoolKey validation in MainnetController to prevent pools with hooks from being configured, and refactor UniswapV4Lib by converting an internal poolKey retrieval function to a public getter function.

Changes

Cohort / File(s)	Summary
PoolKey Import & Hooks Validation src/MainnetController.sol	Added PoolKey import from Uniswap V4 core; introduced precondition in setUniswapV4TickLimits to derive PoolKey from poolId and require that poolKey.hooks equals zero address before tick parameter validation
PoolKey Retrieval Refactoring src/libraries/UniswapV4Lib.sol	Converted internal function _getPoolKeyFromPoolId to public getter function getPoolKeyFromPoolId; updated call sites throughout minting and swapping operations to use the new public function

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

• Verify hooks validation logic in MainnetController.setUniswapV4TickLimits correctly prevents pool configuration
• Confirm all internal call sites in UniswapV4Lib have been updated to use the new public getter
• Review exposure of getPoolKeyFromPoolId as public API to ensure no unintended side effects

Possibly related PRs

• PR #197: Both modify PoolKey derivation and usage between MainnetController and UniswapV4Lib, changing poolKey getter helpers and consolidating call sites
• PR #194: Both address UniswapV4 poolKey handling and MainnetController's V4 integration; this PR adds hooks validation while the related PR implements V4 swap flows that depend on PoolKey retrieval

Suggested reviewers

• lucas-manuel

Poem

🐰 A poolKey emerges from storage deep,
No hooks allowed—this promise we keep!
From internal shadows to public light,
The getter now shines, refactored just right! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title 'fix: enforce at the code level to not support hooks' accurately describes the main changes, which add a precondition to prevent pools with hooks from being configured.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/enfore-no-hooks

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 61cbc57 and dffca50.

📒 Files selected for processing (2)

• src/MainnetController.sol (2 hunks)
• src/libraries/UniswapV4Lib.sol (3 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: coverage
• GitHub Check: test
• GitHub Check: build
• GitHub Check: Octane automated security check

🔇 Additional comments (3)

src/libraries/UniswapV4Lib.sol (2)

56-56: LGTM: Refactor to use public getter.

The change from internal _getPoolKeyFromPoolId to the new public getPoolKeyFromPoolId maintains the same functionality while improving code reusability.

Also applies to: 179-179

---

246-248: The bytes32 to bytes25 cast is correct and follows Uniswap V4's standard implementation.

The PositionManager's poolKeys mapping expects bytes25, and the poolId is truncated to 25 bytes to lookup PoolKey in the poolKeys mapping. This is the standard conversion used in Uniswap V4: bytes25 _poolId = bytes25(PoolId.unwrap(_poolKey.toId())).

src/MainnetController.sol (1)

13-13: LGTM: Required import for hook validation.

The PoolKey import is necessary for the new hook validation logic introduced in setUniswapV4TickLimits.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Hook validation is incomplete - critical gap in swap operations.

While this validation correctly prevents configuring tick limits for pools with hooks, it doesn't prevent swaps on such pools. The swapUniswapV4 function (line 719-738) relies on maxSlippages[address(uint160(uint256(poolId)))] which is set via setMaxSlippage (line 267-274), bypassing this hook check entirely.

Attack scenario:

1. Admin sets maxSlippage for a pool with hooks (no validation)
2. Relayer calls swapUniswapV4 with that poolId
3. Swap executes on a hooked pool, violating the "no hooks" requirement

🔎 Recommended fix: Add hook validation to swap operations

Option 1: Add validation in UniswapV4Lib.swap function

In src/libraries/UniswapV4Lib.sol, add hook validation after retrieving the poolKey:

 function swap(
     address proxy,
     address rateLimits,
     bytes32 poolId,
     address tokenIn,
     uint128 amountIn,
     uint128 amountOutMin,
     uint256 maxSlippage
 )
     external
 {
     require(maxSlippage != 0, "MC/max-slippage-not-set");

     PoolKey memory poolKey = getPoolKeyFromPoolId(poolId);
+
+    require(address(poolKey.hooks) == address(0), "MC/pool-has-hooks");

     require(
         tokenIn == Currency.unwrap(poolKey.currency0) ||

Option 2: Add validation in setMaxSlippage for Uniswap V4 pools

Modify setMaxSlippage to detect and validate Uniswap V4 poolIds. However, this is more complex as you'd need to distinguish pool types.

Recommendation: Option 1 is cleaner and ensures runtime protection.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In src/MainnetController.sol around lines 335-337 the code checks PoolKey memory
poolKey = UniswapV4Lib.getPoolKeyFromPoolId(poolId);
require(address(poolKey.hooks) == address(0), "MC/pool-has-hooks"); but swap
paths still allow swaps against pools with hooks via swapUniswapV4 (lines
~719-738) because maxSlippage is set elsewhere; to fix, add the same hooks
validation inside the UniswapV4Lib.swap implementation immediately after
retrieving the poolKey (or at the start of swapUniswapV4) so swaps revert for
hooked pools; use the same revert message/semantic ("MC/pool-has-hooks") and
ensure the check uses address(poolKey.hooks) == address(0) to block any pool
with nonzero hooks.

[octane-security-app]

Overview

Vulnerabilities found:	11

Warnings found:	10

---

🔗 Commit Hash: dffca50
🛡️ Octane Dashboard: All vulnerabilities