Organization
- @euler-xyz
Engagement Type
Cantina Reviews
Period
-
Repositories
N/A
Findings
Medium Risk
3 findings
3 fixed
0 acknowledged
Low Risk
6 findings
6 fixed
0 acknowledged
Informational
6 findings
5 fixed
1 acknowledged
Medium Risk3 findings
Unsafe Token Transfer
Description
In the
FundsLib
contract, the transfer of tokens to the protocol fee recipient is performed using the standard ERC-20transfer()
function:IERC20(asset).transfer(p.protocolFeeRecipient, protocolFeeAmount);This approach assumes compliance with the ERC-20 specification, including the return of a boolean success value. However, many tokens in the Ethereum ecosystem, such as USDT and others, do not strictly follow the ERC-20 standard and either do not return a value or behave inconsistently.
Recommendation
Replace the use of
IERC20(asset).transfer(...)
with OpenZeppelin’sSafeERC20.safeTransfer(...)
, which safely handles non-compliant tokens by suppressing return value decoding and inferring success from the absence of reverts. This wrapper ensures maximum compatibility across token implementations.Euler: Fixed in PR#74.
Cantina: Verified.
Incorrect Parameter Ordering in Curve Evaluation
Description
In the
EulerSwap
swap logic, the call toCurveLib.f
is made with an incorrect ordering of parameters during the evaluation of the post-swap state. The functionf()
is defined to compute a new reserve value based on the curve. However, theEulerSwap
contract incorrectly inverts the order ofpx
andpy
in some branches of its logic:- yNew = CurveLib.f(xNew, py, px, y0, x0, cx);+ yNew = CurveLib.f(xNew, px, py, x0, y0, cx);This misordering distorts the price weight applied in the curve’s internal calculations, leading to incorrect reserve updates and swap results that violate the intended AMM curve. Specifically, this causes inconsistencies in the swap symmetry: when executing an exact-out swap followed by an exact-in reversal using the same token pair and path, the system fails to return to the original state — demonstrating loss or unintended gain.
This error was clearly observed in simulated outputs. For instance, under the following parameter set:
priceX = 1priceY = 2x0 = 1000y0 = 1000cX = 0.8cY = 0.8reserve0 = 1000reserve1 = 1000amount = 500asset0IsInput = FalseexactIn = FalseThe uncorrected logic produced:
Starting reserves: (1000, 1000) New Reserve: (500.00, 2200.00) Output: 1200.00
Attempting to reverse this swap using exact input
1200
withexactIn = True
should have brought the reserves back to(1000,1000)
. Instead, the incorrect computation yielded:New Reserve: (106.11, 2200.00) Output: 893.89
This asymmetry stems from misaligned price parameters in the curve formula. After correcting the line the outputs aligned as expected. The output of the exact-out leg becomes:
New Reserve: (500.00, 1300.50) Output: 300.50
and the exact-in reversal with
amount = 300
restores the reserves to:New Reserve: (500.00, 1300.00) Output: 500.00
This confirms that the pricing logic and reserve transitions now obey the intended curve dynamics and yield symmetry across swap paths.
Visual validation using the provided plot script (see below) further illustrates the discrepancy. The invalid version of the logic plots a destination point far outside the feasible curve region:
Whereas the corrected computation yields a valid, curve-consistent transition:
Recommendation
All calls to
CurveLib.f()
andCurveLib.fInverse()
must be carefully reviewed to ensure that parameters are passed in the correct order, specifically:- yNew = CurveLib.f(xNew, py, px, y0, x0, cx);+ yNew = CurveLib.f(xNew, px, py, x0, y0, cx);In particular, care must be taken when switching between x-based and y-based flows to ensure that price tokens and reserve coordinates are not accidentally flipped. Consider isolating the curve application logic into well-named wrapper functions that enforce correct parameter order depending on swap direction, reducing the surface area for future human error.
Further, add test cases and simulation plots that validate the invertibility of swaps under various exact-in/exact-out scenarios across curve segments. Plotting the reserve trajectory with respect to the invariant is highly recommended for debugging and internal QA.
Euler: Fixed in PR#67
Cantina: Verified.
Removing pools with swapAndPop() in EulerSwapFactory.uninstall() corrupts stored indexes
Context:
Description:
When deploying a new pool, the index of the pool's address in the
allPools
andpoolMap
arrays are stored:eulerAccountState[eulerAccount] = EulerAccountState({pool: newOperator,allPoolsIndex: uint48(allPools.length),poolMapIndex: uint48(poolMapArray.length)});When uninstalling a pool with
uninstall()
,swapAndPop()
is used to remove pool addresses from theallPools
/poolMap
array:function swapAndPop(address[] storage arr, uint256 index) internal {arr[index] = arr[arr.length - 1];arr.pop();}However, this causes the index stored in
eulerAccountState
for the last pool address to become incorrect.For example:
- Assume two pools are installed, so
allPools = [0x11, 0x22]
- For the second pool,
allPoolsIndex = 1
- The first pool is uninstalled, so
allPools = [0x22]
- Since
allPoolsIndex
is not updated, it is now invalid.
When the second pool is uninstalled, it will remove the wrong index in the
allPools
array. As a result, it will be impossible for accounts to uninstall pools.The following PoC demonstrates how the second pool cannot be uninstalled due to an OOB access:
// SPDX-License-Identifier: GPL-2.0-or-laterpragma solidity ^0.8.24;import "forge-std/Test.sol";import {EulerSwapTestBase, IEulerSwap} from "test/EulerSwapTestBase.t.sol";contract EulerSwapFactoryTest is EulerSwapTestBase {function test_multipleUninstalls() public {// Usersaddress alice = makeAddr("alice");address bob = makeAddr("bob");// Parameters for deployPool()IEulerSwap.Params memory params = getEulerSwapParams(1e18, 1e18, 1e18, 1e18, 0, 0, 0, 0, address(0));IEulerSwap.InitialState memory initialState = IEulerSwap.InitialState(1e18, 1e18);bytes32 salt = bytes32(0);// Deploy pool for Aliceparams.eulerAccount = alice;address alicePool = eulerSwapFactory.computePoolAddress(params, salt);vm.startPrank(alice);evc.setAccountOperator(alice, alicePool, true);eulerSwapFactory.deployPool(params, initialState, salt);// Deploy pool for Bobparams.eulerAccount = bob;address bobPool = eulerSwapFactory.computePoolAddress(params, salt);vm.startPrank(bob);evc.setAccountOperator(bob, bobPool, true);eulerSwapFactory.deployPool(params, initialState, salt);// Uninstall pool for Alicevm.startPrank(alice);evc.setAccountOperator(alice, alicePool, false);eulerSwapFactory.uninstallPool();// Uninstalling pool for Bob reverts due to an OOB access of the allPools arrayvm.startPrank(bob);evc.setAccountOperator(bob, bobPool, false);vm.expectRevert(stdError.indexOOBError);eulerSwapFactory.uninstallPool();}}Recommendation:
Consider storing pools with OpenZeppelin's
EnumerableSet
instead of arrays. The pools can be removed from the set based on their address instead of index.Euler: Fixed in PR 66.
Cantina: Verified, the recommendation has been implemented.
- Assume two pools are installed, so
Low Risk6 findings
Potential Re-Entrancy via uniswapV2Call Hook in _beforeSwap
Description
In the
UniswapHook
contract, the_beforeSwap()
function is invoked as a pre-swap hook in theEulerSwap
architecture. Within this function, important mutable state — specificallys.reserve0
ands.reserve1
— is accessed and potentially acted upon. Simultaneously, the mainEulerSwap.swap()
function does not follow a strict checks-effects-interactions pattern: it invokes theuniswapV2Call
callback before updating the reserve values.This ordering opens a subtle but dangerous surface for re-entrancy attacks via Uniswap-style flash loan callbacks. Since
s.reserve0
ands.reserve1
are only written after the callback returns, a malicious contract could re-enterswap()
from withinuniswapV2Call()
and trigger_beforeSwap()
again with the stale reserves — possibly manipulating them further, creating inconsistencies, or even circumventing pricing invariants.While no concrete exploit has been identified in the current implementation, this behavior creates an attack surface for reserve desynchronization or double-usage, especially when interacting with other composable DeFi primitives (e.g., nested swaps, lending hooks, callbacks with
delegatecall
, etc.).Recommendation
Protect
_beforeSwap
by applying the existingnonReentrantHook
modifier usingCtxLib.Storage
and thes.status
field. This ensures the hook cannot be re-entered mid-swap and preserves reserve integrity.Euler: Fixed in PR#77.
Cantina: Verified. The re-entrancy lock was added.
Unrestricted Donations via Uniswap V4 Integration with EulerSwap Hook
Description
In the current design,
EulerSwap
acts as a Uniswap V4-style hook, but it does not override or restrict the_beforeDonate
function. While these donations do not affectEulerSwap
's internal reserve tracking, they do affect the underlying Uniswap V4 pool contract, which receives and accounts for these tokens.This introduces a mismatch: tokens can be donated directly to the Uniswap V4 pool contract during swap flows or hook invocations without passing through
EulerSwap
's intended control path. These donations can cause offsets in Uniswap V4’s internal balance tracking, leading to incorrect tick updates, mispriced liquidity ranges, and potentially broken accounting. SinceEulerSwap
does not mirror or react to these changes, the system ends up with inconsistent reserve views between the hook and the core pool.The issue is made more dangerous by the flexibility of the V4 architecture — for instance, a flash swap callback or misconfigured hook integration could inadvertently or maliciously trigger a donation, causing state divergence that is not recoverable.
Recommendation
Override
_beforeDonate
inEulerSwap
and revert unconditionally, matching the behavior of_beforeAddLiquidity
. This ensures that any attempt to donate tokens directly to the pool whileEulerSwap
is the active hook is rejected, preserving alignment between the hook logic and the Uniswap V4 core pool state.Euler: Fixed in PR 83.
Cantina: Verified, the
beforeDonate
hook now reverts. This is implemented by settingbeforeDonate
ingetHookPermissions()
totrue
, which would revert whenBaseHook.beforeDonate()
is called.Fee collection in FundsLib.depositAssets() reverts when the protocol fee recipient is the zero address
Context:
Description:
In
FundsLib.depositAssets()
, the protocol fee is calculated and sent top.protocolFeeRecipient
without checking if the recipient is the zero address:{uint256 protocolFeeAmount = feeAmount * p.protocolFee / 1e18;if (protocolFeeAmount != 0) {IERC20(asset).transfer(p.protocolFeeRecipient, protocolFeeAmount);amount -= protocolFeeAmount;feeAmount -= protocolFeeAmount;}}As such, if the protocol fee recipient is ever configured to be the zero address and
protocolFeeAmount != 0
, a transfer to the zero address is performed, which reverts for many tokens. An example would be any token inheriting OpenZeppelin'sERC20
, since it explicitly checks for this case.This would make it impossible to swap through the protocol as
FundsLib.depositAssets()
will always revert.Recommendation:
Consider only taking a fee when the protocol fee recipient is not the zero address:
- {+ if (p.protocolFeeRecipient != address(0)) {uint256 protocolFeeAmount = feeAmount * p.protocolFee / 1e18;if (protocolFeeAmount != 0) {IERC20(asset).transfer(p.protocolFeeRecipient, protocolFeeAmount);amount -= protocolFeeAmount;feeAmount -= protocolFeeAmount;}}Euler: Fixed in PR 76.
Cantina: Verified, the recommendation was implemented.
Checks in EulerSwap.activate() prevent deploying pools with a one-sided curve
Context:
Description:
EulerSwap.activate()
performs the following checks to ensure the pool's reserves are on the curve:require(CurveLib.verify(p, s.reserve0, s.reserve1), CurveLib.CurveViolation());require(!CurveLib.verify(p, s.reserve0 > 0 ? s.reserve0 - 1 : 0, s.reserve1), CurveLib.CurveViolation());require(!CurveLib.verify(p, s.reserve0, s.reserve1 > 0 ? s.reserve1 - 1 : 0), CurveLib.CurveViolation());However, these checks make it impossible to deploy a pool with either reserve as
0
, which is a valid input ifx0
ory0
is also0
(i.e. a one-sided curve). For example, ifx = 0
andx0 = 0
, the first two checks would simplify to:require(CurveLib.verify(p, s.reserve0, s.reserve1), CurveLib.CurveViolation());require(!CurveLib.verify(p, 0, s.reserve1), CurveLib.CurveViolation());Both checks are called with the same arguments as
reserve0 = 0
. However, assumingy
andy0
are valid values,CurveLib.verify(p, reserve0, reserve1)
would pass but!CurveLib.verify(p, 0, reserve1)
would fail.The same problem exists if
f()
is called directly to check if the current reserves are on the curve, sincex = 0
is not a valid input forf()
, but is technically on the curve sincex0 = 0
.Recommendation:
Consider modifying the checks to:
require(CurveLib.verify(p, s.reserve0, s.reserve1), CurveLib.CurveViolation());if (s.reserve0 != 0) require(!CurveLib.verify(p, s.reserve0 - 1, s.reserve1), CurveLib.CurveViolation());if (s.reserve1 != 0) require(!CurveLib.verify(p, s.reserve0, s.reserve1 - 1), CurveLib.CurveViolation());Calling
f()
directly is avoided asactivate()
would need to determine which curve (i.e.f(x)
orf(y)
) to use, which would be re-implementing the logic inverify()
.Euler: FIxed in PR 75.
Cantina: Verified, the recommendation was implemented.
Division by zero occurs for CurveLib.fInverse() due to c == 0 edge case
Context:
Description:
In
CurveLib.fInverse()
,x
is determined as follows whenB <= 0
:if (B <= 0) {// use the regular quadratic formula solution (-b + sqrt(b^2 - 4ac)) / 2ax = Math.mulDiv(absB + sqrt, 1e18, 2 * c, Math.Rounding.Ceil) + 1;} else {However, there is an edge case here when
c = 0
as a division-by-zero occurs, causingfInverse()
to revert. It occurs wheny = y0
andx0 = 0
, which causesB == 0
.This edge case is reachable if the pool is deployed with a one-sided curve (i.e.
x0 = 0
ory0 = 0
) andc
for the other curve is0
. For example, assume a pool is deployed with:x0 = 0
,y0 != 0
cx = 0
Consider a swap from
asset1
toasset0
where the input amount is specified inQuoteLib.findCurvePoint()
:// swap Y in and X outyNew = reserve1 + amount;if (yNew < y0) {// remain on g()xNew = CurveLib.f(yNew, py, px, y0, x0, cy);} else {// move to f()xNew = CurveLib.fInverse(yNew, px, py, x0, y0, cx);}If
yNew == y0
,fInverse()
is called withy == y0
,x0 = 0
andc = 0
, which fulfils all the listed conditions above for the edge case to occur.Recommendation:
There are two ways to handle this edge case:
-
In
QuoteLib.findCurvePoint()
, modify all the conditions that determine which curve to use tox <= x0
/y <= y0
(i.e. callCurveLib.f()
whenx == x0
/y == y0
). Alternatively, implement a short-circuit forx == x0
/y == y0
which returns(x0, y0)
. -
Handle the
c == 0
case inCurveLib.fInverse()
by adding this branch to the top of the function:
if (c == 0) {return Math.mulDiv(x0 * x0, px, x0 * px + py * (y - y0), Math.Rounding.Ceil);}This is equal to:
Which is obtained by substituting into equation 20 in the whitepaper and simplifying.
Euler: Fixed in PR 86.
Cantina: Verified,
QuoteLib.findCurvePoint()
now callsCurveLib.f()
whenx == x0
/y == y0
.Calculation of term1 in CurveLib.fInverse() could overflow int256.max with extreme prices
Severity
- Severity: Low
Submitted by
MiloTruck
Context:
Description:
CurveLib.fInverse()
performs the following calculation when calculatingB
:int256 term1 = int256(Math.mulDiv(py * 1e18, y - y0, px, Math.Rounding.Ceil)); // scale: 1e36However, it is possible for
term1
to exceeduint256.max
(andint256.max
by extension) within the given bounds whenpx
is extremely small andpy
is extremely large.For example:
y = uint112.max
px = 1e10
,py = 1e36
y0 = 0
The price in this example would be
py / px = 1e26
, which extreme, but not entirely unreachable. For reference, WBTC / PEPE has a price of1e21
(assuming WBTC = 100,000 USD and PEPE = 0.000001 USD).Additionally, the following inputs result in
int256.max < term1 <= uint256.max
, causing theint256
cast to overflow:y: 4601578453167855735395833887649913px: 23200824491py: 758264634474574188751969252527909293y0: 1302464407997906980507415513033026As such, if a pool is deployed with an extreme price, functions involving quotes could unexpectedly revert since they call
CurveLib.fInverse()
.Recommendation:
A suitable upper bound for
px
andpy
can be found by solving the following inequality:(py * 1e18) * (y - y0) / px <= int256.maxAssuming the worst-case of
y - y0 = uint112.max
, the inequality simplifies to:py / px <= int256.max / uint112.max / 1e18Which is roughly equal to
1.1e25
. As such, consider restricting the upper bound ofpx
andpy
to1e25
to ensurepy / px
(orpx / py
) does not exceed1.1e25
.While this reduces the maximum price which can be configured, it is still a reasonable price range. Realistically, the largest possible price range is GUSD / PEPE, which has a price of:
price of 1 wei of GUSD / price of 1 wei of PEPE = (1 / 1e2) / (0.000001 / 1e18) = 1e22Euler: Fixed in PR 85.
Cantina: Verified,
px
andpy
are now checked to not be greater than1e25
inEulerSwap.activate()
.
Informational6 findings
Missing Internal Function Naming Convention
State
- Acknowledged
Severity
- Severity: Informational
Submitted by
Cryptara
Description
The
EulerSwapFactory
contract and other related contracts (such asEulerSwap
,EulerSwapPeriphery
, andMetaProxyDeployer
) do not consistently follow a naming convention to clearly distinguish between public/external and internal/private functions. Specifically, internal helper functions likeuninstall
,swapAndPop
, and similarly scoped utility functions are defined without a leading underscore (_
), which makes it difficult to differentiate at a glance between callable external APIs and internal logic. This is especially problematic in contracts with complex state transitions or logic branching, where traceability of function calls plays a critical role in auditing and maintaining correctness.This inconsistent naming can cause confusion during review and debugging phases, and it opens the possibility for accidental misuse or misidentification of function purpose. It also breaks with widely adopted Solidity conventions, which favor using a prefixed underscore (
_
) for non-external functions that should not be exposed or used directly by consumers of the contract.Recommendation
It is recommended to refactor all internal and private functions across
EulerSwapFactory
and associated contracts (EulerSwap
,FundsLib
,MetaProxyDeployer
, etc.) to include a leading underscore (_
) in their names. For example,swapAndPop
should be renamed to_swapAndPop
, and similarly foruninstall
and other auxiliary logic handlers. This will improve code readability, clarify intent, and align the codebase with Solidity best practices.If the team prefers to maintain the current naming scheme for legacy or readability reasons, an acceptable alternative is to clearly document the visibility and intended usage of such functions with NatSpec annotations and visibility modifiers, though this is still considered a weaker practice than using naming convention alone.
Euler: Acknowledged.
Cantina: Acknowledged.
Incompatibility with uniswapV2Call Interface Expectations
Description
The
EulerSwap
contract is designed to behave similarly to a Uniswap V2 pair, but it does not implement all interface expectations of the UniswapV2 ecosystem. Specifically, it omits thetoken0()
andtoken1()
public getters, which are critical for integration with external contracts using theuniswapV2Call()
flash swap pattern.As described in the Uniswap V2 documentation, integrators typically write callback handlers under the assumption that:
function uniswapV2Call(address sender, uint amount0, uint amount1, bytes calldata data) external {address token0 = IUniswapV2Pair(msg.sender).token0();address token1 = IUniswapV2Pair(msg.sender).token1();assert(msg.sender == IUniswapV2Factory(factoryV2).getPair(token0, token1));...}This pattern assumes that:
- The pair contract exposes
token0()
andtoken1()
as public view functions. - The caller (
msg.sender
) is a contract created by a knownUniswapV2Factory
, registered viagetPair(token0, token1)
.
The issue is that
EulerSwap
pairs are not deployed or tracked by a canonical UniswapV2Factory instance. Therefore, any standard check like:assert(msg.sender == IUniswapV2Factory(factoryV2).getPair(token0, token1));will fail, unless integrators explicitly whitelist
EulerSwap
addresses or bypass the factory check entirely. Even thoughEulerSwapFactory
exists, it is not designed to conform toUniswapV2Factory
's interface, and integrators will continue targeting Uniswap's version.Moreover, without
token0()
andtoken1()
getters, the very first step inuniswapV2Call
reverts, asmsg.sender
does not expose these functions. This breaks compatibility with flash swap routers, arbitrage bots, and legacy DeFi integrations that assume drop-in V2 pair behavior.Recommendation
If compatibility with existing
uniswapV2Call
-based contracts is a goal (e.g., allowing integrators to plug EulerSwap into existing arbitrage or flash swap flows), the following steps should be taken:-
Add
token0()
andtoken1()
public view functions to theEulerSwap
pair contracts. These should return the respective token addresses in fixed canonical order. This change alone will enable basic integration and unlock compatibility with a broad range ofuniswapV2Call
consumers. -
Document factory expectations clearly. While
EulerSwapFactory
is not meant to mimicUniswapV2Factory
, it's important to communicate to integrators that the factory validation step (i.e., usinggetPair(token0, token1)
) must be either removed or replaced by a manual whitelisting process. -
Consider renaming the callback (e.g.,
eulerSwapCall()
) or explicitly documenting that while the mechanism is similar to UniswapV2, it is not drop-in compatible unless these caveats are handled.
If compatibility is not a design goal, the documentation should strongly emphasize that integrators must not assume this contract adheres to the V2 pair interface, and should instead follow custom integration guidelines.
Euler: Fixed in PR 80.
Cantina: Verified, the callback has been renamed to
IEulerSwapCallee.eulerSwapCall()
.- The pair contract exposes
maxWithdraw in QuoteLib.calcLimits() double-counts deposited assets
Context:
Description:
In
QuoteLib.calcLimits()
,outLimit
is restricted by the remaining cash and borrow caps in the vault as such:(, uint16 borrowCap) = vault.caps();uint256 maxWithdraw = decodeCap(uint256(borrowCap));maxWithdraw = vault.totalBorrows() > maxWithdraw ? 0 : maxWithdraw - vault.totalBorrows();if (maxWithdraw > cash) maxWithdraw = cash;maxWithdraw += vault.convertToAssets(vault.balanceOf(eulerAccount));if (maxWithdraw < outLimit) outLimit = maxWithdraw;However, the following line:
if (maxWithdraw > cash) maxWithdraw = cash;should be removed for two reasons:
- Assets deposited by
eulerAccount
are part ofcash
, somaxWithdraw
is double-counting deposited assets. - The case where
maxWithdraw > cash
is implicitly covered by thecash < outLimit
check above.
For example, assume the following numbers:
cash = 100
borrowCap - totalBorrows = 120
depositedAssets = vault.convertToAssets(vault.balanceOf(eulerAccount)) = 20
For point (1),
maxWithdraw
is calculated as:min(borrowCap - totalBorrows, cash) + depositedAssets = min(120, 100) + 20 = 120120
is clearly wrong as the vault only has a total of100
assets that can be withdrawn/borrowed.Due to point (2), the inflated
maxWithdraw
does not cause any issues asoutLimit = cash = 100
from thecash < outLimit
check above. In general,borrowCap - totalBorrows > cash
impliesmaxWithdraw > cash
, so it will always be covered by thecash < outLimit
check.Recommendation:
The
maxWithdraw > cash
check preventsmaxWithdraw + depositedAssets
from overflowing, sincemaxWithdraw
could be up touint256.max
.Consider short-circuiting if the intermediate
maxWithdraw
is greater thanoutLimit
:- if (maxWithdraw > cash) maxWithdraw = cash;- maxWithdraw += vault.convertToAssets(vault.balanceOf(eulerAccount));- if (maxWithdraw < outLimit) outLimit = maxWithdraw;+ if (maxWithdraw < outLimit) {+ maxWithdraw += vault.convertToAssets(vault.balanceOf(eulerAccount));+ if (maxWithdraw < outLimit) outLimit = maxWithdraw;+ }This implementation should save some gas since further calculations are skipped when
maxWithdraw > outLimit
can be determined early.Euler: Fixed in PR 81.
Cantina: Verified, the recommendation has been implemented.
- Assets deposited by
Minor improvements to code and comments
Context
See below.
Description/Recommendation:
- CurveLib.sol#L71-L72, CurveLib.sol#L79-L80 - Consider using OZ's
sqrt()
with rounding to round up. For example:
- sqrt = Math.sqrt(discriminant); // drop back to 1e18 scale- sqrt = (sqrt * sqrt < discriminant) ? sqrt + 1 : sqrt;+ sqrt = Math.sqrt(discriminant, Math.Rounding.Ceil);- CurveLib.sol#L38, CurveLib.sol#L90 - Consider using
Math.ceilDiv()
for rounding up instead, it improves readability and avoids the case wheren + (n - 1)
overflowsuint256.max
.
- return y0 + (v + (py - 1)) / py;+ return y0 + Math.ceilDiv(v, py);- x = (2 * C + (absB + sqrt - 1)) / (absB + sqrt) + 1;+ x = Math.ceilDiv(2 * C, absB + sqrt) + 1;-
CurveLib.sol#L71 - Typo, double comment slashes.
-
CurveLib.sol#L58 - Unncessary brackets can be removed:
- C = Math.mulDiv((1e18 - c), x0 * x0, 1e18, Math.Rounding.Ceil); // scale: 1e36+ C = Math.mulDiv(1e18 - c, x0 * x0, 1e18, Math.Rounding.Ceil); // scale: 1e36- UniswapHook.sol#L82-L88 - The code here can be simplified:
if (isExactInput) {amountIn = uint256(-params.amountSpecified);- amountOut = QuoteLib.computeQuote(evc, p, params.zeroForOne, uint256(-params.amountSpecified), true);+ amountOut = QuoteLib.computeQuote(evc, p, params.zeroForOne, amountIn, true);} else {- amountIn = QuoteLib.computeQuote(evc, p, params.zeroForOne, uint256(params.amountSpecified), false);amountOut = uint256(params.amountSpecified);+ amountIn = QuoteLib.computeQuote(evc, p, params.zeroForOne, amountOut, false);}-
UniswapHook.sol#L119 - This check is not needed as
CurveLib.verify()
has the same check. Consider removing it. -
EulerSwapFactory.sol#L63-L67 -
params.eulerAccount
does not need to be included in the salt as it in the creation code as part ofparams
. Therefore, a differenteulerAccount
would result in a different pool address. -
EulerSwapFactory.sol#L94 -
poolParams.eulerAccount
doesn't need to be cast to an address as it is already one:
- keccak256(abi.encode(address(poolParams.eulerAccount), salt)),+ keccak256(abi.encode(poolParams.eulerAccount, salt)),- CtxLib.sol#L24-L40 - The code here can be simplified to avoid using assembly:
return abi.decode(msg.data[msg.data.length - 384:], (IEulerSwap.Params));- EulerSwap.sol#L61 -
activate()
should be declared external.
Euler: Fixed in the following PRs:
Cantina: Verified.
- CurveLib.sol#L71-L72, CurveLib.sol#L79-L80 - Consider using OZ's
UniswapHook._beforeInitialize() is never reached during normal initialization
Context:
Description:
UniswapHook._beforeInitialize()
is overridden to check that_poolKey.tickSpacing
has not been set. This is meant to ensure the pool can only be initialized throughUniswapHook.activateHook()
:function _beforeInitialize(address, PoolKey calldata, uint160) internal view override returns (bytes4) {// when the hook is deployed for the first time, the internal _poolKey is empty// upon activation, the internal _poolKey is initialized and set// once the hook contract is activated, do not allow subsequent initializationsrequire(_poolKey.tickSpacing == 0, AlreadyInitialized());return BaseHook.beforeInitialize.selector;}However, Uniswap V4 calls hooks with the
noSelfCall
modifier, which skips calling the hook if the caller is the hook address itself:/// @notice modifier to prevent calling a hook if they initiated the actionmodifier noSelfCall(IHooks self) {if (msg.sender != address(self)) {_;}}Therefore, when
poolManager.initialize()
is called inactivateHook()
, thebeforeInitialize
hook is actually never called and_beforeInitialize()
is never reached. In fact, checking_poolKey.tickSpacing == 0
is incorrect as_poolKey
is set before callingpoolManager.initialize()
inactivateHook()
.This behavior can be observed by replacing the require statement with
revert AlreadyInitialized()
and running the tests; they will all still pass.Recommendation:
Consider removing
_beforeInitialize()
entirely. Initializations outside ofactivateHook()
would revert withHookNotImplemented
inBaseHook
, while normal initialization works as intended._beforeAddLiquidity()
can also be removed, since the implementation inBaseHook
also reverts.Alternatively, remove the check and simply revert with
AlreadyInitialized
.Euler: Fixed in PR 83.
Cantina: Verified,
_beforeInitialize()
and_beforeAddLiquidity()
have been removed.Additional fuzz tests for CurveLib
Context:
Description:
The following tests may be helpful for future changes or fixes:
- Fuzz tests for
f()
andfInverse()
to check for reverts/overflows. - Differential tests for
f()
andfInverse()
against the equations from the whitepaper translated into python.
Some things to note for (2):
- The tests require enabling ffi in your Foundry config.
- The tests skip any inputs where
f()
/fInverse()
revert, or the case where castingterm1
toint256
overflows infInverse()
. y
is bounded to[y0, uint112.max]
forfInverse()
although it isn't listed in the pre-conditions.
Euler
Improved NatSpec for [y0, uint112.max] for fInverse() in PR 85 and PR 87.
Cantina
Verified.
- Fuzz tests for