1inch Aqua and SwapVM MVP v1.0 Audit

Summary

Type: DeFi
Timeline: 2026-01-12 → 2026-02-02
Languages: Solidity

Findings
Total issues: 53 (33 resolved)
Critical: 3 (3 resolved) · High: 2 (2 resolved) · Medium: 12 (6 resolved) · Low: 16 (6 resolved)

Notes & Additional Information
20 notes raised (16 resolved)

Scope

OpenZeppelin performed an audit of the following repositories:

• The 1inch/swap-vm repository at commit 60172b
• The 1inch/aqua repository at commit 89cedfa
• The 1inch/solidity-utils repository at commit 38e5f8d.
• During fix review, we conducted audit of the new refactored XYCConcentrate instruction at commit 9b9d821.
• Additionally commits af53fc3 on Aqua and f6631a0 on SwapVM were reviewed implementing the Rescuable contract introduced in solidity-utils at commit 81ad207 making them the final commits reviewed for Aqua and solidity-utils during the engagement.
• The final SwapVM commit reviewed for the engagement was b2daef8. The final reviewed SwapVM state includes a maker-favoring rounding adjustment.

In scope were the following files:

1inch/swap-vm
└── src
    ├── instructions
    │   ├── Balances.sol
    │   ├── Controls.sol
    │   ├── Decay.sol
    │   ├── Extruction.sol
    │   ├── Fee.sol
    │   ├── PeggedSwap.sol
    │   ├── XYCConcentrate.sol
    │   └── XYCSwap.sol
    ├── libs
    │   ├── MakerTraits.sol
    │   ├── PeggedSwapMath.sol
    │   ├── Power.sol
    │   ├── TakerTraits.sol
    │   └── VM.sol
    ├── opcodes
    │   └── AquaOpcodes.sol
    ├── routers
    │   └── AquaSwapVMRouter.sol
    └── SwapVM.sol

1inch/aqua
└── src
    ├── Aqua.sol
    ├── AquaApp.sol
    ├── AquaRouter.sol
    ├── interfaces
    │   └── IAqua.sol
    └── libs
        └── Balance.sol

1inch/solidity-utils
└── contracts
    ├── libraries
    │   ├── Calldata.sol
    │   ├── CalldataPtr.sol
    │   ├── Transient.sol
    │   └── TransientLock.sol
    └── mixins
        ├── Multicall.sol
        └── Simulator.sol

System Overview

Aqua

Aqua is a decentralized shared liquidity layer designed by 1inch to address capital inefficiencies within the DeFi ecosystem. Traditional AMMs (Automated Market Makers) often require liquidity providers to "lock" assets into specific pools, which leads to fragmented and underutilized capital. Aqua introduces a model where liquidity remains in the user wallet while being available for multiple applications to utilize according to market demand.

The protocol functions as an accounting and fund-management engine that sits between liquidity providers and trading applications. Liquidity providers, referred to as makers, grant token approvals to the Aqua contract instead of depositing them into a traditional pool. This allows makers to maintain self-custody of their assets while the protocol manages the permissions and balances required for automated execution.

Makers define their participation through a mechanism called "shipping a strategy". When a maker ships a strategy, they provide a custom calldata that defines the rules of the engagement and the specific tokens involved. This strategy is identified by a unique hash and becomes immutable once active. This design ensures that the rules governing the liquidity cannot be changed unexpectedly by any party during the lifecycle of the strategy.

Applications, or AquaApps, are the external contracts that implement the actual trading logic, such as constant product formulas or limit order books. These applications interact with the core Aqua contract to move funds. When a trade is executed, the application pulls the necessary tokens from the maker and subsequently verifies that the taker has pushed the required counterparty tokens back into the maker's balance.

The core Aqua contract is responsible for updating internal balances and emitting events that reflect the movement of assets. It does not enforce specific pricing logic but acts as a secure conduit for transfers. By separating the liquidity management from the application logic, the protocol allows for a highly flexible environment where the same capital can support various trading strategies simultaneously.

Swap VM

SwapVM is a programmable execution engine that provides a highly customizable framework for executing swaps. It functions as a virtual machine where the logic of a trade is not hardcoded but defined by a series of instructions. This flexibility allows makers to create sophisticated order types that can include custom fee structures, price calculations, and specific execution constraints.

At the heart of the system is the concept of a program, which is a piece of bytecode containing chained instructions. Each instruction corresponds to an opcode implemented by the application contract, such as a constant product formula, a decay mechanism, or a fee mechanism. When a trade occurs, the VM loops through these instructions to compute the final input and output amounts required for the swap. An opcode is the index of the internal function selector in the instructions array. Thus, SwapVM provides the ability for makers to create dynamic programs that can loop through the predefined set of internal functions. However, unlike multicall, these instructions have the ability to jump to the next instructions in-between, manipulate program bytes, and affect the outcome of the swap for the maker.

At its core, the VM maintains four internal registers (balanceIn, balanceOut, amountIn, and amountOut) in the SwapRegisters struct to track token balances and swap amounts throughout the execution loop, along with a program counter (nextPC). Each instruction can update these registers or the program counter based on its functionality. At the end of the loop, the resulting amountIn is fulfilled by the taker in tokenIn, and amountOut is fulfilled by the maker in tokenOut.

A maker — considered as a liquidity provider — initiates the process by constructing an order that includes their specific program and operational traits. These traits, managed by the MakerTraits library, define whether the order uses Aqua for liquidity or relies on off-chain EIP-712 signatures for validation. This dual approach allows the SwapVM to operate both as a standalone settlement layer and as a specialized application on top of the Aqua protocol.

A taker initiates the swap using the exact order data signed or shipped (in Aqua) by the maker, including the required tokenIn, tokenOut, and either amountIn or amountOut. The taker must supply the correct program and operational traits specified by the maker in order to generate a valid orderHash to proceed. The TakerTraits contract provides slippage protection to the taker as per the thresholdAmount value set during the swap.

The execution flow within SwapVM is strictly governed by the sequence of opcodes chosen by the maker. Once the program completes and the final amounts are determined, the contract handles the transfer of assets between the maker and the taker to finalize the trade. The quote function simulates the swap in a static context, allowing callers to verify both the correctness of the program implementation and the expected swap outcome, while the swap function performs the actual swap execution. During execution in the static context, instructions are restricted from modifying contract state, and no instruction updates its associated storage. An ideal program is expected to ensure that both quote and swap result in the same output and quote/swap consistency is maintained.

SwapVM introduces hooks that allow makers and takers to manage liquidity dynamically during the execution cycle. Through the preTransferIn, postTransferIn, preTransferOut, and postTransferOut functions, users can perform targeted actions before and after asset transfers.

Security Model and Trust Assumptions

Throughout the in-scope codebase, the following trust assumptions and privileged roles were identified:

• Asset Compatibility: The protocol assumes that fee-on-transfer tokens and rebasing tokens will not be used. Such assets can lead to accounting discrepancies in both SwapVM and Aqua, because the contract expects the amount sent to be exactly what is received.
• Strategy Uniqueness: Makers must use a unique salt for every strategy when integrating SwapVM with Aqua. If the same program is used for different token pairs without a unique salt, it generates identical order hashes that could enable takers to swap unauthorized tokens.
• Application Integrity: All applications developed on top of Aqua and SwapVM are assumed to be securely implemented and audited. Since these applications have the power to trigger fund movements, their security is vital to the safety of the core protocol.
• User Awareness: Protocol participants are assumed to understand the risks associated with the specific applications they choose to use. Users must perform their own security assessments of any third-party logic before committing liquidity or executing trades.
  • Maker: The maker is assumed to be a sophisticated user with a strong understanding of the application's instruction set and the ability to construct valid programs correctly. Errors in program construction can lead to unintended or unsafe behavior.
  • Taker: The taker is expected to thoroughly review and verify the maker's program before executing a swap. A malicious maker may construct a program that results in loss of funds, and it is the taker's responsibility to detect and avoid such programs.
• Fees: The maker is assumed to be aware that flatFees accrued are automatically reinvested in the strategy and included in the internal accounting of balances.

Privileged Roles

Aqua

• Maker: The maker is the liquidity provider who owns the underlying assets and creates strategies. This role has the exclusive authority to authorize an application to use their tokens through the ship function. Additionally, the maker retains the power to terminate a strategy and revoke token access at any time by calling the dock function.
• AquaApp: Applications are smart contracts authorized by makers to execute trades on their behalf. An application has the privileged capability to pull tokens from a maker wallet as long as a valid strategy hash is provided. These contracts are trusted to implement correct pricing logic and to verify that the maker receives the appropriate tokens from the taker.
• Taker: The taker is the counterparty in a trade who interacts with an AquaApp to swap assets. While not a privileged role in terms of protocol management, the taker is responsible for pushing the required funds to the maker to satisfy the conditions of the strategy. The protocol relies on the application logic to ensure this push occurs before the transaction completes.

SwapVM

• Maker: The liquidity provider who creates and authorizes trading orders. The maker is responsible for defining the bytecode program that governs the swap logic and pricing. They also choose the security model for their orders, either by shipping a strategy in Aqua or by providing cryptographic signatures for direct swaps.
• App/Router: The smart contract that inherits from SwapVM and implements the available opcodes. This contract acts as the entry point for takers and defines the set of instructions that makers can use in their programs. It is trusted to correctly map instruction IDs to the intended logic and to manage the execution environment.
• Taker: The participant who executes an order created by a maker. The taker provides the necessary traits and data to satisfy the order requirements, such as minimum threshold amounts (slippage) or deadlines. They interact with the App contract to trigger the swap and are responsible for supplying the input tokens required by the maker program.

Critical Severity

Precision Loss in solve() Quadratic Solver Breaks Invariant Protection

The PeggedSwap instruction implements a square-root linear curve for pegged asset swaps using the following invariant formula:

√(x/X₀) + √(y/Y₀) + A · (x/X₀ + y/Y₀) = C

where x and y are the pool's current token reserves, X₀ and Y₀ are the normalization factors, A (referred to as linearWidth in code) controls how linear versus curved the pricing behaves, and C is the invariant constant representing the pool's total value. In a correctly functioning swap, C should remain constant or slightly increase due to rounding that favors the maker. When C decreases, the maker has paid out more tokens than the curve dictates — a direct financial loss.

The protocol's documentation in PeggedSwap.sol provides parameter guidance for different use cases:

Parameters guide:
  - For stablecoins (USDC/USDT): A ≈ 0.8-1.5
  - For wrapped tokens (WETH/stETH): A ≈ 0.3-0.6
  - For volatile pairs: A ≈ 0.0-0.2

The suggested range for volatile pairs (A ≈ 0.0-0.2) includes values near zero, which overlaps with the numerically unstable region described below.

The PeggedSwapMath.solve() function rearranges the invariant equation to find v (normalized output reserve) given u (normalized input reserve). It calculates the right side as R = C - √u - A · u, then solves the quadratic equation A · w² + w - R = 0 using the standard formula w = (-1 + √(1 + 4AR)) / (2A), and finally computes v = w². The implementation computes the numerator as sqrtDiscriminant - ONE (i.e., √D - 1) and divides by 2 * a. When A is extremely small, this formula suffers from catastrophic cancellation — a well-known numerical analysis problem where subtracting two nearly equal numbers destroys significant digits. Specifically, when A is tiny the discriminant D = 1 + 4AR/ONE ≈ 1, so √D ≈ 1 and the numerator √D - 1 ≈ 0, producing an incorrect v value regardless of the small denominator.

Consider a balanced pool with 1,000 tokens of each asset:

With a healthy linearWidth, the maker gives 98 tokens and the invariant increases by 88,518 units, confirming the rounding protects them. With linearWidth = 2, the precision errors cause the maker to give out ~101.5 tokens while the invariant decreases by 3.56% — this loss compounds with every subsequent swap against the pool.

In the above example, the impact demonstrated would only affect a small portion of the pool, providing a small profit to the user. However, besides this exploitation being scalable through a looping swap, during fuzzing tests it was found that it is possible to easily drain more than 15% of the pool in a single swap, depending on the value of A and by manipulating the amount of amountIn entered.

Consider replacing the unstable quadratic formula with its mathematically equivalent stable form in PeggedSwapMath.solve():

- // w = (-1 + √D) / (2a)   — unstable when a is small
- uint256 numerator = sqrtDiscriminant - ONE;
- uint256 denominator = 2 * a;
- uint256 w = (numerator * ONE) / denominator;
+ // w = 2·rightSide / (1 + √D)   — stable for all a values
+ uint256 denominator = ONE + sqrtDiscriminant;
+ uint256 w = (2 * rightSide * ONE) / denominator;

Both formulas produce identical results mathematically, but the second form avoids subtracting two numbers that become nearly equal when A is small — mitigating the root cause of the precision loss.

Update: Resolved in pull request #67.

Shared Liquidity Tracking in Multi-Token XYCConcentrate Enables Cyclic Arbitrage

The XYCConcentrate instruction implements concentrated liquidity, allowing pools to amplify effective token balances within a price range. The mechanism works as follows:

• Virtual balances are computed via concentratedBalance(): balance + (delta * currentLiquidity) / initialLiquidity
• The currentLiquidity value is stored per order in liquidity[orderHash]
• After each swap, _updateScales() recalculates liquidity as sqrt(balanceIn * balanceOut) using only the two tokens involved in that swap

When a pool contains three tokens (A, B, C) under the same orderHash, all token pairs read from and write to the same liquidity[orderHash] slot. The problem is that each pair's liquidity should be independent — the mathematical relationship between A and B has no bearing on the relationship between B and C.

However, when an A→B swap occurs, it overwrites the shared liquidity with a value derived solely from A and B balances. When a subsequent B→C swap reads this value to compute virtual balances for B and C, it uses a fundamentally incompatible number, causing the swap to execute at a distorted rate. An attacker exploits this by executing a circular trade through all three pairs:

1. Swap A→B, which sets liquidity[orderHash] based on A-B balances
2. Swap B→C using the A-B derived liquidity to compute B-C virtual balances, resulting in a favorable rate
3. Swap C→A using the now B-C derived liquidity, again at a distorted rate
4. The cycle completes with more tokens than started

This PoC showcases asset loss when cycling assets.

Consider implementing per-pair liquidity tracking using a nested mapping like liquidity[orderHash][pairId].

Update: Resolved in pull request #82. The support for multiple token strategies was dropped and the XYCConcentrate instruction was updated to a stateless model designed to operate strictly within a two-token model for the current release.

Invariant Violation and Fund Drainage Due to Axis Mismatch in PeggedSwap

The _peggedSwapGrowPriceRange2D function calculates the swap invariant using local variables x0 and y0, which correspond to balanceIn and balanceOut. However, the normalization logic incorrectly divides these dynamic values by the static configuration parameters config.x0 and config.y0. This mismatch occurs because the configuration parameters are statically mapped to the token sort order, while the local variables depend on the swap direction.

When a swap occurs in the reverse direction (where tokenIn > tokenOut), the local x0 represents the balance of the greater token, but it is normalized by the capacity of the lower token (config.x0). This effectively swaps the axes of the bonding curve. If the pool has asymmetric capacities, the contract calculates a wildly incorrect invariant, leading to a broken pricing model where the pool offers exchange rates that do not reflect the actual market value.

Assume a pool with Token A (Lower Address) and Token B (Greater Address) that is asymmetric. For simplicity, assume linearWidth (A) = 0:

• Capacity A: 1,000,000 (Abundant asset)
• Capacity B: 1,000 (Scarce asset)
• Current State: Balanced relative to capacity (Balance A = 1M, Balance B = 1k).
• True Invariant: sqrt(1) + sqrt(1) = 2

The Attack (Reverse Swap: B → A):

1. User swaps Token B (tokenIn).
2. The contract sets local x0 = Balance B (1,000).
3. The contract incorrectly calculates the normalized input using config.x0 (Capacity A): u = 1,000 / 1,000,000 = 0.001
4. The contract calculates the target invariant based on this cross-wired math: K = sqrt(0.001) + sqrt(1000) ≈ 31.65

The contract now believes that the invariant is 31.65 (far higher than the real 2.0). To satisfy this inflated invariant during the swap solving process, the contract will output a massive amount of Token A for a small amount of Token B, effectively draining the pool reserves. This PoC showcases the fund drainage when the swap is reversed.

Consider dynamically assigning the normalization factors based on the swap direction. If tokenIn > tokenOut, the calculation should use config.y0 to normalize the input balance and config.x0 to normalize the output balance.

Update: Resolved at PR #68. The fix introduces dynamic assignment of normalization factors based on the swap direction, using the parseRatesAndBalances function, effectively mitigating the issue.

High Severity

XYCConcentrate Records Incorrect Balances When Protocol Fees Are Extracted

XYCConcentrate tracks pool liquidity to scale virtual reserves for concentrated liquidity AMMs. After each swap, _updateScales records the new liquidity based on amountIn and amountOut — the tokens that entered and left the pool. Protocol fee instructions like _aquaProtocolFeeAmountOutXD temporarily inflate the swap amount, execute the swap, then restore the original amount before extracting the fee. When XYCConcentrate wraps a protocol fee instruction, _updateScales runs after restoration — recording the original amount instead of the actual tokens transferred.

The AquaAMM strategy places XYCConcentrate before _aquaProtocolFeeAmountOutXD, creating this execution flow:

XYCConcentrate
     │
     ├── runLoop() ───► ProtocolFeeOut
     │                     ├── inflates amountOut (100 → 111)
     │                     ├── runLoop() ───► Swap accounts 111 tokens
     │                     ├── restores amountOut (111 → 100)
     │                     └── transfers 11 tokens as fee
     │
     └── _updateScales() sees amountOut = 100, not 111
 |
 └──  _transferOut() transfers 100 tokens - total tokens transferred = 100 + 11 = 111

The pool records that it sent 100 tokens whereas it actually sent 111. Each swap with protocol fees widens this gap. Over time, recorded liquidity diverges from actual reserves, causing the pool to promise swaps it cannot fulfill.

Consider placing protocol fee instructions before XYCConcentrate so that _updateScales executes while amounts still reflect actual transfers. Alternatively, consider capturing the transferred amounts before restoration.

Update: Resolved in pull request #75 and pull request #82. The team stated:

We implemented a stateless _xycConcentrateGrowLiquidity2D instruction and defined the correct instruction ordering in the README, including: protocol fees, concentrate, flat. This makes the intended execution flow explicit and ensures the instruction is used within the expected setup. The maker must place the protocol fees instruction first in the strategy program.

Dynamic Balance Tracking Does Not Account for Instantly Transferred Protocol Fees

The _dynamicBalancesXD instruction maintains a persistent balance mapping to track token amounts across multiple swaps for a given order. After executing nested instructions via ctx.runLoop(), it updates the stored balances by adding swapAmountIn to tokenIn balance and subtracting swapAmountOut from tokenOut balance. These balances are subsequently used in AMM curve calculations to determine swap prices.

When _dynamicBalancesXD wraps protocol fee instructions such as _dynamicProtocolFeeAmountInXD, the balance tracking becomes incorrect. The _feeAmountIn function temporarily adjusts ctx.swap.amountIn for the nested swap calculation, but then restores it to the original value (ExactIn) or increases it by the fee amount (ExactOut). The fee is then transferred directly from the maker to the fee recipient. When control returns to _dynamicBalancesXD, the returned swapAmountIn reflects this restored/increased value, which gets recorded in the balance mapping.

However, the maker no longer possesses the fee portion as it was already transferred to the fee recipient. For example, if a user swaps with amountIn = 1000 tokens and a 1% protocol fee applies, 10 tokens are transferred to the fee recipient while 990 tokens go to the pool. The balance mapping incorrectly records +1000 instead of +990, overstating the pool's actual balance by the fee amount. This error accumulates with each fee-bearing swap, leading to progressively incorrect price calculations.

Consider either returning the fee amounts from protocol fee instructions so _dynamicBalancesXD can adjust the balance updates accordingly, or documenting that _dynamicBalancesXD should not be used in combination with instantly-transferring protocol fee instructions.

Update: Resolved in pull request #75. The team stated:

We defined a strict instruction ordering in the README, requiring the protocol fees instructions to be placed first in the maker's strategy program. With this ordering, all subsequent instructions receive the correct amount, which ensures that the strategy accounting remains correct. In addition, we introduced an extra register, amountNetPulled, in the SwapRegisters struct of SwapVM, which allows the VM to correctly account for the amount of protocol fees pulled out during execution.

Medium Severity

Strategy Reinitialization via ship Allows Breaking Token Limits and Docking Invariants

The ship function has been designed to initialize a strategy once, but the current implementation allows the function to be executed multiple times for the same strategy hash as long as the token sets are disjoint. The validation logic only checks if the individual token balance has a token count of zero, failing to verify if the strategy hash itself has already been registered. This oversight allows a maker to append new tokens to an existing strategy by calling ship repeatedly with different token arrays. Consequently, a user can bypass the immutability requirement and circumvent the maximum limit of 254 tokens by adding them in separate batches, as the length check only applies to the array in the current transaction.

This flaw also undermines the consistency of the docking mechanism. The protocol assumes that a strategy is either fully active or fully docked. However, if a strategy is constructed via multiple ship calls with different tokens, the maker can subsequently only dock a subset of those tokens. This leaves the strategy in a mixed state where some tokens are marked as docked while others remain active and tradable. This violates the invariant that docking a strategy should universally deactivate it.

Consider implementing a state variable or mapping to track whether a strategy hash has already been initialized for a given maker and app. By checking this flag at the beginning of the ship function, the protocol can ensure that a strategy hash is used exactly once.

Update: Acknowledged, not resolved. The team clarified that the contract enforces immutability on a per-token basis (once a token is shipped for a given (maker, app, strategyHash), shipping it again reverts), that calling ship() multiple times with disjoint token sets is acceptable by design, and that Aqua App Builders should encode the token set into the strategy calldata so strategyHash = keccak256(strategy) becomes unique. Takers remain protected by profit incentive and can validate flows via quote(). Additional persistent state was intentionally avoided to keep gas costs low.

Unrestricted Aqua.push Allows Direct Balance Manipulation

Aqua.push updates a maker's balance and transfers tokens as part of the swap flow. Unlike Aqua.pull, it is intentionally callable by external accounts to support callback-based integrations. Since Aqua.push is externally callable and accepts a maker parameter, a maker (or any account they control) can invoke it directly with their own address. This updates internal balances and triggers transfers without a validated app context. A check like msg.sender != maker would be insufficient, as a different account controlled by the maker could still perform the call. Moreover, a self-transfer results in no net balance change for the maker, allowing them to manipulate the flow without actual fund movement.

Consider allowing apps to flag when push is permitted, ensuring that balances can only be modified through the respective app. This provides flexibility for apps to either allow donations by implementing a dedicated function or restrict balance changes to specific actions (e.g., swap, stake).

Update: Acknowledged, not resolved. The team stated that Aqua.push is intentionally externally callable to support callback-based integrations. Maker-side self-calls only create self-inflicted arbitrage/loss, third-party calls act as donations to the maker, and neither allows extraction of funds from other participants. The team considers the external callability a deliberate tradeoff and not a vulnerability.

Missing Validation for begin < end in Calldata.slice Allows Underflow

The Calldata.slice() function is used throughout the protocol to extract data slices based on user-controlled index values, notably in MakerTraitsLib._getDataSlice() and TakerTraitsLib._getDataSlice(). These slices are converted into CalldataPtr pointers via CalldataPtrLib.from() for use by the SwapVM during execution.

The function validates that end <= calls.length but does not verify that begin <= end. When begin > end, the operation sub(end, begin) underflows, producing a length close to 2^256. When this corrupted slice is passed to CalldataPtrLib.from(), which packs offset and length into a uint256 via or(shl(128, data.offset), data.length), the underflowed length exceeds uint128.max and overflows into the upper 128 bits, corrupting the offset. When runLoop() uses this pointer, the corrupted offset and unbounded length cause the VM to read arbitrary calldata as opcodes, potentially resulting in denial of service or manipulation of swap computations.

Consider adding validation to ensure that begin <= end before computing the slice length.

Update: Acknowledged, not resolved. The team intentionally keeps the library gas-cheap and relies on strategies being validated before use, with takers interacting only with validated strategies and using SwapVM.quote() to verify results before execution.

Aqua Protocol Fee Opcodes Cause Double Fee Payment in Direct Push Mode

When using Aqua-based orders, takers can settle swaps by pushing tokens directly to the maker's Aqua balance. The contract verifies this by comparing the current balance against originalAquaBalanceIn, a snapshot captured before runLoop() executes. The _aquaProtocolFeeAmountInXD opcode calls AQUA.pull() during runLoop() to transfer fees from the maker's Aqua balance. This reduces the balance below the snapshot value. The subsequent balance check (balanceIn >= originalAquaBalanceIn + amountIn) then requires the taker to push extra tokens to compensate for the pull, effectively paying the fee twice.

Example (1:1 swap, 10% fee, taker wants 90 tokenOut):

• Maker's Aqua balance: 1000
• originalAquaBalanceIn captured: 1000
• Computed amountIn: 100 (90 base + 10 fee)
• Fee opcode pulls 10 → balance becomes 990
• Check requires: 990 + takerPush >= 1000 + 100
• Taker must push: 110 (instead of expected 100)

Consider capturing originalAquaBalanceIn after runLoop() completes, or adjusting the balance check to account for Aqua balance changes during program execution.

Update: Resolved in pull request #75. The team enforced a strict instruction ordering requiring protocol fees instructions first, and introduced the amountNetPulled register so the balance check becomes require(balanceIn >= originalAquaBalanceIn + ctx.swap.amountIn - ctx.swap.amountNetPulled, ...).

Extruction Can Violate the Quote/Swap Consistency Invariant

The Extruction instruction delegates pricing/state logic to a maker-chosen external target. During quote(), the VM executes IStaticExtruction(target).extruction() (a view path), while during swap() it executes IExtruction(target).extruction() (a non-view path), and these two interfaces may have different implementations or behavior. In addition, the target can be maliciously formed or updated later using an upgradeable contract such that its logic can change between a taker's quote() and subsequent swap(). As a result, the protocol cannot enforce quote/swap amount equivalence for Extruction-based strategies.

Takers may select orders based on a favorable quote() that later executes with different amounts or reverts in swap(), leading to degraded UX, misrouting, and denial-of-service style failures. While taker-side slippage limits can prevent unexpected fills, this still breaks the Quote/Swap Consistency invariant and can cause unexpected execution outcomes.

Consider adding protocol-level safeguards to omit quote/swap divergence for Extruction, or clearly documenting that programs with Extruction instructions may cause different behavior and amounts between quote() and swap().

Update: Resolved. The team agreed this flexibility is an intentional design decision and mitigated it by adding comprehensive NatSpec documentation warning that target contracts should be immutable and deterministic, noting takers already have slippage protection, and framing it as a "use at your own risk" feature where takers should only interact with validated strategies.

Signing Loose Non-Aqua Strategy Affects All Non-Aqua Strategies of the Maker

The orderHash generated in the SwapVM does not utilize tokenIn and tokenOut addresses given as input by taker. While the Aqua protocol requires the strategies to be shipped with the token, the non-Aqua strategy (i.e., the strategy verified using a signature) does not have any constraints on which tokens will be utilized.

Assume a maker that has multiple non-Aqua strategies signed and active in the SwapVM and the underlying tokens are already approved to the SwapVM. If the maker adds a loose strategy (e.g., signing an order without the balances instruction), the taker can target all the approved tokens to the SwapVM by changing the tokenIn and tokenOut addresses while swapping using the loose strategy. These tokens could have been only approved for a particular strategy on the SwapVM. In this scenario, all non-Aqua strategies are dependent on each other and are highly error-prone to new types of instruction versions and new strategies added by the maker.

This PoC showcases how tokens of perfectly fine strategy1 and strategy2 are swapped using a loose strategy3, bypassing the ideal behavior of strategy1 and strategy2.

Consider adding protocol-level protection for non-Aqua strategies ensuring that all relevant tokens are considered when generating the orderHash. Alternatively, consider thoroughly documenting this interdependency of non-Aqua strategies.

Update: Resolved in pull request #85. The team added a comprehensive "Strategy Hash Uniqueness and Token Safety" section to the README covering mandatory balance instructions for non-Aqua strategies, unique orderHash creation via the _salt() instruction, custom-accounting considerations for _extruction(), safe vs unsafe code examples, risk scenarios with mitigations, and a best-practices summary.

Decay Stores Pre-Fee amountIn Creating Asymmetric Price Impact Protection

In AquaAMM, instructions execute in the following sequence: XYCConcentrate → Decay → Fee → XYCSwap. The Decay instruction provides temporary price impact protection by storing trade amounts as offsets after calling runLoop(), which are later applied to balances on reverse-direction trades to neutralize arbitrage opportunities. The Fee instruction temporarily reduces amountIn for the swap calculation, then restores it to the original value afterward.

Since Fee restores amountIn before control returns to Decay, the stored offset for tokenIn reflects the pre-fee amount instead of the post-fee amount actually used in the swap. For example, if a trader swaps 500 tokens with a 1% fee, XYCSwap computes amountOut based on 495 tokens, but Decay stores 500 as the offset. On a subsequent reverse trade, Decay subtracts 500 from balanceOut instead of 495, over-compensating by the fee amount. This discrepancy is further amplified when using _aquaProtocolFeeAmountInXD, which immediately transfers the fee to the recipient. In this case, the maker's actual balance increases by amountIn - fee, yet Decay stores the full amountIn as the offset, creating an even larger mismatch between the recorded price impact and the actual balance change.

Consider storing the decay offsets based on the post-fee amountIn value. This could be achieved by having Decay capture amountIn after Fee has processed it, or by adjusting the instruction order so that Decay's offset storage occurs before Fee restores the original value.

Update: Resolved in pull request #75. The team described the correct order of instructions in the README and added test/SwapVmAccounting.t.sol and test/AquaAccounting.t.sol for comprehensive conservation checks validating this behavior.

Hook Token Injection Bypasses Concentrated Liquidity Price Bounds

The XYCConcentrate instruction implements concentrated liquidity by adding virtual "delta" balances to real token reserves. These deltas are computed via computeDeltas using the formula deltaA = balanceA / (√(price/priceMin) - 1), which inflates the effective pool size used in the constant product (x*y=k) swap calculation. This reduces price impact for traders while concentrating the maker's capital within a defined price range [priceMin, priceMax]. The delta values are mathematically designed such that when the price reaches a bound, the corresponding real token balance depletes to zero, at which point any further swap attempting to extract that token would revert during the ERC20 transfer, naturally enforcing the price range without explicit validation.

The SwapVM executes maker hooks and taker callbacks via _transferIn and _transferOut between the program computation and the actual token transfers. Specifically, the maker's preTransferOutHook and the taker's preTransferOutCallback execute after runLoop completes swap amount computation but before the outbound safeTransferFrom occurs. Either party can configure a hook contract that injects the output token into the maker's balance during this window. Since the swap amount was calculated using virtual balances (which showed sufficient liquidity via the delta), and the hook provides real tokens for the transfer, the swap succeeds despite the pool being at its price bound. This pushes the effective price below priceMin (or above priceMax), violating the concentrated liquidity invariant. The _updateScales function then persists an inconsistent liquidity state, as it updates based on computed concentrated amounts rather than actual post-hook real balances.

Numerical Example. Consider a pool with priceMin = 0.25 configured via deltaA = 1000 and deltaB = 1000. After several swaps, the pool reaches its lower bound:

An attacker initiates a swap to buy 100 tokenB. The VM computes amountIn = (100 × 4000) / (1000 - 100) = 444 tokenA using virtual balances. Normally, the transfer would revert since real_balanceB = 0. However, the attacker's preTransferOutCallback injects 100 tokenB into the maker's balance before the transfer executes. The swap succeeds, resulting in:

Consider validating that real token balances are sufficient to cover the computed amountOut before executing the transfer, independent of concentrated balance calculations. Alternatively, recalculating and verifying price bounds after hooks execute would ensure the invariant priceMin ≤ effectivePrice ≤ priceMax is maintained. Another approach would be restricting hook capabilities when concentrated liquidity instructions are active.

Update: Acknowledged, will resolve. The team will add documentation notifying makers to not interact with Aqua through hooks.

Protocol Fee on amountIn Is Pulled From the Maker Before the Taker's tokenIn Transfer, Risking Swap Reverts

The fee-on-amountIn opcodes (e.g., _protocolFeeAmountInXD, _aquaProtocolFeeAmountInXD, _dynamicProtocolFeeAmountInXD, and _aquaDynamicProtocolFeeAmountInXD) perform the transfer of fees from the maker inside ctx.runLoop() (i.e., before SwapVM.swap() executes the taker→maker tokenIn transfer). If the maker does not already have sufficient tokenIn balance/allowance, the swap reverts even though the maker would have received tokenIn later in the same transaction. This can cause unexpected swap reverts and also quote/swap divergence in practice for strategies that apply protocol fee on amountIn, reducing liveness/UX.

Consider redesigning fee collection so it is taken after the taker's tokenIn transfer to avoid reliance on the maker's pre-existing tokenIn balance, or document this requirement explicitly.

Update: Acknowledged, not resolved. The team considers this an accepted design constraint for AMM strategies, where maintaining some tokenIn balance/allowance on the maker side is a natural operational requirement of running a two-sided pool, and swaps may revert if the maker does not maintain sufficient balance.

Multiple Fee Instructions Compound Causing Fee-on-Fee for Swaps During isExactOut Path

In _feeAmountIn (ExactOut branch), the fee instruction first executes the remainder of the VM program via ctx.runLoop(), then computes feeAmountIn from the computed ctx.swap.amountIn, and finally increases ctx.swap.amountIn by feeAmountIn. Consequently, if multiple fee-on-amountIn instructions are included in the same program, the outer fee is computed on an amountIn that may already include inner fee adjustments (fee-on-fee), making the total fee order-dependent and effectively compounding rather than being calculated once on the raw swap amount.

This can lead to unexpected economics (total fees higher than an additive interpretation), configuration-order sensitivity (reordering fee instructions changes the effective price/fees), and integration/UX risk (off-chain quoting may assume fees apply to the raw swap amount only).

Consider redesigning the fee collection logic to accumulate fee parameters in context and settle once based on the raw swap-computed amountIn (pre-fee), ensuring fee calculation is independent of instruction nesting/order.

Update: Acknowledged, not resolved. The team considers this behavior intentional and fundamental to SwapVM's composable architecture, giving makers flexibility to construct sophisticated fee structures. Both makers and takers must understand that instruction composition creates fee-on-fee effects and should verify amounts via quote() before execution.

AquaAMM Forwards uint16 Fees Into BPS = 1e9 Fee Math, Capping Fees Below 1 BPS

The AquaAMM.buildProgram helper builds a SwapVM program and appends fee instructions by encoding arguments with FeeArgsBuilder.buildFlatFee and FeeArgsBuilder.buildProtocolFee. Fee opcodes interpret feeBps under the non-standard denominator BPS = 1e9.

Since feeBpsIn and protocolFeeBpsIn are uint16 values, the maximum representable fee is 65535/1e9 (0.0065535%), which is below 1 conventional basis point (1 bps would require feeBps = 1e5). This makes typical fee tiers (for example, 1-100 bps) impossible to express through AquaAMM.buildProgram. Furthermore, supplying values under the conventional 1e4 basis-point convention silently undercharges maker and protocol fees by 100,000x, causing systematic fee under-collection and mispricing. The protocolFeeBpsIn <= feeBpsIn check does not mitigate this unit mismatch.

Consider changing feeBpsIn and protocolFeeBpsIn to uint32 (or uint256) and documenting that the expected scale is BPS = 1e9. Consider also renaming inputs to reflect the 1e9 scale and validating that the chosen representation can express intended fee tiers.

Update: Resolved. The AquaAMM.buildProgram helper was removed from the SwapVM repository.

quote Is Not a Faithful Simulation of swap due to isStaticContext-Dependent Opcode Semantics

The documentation defines Quote/Swap Consistency as a core invariant and describes quote() as a view preview whose returned amounts must match swap(). In the implementation, SwapVM.quote() explicitly constructs the VM context with isStaticContext: true, while SwapVM.swap() sets isStaticContext: false. The program execution engine, ContextLib.runLoop(), executes arbitrary opcodes and trusts ctx.vm.nextPC updates made by those opcodes.

Multiple instructions branch on ctx.vm.isStaticContext, making quote-mode and swap-mode semantics materially different. For example, fee instructions condition real token movement on !isStaticContext, so quote() can succeed even when swap() later reverts due to missing approvals or balances. Stateful instructions similarly skip persistence in quote-mode (e.g., Invalidators._invalidateBit1D), enabling control-flow differences when combined with backwards jumps and the run loop program-counter handling.

Example. A program executes _invalidateBit1D, then uses _jump to execute _invalidateBit1D again. quote() succeeds because _invalidateBit1D does not persist the bit in static context, while swap() reverts on the second _invalidateBit1D because the bit is persisted after the first call.

Consider documenting all such instructions and scenarios that may break the Quote/Swap consistency to ensure that takers are aware of them.

Update: Resolved in pull request #86. Documentation was updated for such cases.

Low Severity

Emission of Misleading Events When Processing Empty Arrays in ship and dock Functions

The ship and dock functions execute without verifying that the provided token arrays contain at least one element. If a caller invokes these functions with an empty array, the contract emits Shipped or Docked events but skips the logic inside the loops that handles balance updates and storage modifications. As a result, the blockchain records an event signaling that a strategy was deployed or deactivated, while strictly speaking, no operations were performed on any liquidity positions or balances. This introduces noise into the event log and can mislead off-chain monitoring tools.

Consider adding a require statement at the start of both functions to validate that the input array length is greater than zero.

Update: Acknowledged, not resolved. The team intentionally avoids the tokens.length > 0 check to save gas for honest users, noting that no storage modifications occur for empty arrays and that indexers are expected to validate strategies against on-chain storage and ignore empty event-only strategies.

dock Accepts tokens Array of Length _DOCKED, Enabling Continuous Event Emission

When a maker docks a strategy via dock, the strategy's balance is reset to 0 and tokensCount is set to the sentinel value _DOCKED. The current implementation does not prevent callers from supplying a tokens array whose length equals _DOCKED. A caller can populate this array (even with duplicate entries), pass existing validations, and still trigger the event, despite the internal state being "docked". This leads to events that suggest asset details that do not correspond to the docked state, potentially misleading off-chain consumers.

Consider reverting when tokens.length == _DOCKED to prevent emitting events with misleading tokens data during docking, while retaining the balance.tokensCount == tokens.length check.

Update: Acknowledged, not resolved. The team noted that Docked events do not include token details (only (maker, app, strategyHash)), that the scenario results in a no-op state transition, and that adding the check would introduce gas overhead without meaningful security benefit.

Missing Validation for Incompatible Aqua Order Configurations in MakerTraitsLib.build()

The MakerTraitsLib.build function constructs swap orders by encoding various configuration flags into the MakerTraits type. The useAquaInsteadOfSignature flag cannot be combined with the shouldUnwrapWeth flag, and the order receiver must be the maker address. The build() function does not validate these incompatibilities when constructing the order. While SwapVM.swap() does enforce these constraints at execution time, this late validation means users can construct invalid orders that will inevitably fail upon swap execution.

Consider adding validation checks in MakerTraitsLib.build() to revert early when useAquaInsteadOfSignature is set to true alongside shouldUnwrapWeth or a custom receiver address.

Update: Acknowledged, not resolved. The team clarified that the referenced logic is test-only (used in Foundry), removed from deployed bytecode as dead code, never invoked on-chain in production, and not used by SDK/provider order creation flows. They consider parameter compatibility a maker/SDK responsibility, with protocol guarantees enforced at execution time.

Debug Opcode Included in Production Bytecode Increases Gas Costs

The buildProgram function in AquaAMM constructs bytecode that defines the swap logic for orders. The function includes a _printContext debug opcode in the production bytecode. This debug instruction, which logs internal VM state using console.log, serves no functional purpose in a production environment and will consume unnecessary gas each time an order is executed.

Consider removing the program.build(_printContext) call from the bytecode construction to reduce gas costs.

Update: Resolved in pull request #74.

Balance Underflow in Dynamic Balances

In the _dynamicBalancesXD instruction, the swapAmountOut is deducted from balances based on the orderHash and tokenOut. However, if the swapAmountOut is greater than the balances stored, the swap would fail. As this deduction only occurs when the context is not static, the quote function would return the value while the swap function would revert without any errors, thus breaking the invariant that quote() and swap() should return identical amounts.

Consider checking against the balances first before deduction and reverting with a meaningful error to show consistent behavior between the quote and swap functions.

Update: Acknowledged, not resolved. The team intentionally omits the check to minimize gas costs and considers the current design optimal for gas efficiency, noting takers must exercise due diligence when specifying swap amounts, especially in ExactOut mode.

Asymmetric Decay Rounding Creates Unfavorable Trading Rates for Low-Decimal Token Pairs

The Decay instruction provides temporary price impact protection by storing trade amounts as offsets that linearly decay over time, using the formula offset * timeLeft / decayPeriod. When a pool contains tokens with significantly different decimal precisions (e.g., BTC with 8 decimals paired against ETH with 18 decimals), the low-decimal token's offset rounds to zero before the high-decimal token's offset expires. For example, with a BTC offset of 3,333 satoshis and an ETH offset of 1×10¹⁵ wei, and a decay period of 65,535 seconds, the BTC offset becomes zero when timeLeft < 19 seconds while the ETH offset still holds 274,662,394,140 wei.

During this asymmetric window, a reverse trade has balanceIn unchanged while balanceOut is still reduced, resulting in the trader receiving approximately 0.09% less output compared to trading after full decay expiry. While the impact per trade is negligible and the window is brief, the asymmetry creates a subtle bias against traders who execute during this period.

Consider scaling decay offsets by a constant precision factor before storage and applying inverse scaling when reading, ensuring both tokens decay at equivalent rates regardless of their native decimal precision.

Update: Acknowledged, not resolved. The team considers this an expected consequence of integer math and heterogeneous token decimals, noting the Decay instruction is a lightweight best-effort mechanism and that the negligible magnitude and brief duration do not warrant a change.

Missing Zero-Address Checks

When operations with address parameters are performed, it is crucial to ensure that the provided address value is not zero. Throughout the codebase, multiple instances of missing zero-address checks were identified, including the aqua operation within the SwapVM contract, the tokenIn and tokenOut parameters during the AQUA.safeBalances operation in both the quote and swap functions of the SwapVM contract, and the aqua operation within the Fee contract.

Consider always performing a zero-address check before assigning a state variable.

Update: Acknowledged, not resolved.

Setting feeBPS == BPS Causes Swap to Revert And Missing Runtime Validation for feeBPS

The build function of the Fee instruction currently performs checks which ensure that feeBPS <= BPS, allowing for fees of up to 100% of the swap amount. However, setting feeBPS == BPS leads to problematic behavior: in the isExactOut path this results in a division-by-zero causing the swap to revert without an explicit error, and in the isExactIn path the computed amountIn becomes zero, which can cause downstream instructions (e.g., XYCConcentrate) to fail. Moreover, the parse* functions do not validate whether the decoded feeBps is actually less than BPS.

Consider updating the check in the Fee:build function to enforce that feeBPS < BPS and adding runtime validations for the fees.

Update: Acknowledged, not resolved.

Allow Makers to Revoke a Signature to Protect Other Non-Aqua Strategies

Once a maker creates a signature for a non-Aqua strategy, the signature cannot be revoked. A loose strategy could have a ripple effect on other strategies, forcing the maker to completely get out of the SwapVM system to safeguard their funds. The Invalidators instructions only provide support to the maker if they are included in the program.

Consider implementing a functionality that allows makers to disqualify a signature.

Update: Acknowledged, will resolve.

unwrapWeth Does Not Require Token to Be WETH

SwapVM supports optional WETH unwrapping during settlement. When takerTraits.shouldUnwrapWeth() is set, _transferFrom first transfers the ERC-20 token into address(this) and then calls IWETH(token).safeWithdrawTo(amount, to). No check enforces that token equals the WETH address configured in the SwapVM constructor. As a result, any contract can be used as tokenOut while still hitting the unwrap branch. More than 5100 ERC-20 tokens have been identified on Ethereum Mainnet where performing token.withdraw(uint256 amount) is feasible; these are generally vault tokens whose underlying asset could get stuck in SwapVM. Since SwapVM is not supposed to hold ETH, this issue is identified as low.

Consider enforcing token == WETH before entering the unwrap branch.

Update: Acknowledged, will resolve.

AquaAMM Imports ProgramBuilder from test/, Breaking Builds That Only Ship src/

The AquaAMM strategy builder is a user-facing helper included under src/ and documented for direct consumption. However, AquaAMM imports ProgramBuilder from test/ using a relative path. Relative imports bypass remappings, and common publishing setups intentionally exclude test/ sources, making compilation and deployment packaging brittle.

Consider moving ProgramBuilder (or a minimal subset) under src/ and ensuring published artifacts include every imported source without requiring test/ to compile.

Update: Resolved in pull request #74.

PeggedSwap Exact-Out Rounding Can Compute amountIn == 0 for Non-Zero amountOut

The _peggedSwapGrowPriceRange2D function computes swap amounts using normalized reserves and an analytical solve. Due to integer quantization, small changes in amountOut can fall into rounding plateaus where v1 does not change, making x1 - x0 == 0 and therefore ctx.swap.amountIn == 0 for a non-zero ctx.swap.amountOut. If the maker enables allowZeroAmountIn, settlement skips the input transfer while still transferring the output. If not enabled, the same plateau can make exact-out swaps/quotes revert, creating an implicit minimum trade size and quote/fill inconsistencies for dust-sized requests.

Consider adding an explicit guard in the exact-out branch to ensure that amountOut > 0 implies amountIn > 0, and documenting a minimum viable amountOut for exact-out PeggedSwap usage.

Update: Resolved in pull request #98. The team updated documentation for allowZeroAmountIn cases.

XYCConcentrate Can Brick an Order by Dividing by Zero After the First Successful Fill

XYCConcentrate concentrates liquidity by scaling ctx.swap.balanceIn and ctx.swap.balanceOut using token-specific delta values and a liquidity ratio via concentratedBalance, which uses liquidity[orderHash] as the current liquidity and initialLiquidity from calldata. If initialLiquidity is encoded as 0, the first interaction takes the currentLiquidity == 0 branch and succeeds, but a non-static call updates liquidity[orderHash] to a positive value. Subsequent calls still carry initialLiquidity = 0 and take the division branch (delta * currentLiquidity / initialLiquidity), reverting with division by zero. This permanently DoSes further fills and can break quoting for the affected order.

Consider validating that initialLiquidity != 0 during arg parsing and reverting early with a dedicated error.

Update: Resolved in pull request #82. The latest version of XYCConcentrate is stateless and doesn't use the _updateScales function.

PeggedSwap Missing Token Address Validation Allows Incorrect Rate Application

The PeggedSwap instruction facilitates swaps between pegged assets using rate multipliers (rateLt and rateGt) that normalize tokens with different decimals, assigned based on token address comparison during execution. The build function only encodes the curve parameters without including the intended token addresses. If a maker holds balances in tokens other than the two intended for the pegged swap, a taker could execute swaps against unintended token pairs, and since the rate multipliers were calibrated for specific token decimals, applying them to mismatched decimals could result in incorrect pricing and fund drainage.

Consider encoding the intended token addresses within the instruction arguments and validating that tokenIn and tokenOut match the encoded addresses before applying the rate multipliers.

Update: Acknowledged, not resolved. The team stated PeggedSwap is intentionally generic and must be used only in pair-scoped strategies, where the token set is enforced via _staticBalances()/_dynamicBalances() (non-Aqua) or the ship() token list (Aqua), and makers should ensure order hash uniqueness.

computeLiquidityFromAmounts Reverts For Out-of-Range Spot Prices Despite Partial Handling

In the XYCConcentrate instruction, the computeLiquidityFromAmounts function handles out-of-range spot prices in its own logic, setting type(uint256).max for the non-needed token, but then calls computeBalances which assumes in-range prices and reverts via unsigned underflow when sqrtPspot is outside [sqrtPmin, sqrtPmax]. This affects users computing position initialization parameters when spot price is outside the intended range.

Consider reverting in computeLiquidityFromAmounts if the condition sqrtPmin <= sqrtPspot < sqrtPmax is not satisfied, or adding boundary checks in computeBalances.

Update: Resolved in pull request #105.

Fee Layer Violates Declared Price Bounds From Initial State

The fee instruction (Fee._flatFeeAmountInXD) is applied as a separate layer on top of the AMM invariant. The new stateless XYCConcentrate instruction correctly limits the AMM curve to the range [P_min, P_max], but the fee layer has no awareness of these declared bounds. For example, a maker declares the order will only execute in [2,000 – 4,000], but the actual execution range is [1,880 – 4,255] at 6% fee, or [1,940 – 4,127] at 3% fee. Takers pay prices outside the maker's intended bounds on every marginal trade near the extremes from the very initial state.

Consider documenting this scenario for makers and takers to avoid confusion regarding actual price.

Update: Resolved in pull request #105. The team stated it is normal behavior for such types of AMM and added documentation to highlight it.

Notes & Additional Information

Inconsistent Modifier Documentation in AquaApp

The NatSpec documentation for the AquaApp contract provides incorrect instructions for using the nonReentrantStrategy modifier. While the comments suggest that the modifier accepts a single argument, the actual implementation requires two parameters: address maker and bytes32 strategyHash. Consider updating the NatSpec comments to accurately reflect the required parameters.

Update: Resolved at PR #41.

Incorrect Max Token Count Value When Exceeding Max Number of Tokens

The ship function caps the token list length at _DOCKED - 1 (254). When reverting with MaxNumberOfTokensExceeded(uint256 tokensCount, uint256 maxTokensCount), the code passes _DOCKED as maxTokensCount instead of _DOCKED - 1, which is inconsistent with the enforced bound. Consider passing _DOCKED - 1 and adding a unit test asserting the emitted value.

Update: Resolved at PR #42.

safeBalances Returns Duplicate Balances When Identical Tokens Are Provided

The Aqua.safeBalances function accepts identical addresses for token0 and token1. When the same token is passed twice, the call succeeds and returns the same balance in both outputs, which can cause callers to unintentionally double-count a single token. Consider reverting when token0 == token1.

Update: Acknowledged, not resolved. The team considers passing identical addresses invalid input on the caller side, notes it introduces no on-chain inconsistency, and expects integrators to validate inputs in their higher-level logic.

Inconsistent Scaling Documentation in PeggedSwapMath.solve

The documentation for the parameters and return value of PeggedSwapMath.solve states that values are scaled by 1e18, but they should be scaled by ONE as defined in the library. Consider updating the NatSpec to reference ONE instead of 1e18.

Update: Resolved in pull request #94.

Incorrect Error and Parameter Naming

The errors defined in the Controls instruction use maker instead of taker as the parameter name for the first address argument, whereas these errors are related to the taker. Moreover, in the TakerTokenBalanceSupplyShareIsLessThatRequired error name, 'That' should be replaced with 'Than'. Consider updating the names to avoid confusion.

Update: Resolved in pull request #89.

Redundant Error Selector in tryChopTakerArgs()

In tryChopTakerArgs(), the TryChopTakerArgsExcessiveLength error selector passed to data.slice() can never trigger because the function clamps length to data.length using Math.min(). Consider replacing data.slice(length, TryChopTakerArgsExcessiveLength.selector) with data.slice(length) to use the non-bound-checking variant.

Update: Resolved in pull request #90.

Unused Imports

The import { ITakerCallbacks } from "../interfaces/ITakerCallbacks.sol"; import in TakerTraits.sol is unused. Consider removing unused imports to improve clarity and maintainability.

Update: Resolved in pull request #91.

Unused Errors

Throughout the codebase, multiple instances of unused errors were identified: DecayApplySwapAmountsRequiresAmountsToBeComputed in Decay.sol, MakerTraitsMissingProgramData in MakerTraits.sol, TakerTraitsMissingHookTarget in TakerTraits.sol, and RunLoopSwapAmountsComputationMissing in VM.sol. Consider either using or removing any currently unused errors.

Update: Resolved in pull request #91.

Missing Security Contact

Providing a specific security contact within a smart contract simplifies the process for individuals to report vulnerabilities. Throughout the codebase, no security contact was identified. Consider adding a NatSpec comment containing a security contact above each contract definition, using the @custom:security-contact convention adopted by the OpenZeppelin Wizard and ethereum-lists.

Update: Resolved in pull request #97.

Lack of Indexed Event Parameters

Within SwapVM.sol, the Swapped event does not have indexed parameters. To improve the ability of off-chain services to search and filter for specific events, consider indexing event parameters.

Update: Acknowledged, not resolved.

Incomplete Docstrings or Missing Docstrings

Throughout the codebase, multiple instances of incomplete or missing docstrings were identified across SwapVM, IMakerHooks, ISwapVM, ITakerCallbacks, the MakerTraits and TakerTraits libraries, important functions such as runLoop(), the build/parse functions and public view functions, and the opcodes and routers contracts. Consider thoroughly documenting all functions/events that are part of a contract's public API, following the Ethereum Natural Specification Format (NatSpec).

Update: Resolved in pull request #92.

Missing Explicit Validation for amountOut < balanceOut in the XYCSwap::_xycSwapXD Instruction

In the ExactOut branch of the _xycSwapXD function, the balanceOut - amountOut formula is used as a denominator without an explicit check that amountOut < balanceOut. If amountOut >= balanceOut, the transaction reverts with an opaque panic rather than a descriptive error. Consider adding an explicit amountOut < balanceOut check.

Update: Acknowledged, not resolved. The team will not add this check in order to keep the protocol gas-efficient.

Decay._decayXD Can Underflow balanceOut When Decayed Offsets Exceed Current Reserves

The Decay instruction maintains per-position offsets and applies them before evaluating the remaining VM program. In _decayXD, balanceOut is decremented via a checked subtraction without validating that the decayed offset is less than or equal to the current balanceOut. If balanceOut decreases independently while the stored offsets persist, the subtraction underflows and reverts, blocking swap() and quote() for that order and direction. Consider clamping the applied offset or using saturating subtraction.

Update: Acknowledged, not resolved.

16-Bit Jump Targets Mismatch 256-Bit PC, Causing Unreachable Code and Truncated Jumps

The VM represents the program counter ctx.vm.nextPC as a uint256 byte offset with no cap on programBytes.length, so programs larger than 65,535 bytes are valid and executable. However, control-flow opcodes in Controls encode/decode jump targets as uint16. Targets at byte offsets >= 65536 are therefore unrepresentable via these opcodes, and any larger value is truncated to the low 16 bits, potentially causing silent misdirected execution and leaving intended code unreachable. Consider aligning addressing widths across the VM and validating jump targets during program construction.

Update: Resolved in pull request #93.

Dynamic Fee Provider Call Allows Malformed returndata to Trigger Generic ABI Revert

In _dynamicProtocolFeeAmountInXD and _aquaDynamicProtocolFeeAmountInXD, dynamic protocol fees are obtained via staticcall to feeProvider. The code treats success == true as proof of a well-formed return value, but if result is empty, short, or malformed, abi.decode(result, (uint32,address)) reverts with a generic ABI decoding error, bypassing the intended FeeProtocolProviderFailedCall() revert. Consider validating result.length == 64 before decoding and reverting with FeeProtocolProviderFailedCall() otherwise.

Update: Resolved in pull request #87.

Misdocumented XYCConcentrate XD/2D Argument Layout

For XD instructions, parseXD expects tokensCount, tokens[], deltas[], then a trailing liquidity word, and buildXD encodes exactly that. However, _xycConcentrateGrowLiquidityXD docstrings describe the third segment as args.initialBalances[] and omit the required liquidity. Similarly, _xycConcentrateGrowLiquidity2D omits documenting the trailing liquidity. Consider aligning the docstrings with the code.

Update: Resolved in pull request #82.

Division-By-Zero in computeDeltas due to sqrt Rounding Down and Missing Zero Validation

The computeDeltas library function divides by sqrtPriceMin - ONE and sqrtPriceMax - ONE. Because Math.sqrt floors its result, sqrtPriceMin == ONE or sqrtPriceMax == ONE can occur near the boundaries, making the denominators zero and causing a division-by-zero panic. The function also does not enforce priceMin != 0 or price != 0. Consider hardening input checks and asserting non-zero denominators, or treating rounded-equal cases as equal.

Update: Resolved in pull request #82. The latest version of XYCConcentrate is stateless and doesn't provide the computeDeltas helper function.

Systematic Rounding Down in Grow Liquidity Causes Price-Range Drift

The liquidity growth flow of the XYCConcentrate contract adjusts in-memory balances via concentratedBalance and then persists a new scale in _updateScales. Floor rounding in delta * currentLiquidity / initialLiquidity and in Math.sqrt(newInv) introduces a systematic loss of precision that accumulates across iterations, shifting the implied bounds and altering quotes near the edges of the target range. Consider using a full-precision multiply-divide with explicit rounding, or documenting the rounding behavior and its maximum cumulative impact.

Update: Resolved in pull request #82.

Concentrated-Liquidity Docs are Outdated

The example on concentrated-liquidity in the README.md file uses a non-existent computeDeltas function and a four-argument build2D function. The current build2D implementation takes only sqrtPriceMin and sqrtPriceMax. Integrators following the README will get wrong or failing code as a result. Consider updating the README to the current logic.

Update: Resolved in pull request #105.

Missing Explicit Validation of sqrtPmin < sqrtPmax During Liquidity Computation

In the XYCConcentrate instruction, neither computeLiquidityFromAmounts nor computeBalances validates that sqrtPmin < sqrtPmax. If called with sqrtPmin >= sqrtPmax, the functions revert with an opaque arithmetic underflow rather than a descriptive error. While build2D validates this at bytecode construction time, these library helpers can be called independently. Consider verifying sqrtPmin < sqrtPmax with a descriptive custom error.

Update: Resolved in pull request #105.

Recommendations

Strengthening Documentation for Makers and Takers

Several instructions are interdependent and manage their own internal storage state. Some instructions also rely on explicit assumptions about preceding or subsequent instructions, or on guarantees provided by the maker. As such, it is recommended to provide guide-style documentation for makers that explains how to construct efficient and safe programs, clearly outlining the assumptions, expectations, and constraints associated with each instruction. This would help prevent the creation of unintended or harmful programs.

Similarly, it is recommended to maintain dedicated documentation for takers in order to help them identify potentially malicious programs authored by makers and to reason about the underlying trust assumptions. For example, the presence of an extruction instruction in a program effectively requires the taker to place full trust in the maker, a risk that should be clearly documented and easy to recognize.

Conclusion

The audit identified 3 critical-, 2 high-, and 12 medium-severity issues, along with multiple low-severity and informational findings, which provide a clear path forward before the production launch. In order to strengthen the security of the codebase, it is recommended to not only fix the reported issues but also improve user-facing documentation, strengthen the end-to-end test suite (particularly around fee-related instructions), and add test suites specifically designed to ensure that these vulnerabilities do not re-emerge.

The recommended fixes are self-contained and do not significantly alter the protocol design. However, any scope expansion, especially involving multi-token integration or logical modifications, should undergo a new audit to guarantee safe and secure interactions for all supported tokens.

The most severe findings include invariant-breaking edge cases in PeggedSwap that can lead to materially incorrect pricing and reserve loss under certain parameterizations or swap directions, and an XYCConcentrate multi-token accounting flaw where shared liquidity tracking across pairs enables cyclic arbitrage. Several major issues were also concentrated in the fee implementation and its interaction with nested runLoop() wrappers, creating order-dependent fee behavior and liveness hazards.

Addressing the critical-, high-, and medium-severity issues should be treated as a high priority, alongside strengthening parameter constraints and expanding tests for adversarial opcode compositions and long-term economic invariants to prevent regressions. Given the interdependencies between certain instructions, a recommendation is made to maintain strong documentation describing each instruction's capabilities, constraints, and trust assumptions, to help makers build correct and efficient programs.

The 1inch team is commended for being highly cooperative and providing clear explanations throughout the audit process.

Appendix

Issue Classification

OpenZeppelin classifies smart contract vulnerabilities on a 5-level scale: Critical, High, Medium, Low, and Note/Information.

• Critical Severity: Applied when the issue's impact is catastrophic, threatening extensive damage to the client's reputation and/or causing severe financial loss to the client or users. Critical issues typically involve significant risks such as the permanent loss or locking of a large volume of users' sensitive assets or the failure of core system functionalities without viable mitigations.
• High Severity: Characterized by the potential to substantially impact the client's reputation and/or result in considerable financial losses, such as temporary loss or locking of a significant number of users' sensitive assets or disruptions to critical system functionalities.
• Medium Severity: Can lead to a noticeable negative impact on the client's reputation and/or moderate financial losses, typically confined to a smaller subset of users' sensitive assets or involving deviations from the specified system design.
• Low Severity: Low impact on the client's operations and/or reputation, representing minor risks or inefficiencies. Areas for improvement that, while not urgent, could enhance the security and quality of the codebase.
• Notes & Additional Information: Issues that, despite having minimal impact, are still important to resolve, contributing to the overall security posture and code quality improvement.