GnoSwap Audit

Summary

Type: Blockchain Infrastructure
Timeline: From 2025-08-11 → To 2025-09-12
Languages: Gnolang

Findings
Total issues: 55 (51 resolved, 3 partially resolved, 1 acknowledged)
Critical: 7 (7 resolved) · High: 7 (6 resolved, 1 partially resolved) · Medium: 6 (6 resolved) · Low: 25 (23 resolved, 2 partially resolved)

Notes & Additional Information
9 notes raised (8 resolved)

Client Reported Issues
1 issue reported (1 resolved)

Scope

OpenZeppelin audited the gnoswap-labs/gnoswap repository at commit f5cb397.

In scope were the following files:

 contract
├── p/
│   └── gnoswap/
│       ├── gnsmath/
│       │   ├── sqrt_price_math.gno
│       │   └── swap_math.gno
│       └── uint256/
│           └── fullmath.gno
└── r/
    └── gnoswap/
        ├── access/
        │   └── access.gno
        ├── common/
        │   └── tick_math.gno
        ├── pool/
        │   ├── liquidity_math.gno
        │   ├── manager.gno
        │   ├── pool.gno
        │   ├── position.gno
        │   └── swap.gno
        ├── position/
        │   ├── burn.gno
        │   ├── liquidity_management.gno
        │   ├── manager.gno
        │   ├── mint.gno
        │   └── position.gno
        ├── rbac/
        │   └── rbac.gno
        ├── router/
        │   ├── base.gno
        │   ├── exact_in.gno
        │   ├── exact_out.gno
        │   ├── router.gno
        │   ├── swap_inner.gno
        │   ├── swap_single.gno
        │   ├── swap_multi.gno
        │   └── swap_single.gno
        └── staker/
            ├── calculate_pool_position_reward.gno
            ├── external_incentive.gno
            ├── incentive_id.gno
            ├── mint_stake.gno
            ├── reward_calculation_incentives.gno
            ├── reward_calculation_pool_tier.gno
            ├── reward_calculation_pool.gno
            ├── reward_calculation_tick.gno
            ├── reward_calculation_types.gno
            ├── reward_calculation_warmup.gno
            ├── reward_calculation.gno
            ├── staker.gno
            └── type.gno

System Overview

GnoSwap is an Automated Market Maker (AMM) built on Gno.land, a Layer 1 blockchain platform powered by the GnoVM. The protocol is written in Gnolang (Gno), an interpreted and fully deterministic implementation of the Go programming language, specifically designed for smart contract development. Similar to Uniswap V3, GnoSwap employs a concentrated liquidity model that allows liquidity providers to allocate capital within specific price ranges rather than across the entire price curve. This approach significantly improves capital efficiency, enabling deeper liquidity around current market prices.

The protocol implements a multi-tier fee structure with four distinct levels: 0.01% (1 tick spacing), 0.05% (10 tick spacing), 0.3% (60 tick spacing), and 1% (200 tick spacing). These tiers accommodate various asset pairs and volatility profiles, with tighter tick spacing enabling more granular price ranges for stable pairs and wider spacing for volatile assets. Pool creation requires a fee of 100 GnoSwap (GNS) tokens to prevent spam and ensure commitment from pool creators.

While the AMM's core functionality mirrors that of Uniswap V3, GnoSwap distinguishes itself through its unified architecture where the Pool and position realms operate as tightly coupled components. The pool realm manages the internal state of all pools, facilitates swaps, and handles mint and burn operations for liquidity while tracking fees for each position. All liquidity operations are gated through the position realm, which manages NFT-based position ownership, allowing LPs to mint, increase, or decrease liquidity and collect accumulated fees. These position NFTs are designed to be non-transferrable, attempting to keep positions tied to their original creators.

Swaps are executed through the canonical router contract and other governance-whitelisted entities. The router handles both single and multi-hop swaps with up to 3 hops, supporting split routing across multiple paths for optimal execution. It manages native token (GNOT) wrapping and unwrapping, while enforcing slippage protection through minimum output requirements for exact input swaps and maximum input limits for exact output swaps.

The protocol features a sophisticated incentive layer through its staking mechanism. Liquidity providers can stake their position NFTs to earn rewards beyond trading fees. The reward system operates on two levels:

• Internal Rewards (GNS Emissions): Positions in governance-approved pools earn GNS tokens from the protocol's emission schedule. Eligible pools are classified into three tiers that receive different shares of the total GNS emissions allocated to stakers: 50% (Tier 1), 30% (Tier 2), and 20% (Tier 3) when all tiers are active. If any tier has no pools, its allocation is redistributed to the remaining active tiers.
• External Rewards: Any user can create incentive campaigns for a pool by depositing reward tokens and 1000 GNS as collateral. These campaigns run for specified periods and distribute rewards to staked positions proportionally to their active liquidity and time in range.

All staking rewards are subject to a warmup schedule that gradually increases reward rates: 30% the first 5 days, 50% the next 10 days, 70% within the next 30 days, and 100% thereafter. This mechanism, configurable by governance or the admin, discourages mercenary liquidity and promotes long-term participation. Rewards not distributed due to the warmup schedule are redirected to the community pool for internal rewards, while for external incentives, they are returned to the incentive creator.

The staker realm maintains awareness of pool state through a hook system, receiving real-time notifications of tick crossings and swap events from the pool realm. This enables accurate tracking of in-range liquidity for reward calculations, ensuring that rewards are only attributed to positions providing active liquidity proportional to the time their liquidity has been active.

The protocol implements multiple fee mechanisms:

• Swap Fee: Collected by liquidity providers based on their share of in-range liquidity.
• Protocol Fee: Configurable portion (0-25%) of the swap fees (per token), set to 0% by default.
• Unstaking Fee: Configurable fee on all collected staking rewards directed to the protocol fee address, set to 1% by default.
• Withdrawal Fee: Configurable fee on all collected swap rewards directed to the protocol fee address, set to 1% by default.
• Router Fee: Configurable fee on swap output amounts processed through the router, set to 0.1% by default.

AMM Architecture

GnoSwap's AMM architecture draws inspiration from Uniswap V3's concentrated liquidity model while incorporating elements resembling Uniswap V4's singleton pool pattern. The pool realm consolidates all liquidity pools into a single contract that manages state for multiple token pairs, similar to V4's singleton design. On the other hand, the position realm handles NFT-based position management analogous to Uniswap V3's NonfungiblePositionManager contract. However, the implementation diverges significantly from its inspirations in several critical aspects that fundamentally alter the protocol's capabilities and risk profile.

The protocol employs a singleton pool architecture where all pools exist within a single pool realm, in contrast to Uniswap V3's factory pattern that deploys separate contracts for each pool. This design choice reduces deployment costs and simplifies cross-pool operations but concentrates systemic risk in a single contract. Also, the architecture follows a single-layer design in which the Pool and position realms are tightly coupled, unlike Uniswap's two-layered approach that separates core (v3-core) from periphery (v3-periphery) contracts. While this coupling simplifies the codebase, it reduces modularity and composability.

A fundamental divergence lies in the fund transfer mechanism. GnoSwap implements direct internal fund transfers between contracts instead of adopting Uniswap's callback-based architecture. Uniswap's callback pattern enables significant composability advantages: it allows asset-agnostic operations, eliminates clunky pre-approvals by letting users fund actions from arbitrary sources, and facilitates complex DeFi interactions including flash loans.

In contrast, GnoSwap's direct-transfer approach prioritizes simplicity but sacrifices these capabilities. Users must pre-approve token transfers before interactions, protocol composition is severely limited, and flash loan functionality is entirely absent. This design decision restricts the protocol to standard token implementations and prevents the atomic multi-step transactions that have become foundational to DeFi composability.

The permission models between the two protocols also differ substantially. Pool functions in GnoSwap are restricted to calls from authorized realms, while Uniswap pools adopt a fully permissionless model where any address can interact directly with core pool functions. This creates a fundamental architectural contradiction in GnoSwap: while all positions must originate from the position realm (enforcing a gated model), the pool realm still tracks individual user addresses as position owners rather than the Position contract itself.

In contrast, Uniswap's approach is more architecturally consistent - when positions are created through the NonfungiblePositionManager, the pool recognizes the manager contract as the owner, maintaining a clean separation of concerns. Uniswap also preserves flexibility by allowing sophisticated users to bypass the manager entirely and mint directly to pools, in which case the user becomes the direct owner. Both systems ultimately maintain user-to-position mappings in their respective managers, but GnoSwap's hybrid approach requiring position realm intermediation while tracking user addresses in the Pool introduces unnecessary complexity that contradicts its stated goal of architectural simplification.

In addition, several feature modifications distinguish GnoSwap from standard Uniswap implementation, with the most significant being the non-transferability of position NFTs. This design choice fundamentally alters the liquidity provision model by preventing secondary markets for positions, limiting composability with lending protocols, and restricting advanced liquidity management strategies. The protocol also implements a more rigid fee structure where no new pool fee tiers can be added post-deployment, and protocol fees are applied universally across all pools rather than on a per-pool basis. Beyond standard swap fees, the protocol imposes additional fee layers including withdrawal fees on collected swap fees and staking rewards, plus router fees of on all swap outputs.

Functional constraints further differentiate the protocols. The IncreaseLiquidity function restricts additions exclusively to position owners, preventing third-party top-ups or permissionless position funding that could enable novel incentive mechanisms. The CollectFee function lacks partial collection capabilities with amount limits and cannot direct fees to arbitrary recipients, limiting flexibility in fee-management strategies. Position burning follows an unconventional pattern whereby the positions are only marked as burned through a boolean flag instead of the NFT itself being destroyed, a design choice that accommodates reposition workflows but may cause confusion about the actual state of positions.

Regarding native-token handling, the protocol includes built-in wrapping and unwraping for GNOT tokens, eliminating the need for separate wrapper contracts. While this integration simplifies user interactions with native tokens, it adds complexity to the core protocol logic and increases the attack surface of critical contracts.

These architectural decisions reflect deliberate trade-offs between simplicity, gas efficiency, and functionality. The streamlined approach implemented in the GnoSwap protocol reduces complexity and certain attack surfaces while potentially improving gas costs for common operations. However, these benefits come at the cost of limited flexibility, reduced composability with the broader DeFi ecosystem, and complications for future protocol upgrades. This architectural philosophy positions GnoSwap as a more controlled, less permissionless variant of the Uniswap model, prioritizing operational simplicity over the open composability that has driven much of DeFi's innovation.

Arithmetic Model and Constraints

GnoSwap implements a mixed-precision arithmetic model that largely mirrors Uniswap V3's approach but introduces critical changes in token-amount handling that fundamentally alter the protocol's operational bounds.

The protocol maintains Uniswap's precision standards for core AMM calculations. Price representation uses X96 fixed-point arithmetic (Q64.96 format) where prices are stored as square roots multiplied by 2^96, enabling efficient computation while maintaining sufficient precision for tick-based pricing. Fee-growth tracking employs Q128 fixed-point arithmetic (Q128.128 format) to accumulate fees per unit of liquidity with high precision over extended periods. These design choices ensure mathematical consistency with Uniswap's proven model for price discovery and fee distribution.

Liquidity constraints follow Uniswap's boundaries with positions limited to uint128 maximum total liquidity, while liquidity delta operations are constrained to the int128 range to maintain signed arithmetic safety. These limits ensure that liquidity calculations remain within safe computational bounds while providing sufficient range for practical usage. Notably, while these limits are enforced at operation boundaries, the protocol internally stores and performs calculations using uint256 throughout, creating computational overhead without corresponding precision benefits.

The most significant arithmetic deviation lies in token-transfer constraints. While Uniswap operates with uint256 token amounts throughout, GnoSwap restricts all token transfers to int64 bounds, limiting transfers to a maximum of 2^63-1. This limitation can hinder user experience and becomes particularly acute for tokens with high decimal precision, extreme supplies, or low individual unit value, where normal operating amounts could exceed transfer limits.

The singleton pool architecture compounds these arithmetic constraints by concentrating all tokens from all pools in a single contract. Unlike Uniswap's isolated pool design where each pool manages its own token balances, GnoSwap's pool realm holds aggregate balances across all pools. This concentration increases the likelihood of hitting int64 boundaries as total protocol TVL grows, potentially creating systemic limits on protocol scalability. The mixed use of int64 for transfers while maintaining uint256 for internal accounting creates additional complexity in boundary condition handling and overflow prevention.

These arithmetic constraints create a different operational envelope than Uniswap. While the core AMM mathematics remains compatible, the reduced transfer limits mean that GnoSwap cannot handle the same token scales as Uniswap, particularly for high-supply or high-precision tokens. Protocol integrators must carefully consider these bounds when selecting tokens for pools, as tokens that function normally on Uniswap might hit transfer limits on GnoSwap. The protocol effectively operates with a dual-arithmetic model: high precision for internal calculations but constrained precision for external interactions, creating a unique set of trade-offs between computational efficiency and operational flexibility.

Security Model and Trust Assumptions

The GnoSwap protocol's security model leverages role-based access control and operational pause management to ensure permission enforcement and resilience. This section examines these security components alongside the trust assumptions underlying the protocol's operation on the Gno.land blockchain.

Access-Control Model

The protocol implements a two-tiered Role-Based Access-Control (RBAC) system through the rbac and access realms. The rbac realm serves as the authoritative source of role mappings, maintaining a two-step ownable state governed by the admin role. This realm holds the canonical mapping of roles to addresses and acts as the main access-control keeper for the entire protocol. The rbac package provides the stateless infrastructure supporting the rbac realm, enabling dynamic permission assignment and two-step ownership transfers to prevent accidental loss of control.

The access realm operates as a synchronized mirror of the role configurations stored in the rbac realm, propagating these mappings across all protocol contracts. When the admin or governance updates role addresses in the rbac realm, these changes are synchronized to the access realm, which then provides consistent role information to all protocol components. This architecture enables centralized permission management where role addresses can be dynamically adjusted by the admin or governance, with built-in protections preventing the removal of system-critical roles.

Operational Pause Management

The halt package implements a granular pausability framework with distinct escalation levels, allowing selective disabling of protocol operations. The halt mechanism provides fine-grained control over individual protocol operations that can be paused independently by the admin or governance. These operations include the following:

• Pool operations
• Staker operations
• Router operations
• Position operations
• Withdrawals

While this granular approach offers operational flexibility during emergencies or maintenance, it amplifies centralization risks. The admin or governance maintains unilateral authority to halt any protocol operation instantly, without warning or time delays.

Trust Assumptions

The audit relies on multiple layers of trust assumptions ranging from those pertaining to the Gno ecosystem infrastructure to those regarding the protocol-specific implementations. While some components were outside the audit scope, issues and weaknesses identified in related areas highlight the importance of these assumptions.

Gno Infrastructure and Core Dependencies

• GnoVM and Node Reliability: It is assumed the Gno chain operates correctly, including proper memory allocation limits, accurate gas charging, and transaction atomicity guarantees. Any consensus failures or VM-level bugs could compromise protocol security.
• Time Consistency: It is assumed that time.Now produces consistent timestamps across all nodes equivalent to Ethereum's block.timestamp, critical for time-based reward calculations, emissions and deadline enforcement. The protocol makes an assumption that blockchain consensus mechanisms prevent significant timestamp manipulation, though minor variations within acceptable bounds may occur.
• Address Validation: It is assumed that Address.IsValid correctly validates all address formats and prevents malformed addresses that could cause security issues. Specifically, the protocol relies on this validation to prevent position key collisions where two different users could accidentally or maliciously claim ownership of the same liquidity position due to address parsing errors.
• Pre-Launch Stability: The protocol runs on pre-production Gno.land infrastructure that is still under active development. This creates risks including: undiscovered consensus bugs that could halt the chain, breaking changes to core APIs that contracts depend on, state corruption or rollbacks, and fundamental protocol changes before mainnet that could break deployed contracts entirely.

Token Standards and Interfaces

The protocol assumes strict adherence to established token standards:

• GRC-20 Compliance: All registered tokens must strictly follow standard ERC-20 semantics without hooks, transfer fees, or non-standard behaviors. The protocol lacks compatibility with rebasing tokens, fee-on-transfer tokens, or tokens with callback mechanisms, which could disrupt accounting or enable reentrancy attacks.
• GRC-721 NFT Standard: Position NFTs are assumed to be standard ERC-721-like tokens without hooks or custom logic that could interfere with position management.
• Token Registration Integrity: The token registration system assumes that registered tokens behave predictably and cannot be upgraded to introduce malicious functionality post-registration.

Mathematical and Computational Foundations

Critical mathematical operations and standard libraries are assumed to be sound and secure:

• Standard Library: It is assumed that there are no vulnerabilities in the Gno standard library packages including std, strings, strconv, encoding, and time, along with other core utilities that provide fundamental operations across the protocol.
• Arithmetic Libraries: It is assumed that there are no vulnerabilities in the underlying math libraries, mainly uint256, and int256.
• Bitwise Operations: It is assumed that 2's complement arithmetic is performed for all bitwise operations, which is particularly critical for the tick bitmap management.

Protocol-Specific Components

Several protocol components are assumed to be secure without being part of this audit's scope. However, it is recommended that these components undergo a comprehensive security audit:

• Transfer Logic: The pool/transfer.gno module handling all token movements is assumed to be secure.
• Native Token Handling: The position/native_token.gno and staker/wrap_unwrap.gno wrapping/unwrapping logic is assumed to be secure.
• rbac Package: The underlying role-based access-control package is assumed to work as intended without any vulnerabilities.
• Tick Mathematics: The (pool/tick.gno and pool/tick_bitmap.gno) tick libraries are assumed to work identically to Uniswap V3's implementation without introducing calculation errors, rounding vulnerabilities or other issues.

Privileged Roles

The admin role and governance share extensive overlapping privileges (listed below) that grant them near-complete control over protocol economics and operations:

• Fee Management: Authority to modify all protocol fee parameters without effective limits, including withdrawal fees on collected swap fees, unstaking fees on staking rewards, protocol fees, and router fees.
• Incentive Pool Management: Authority to manage the tiered pool system by adding or removing pools from tiers, directly controlling which pools receive GNS emission rewards and their distribution percentages.
• Reward Schedule Control: Authority to modify the warmup schedule that determines reward vesting percentages and timeframes.
• Emission Distribution: Authority to control the allocation percentages of GNS emissions between stakers, developer operations, the community pool, and governance stakers.
• Protocol Fee Collection: Authority to withdraw accumulated protocol fees from swap operations to arbitrary addresses without any enforced limits.
• Access-Control Management: Authority to register new roles, update role addresses, and remove non-system roles through the RBAC system.
• Swap Whitelist Management: Authority to control which addresses are permitted to directly call the swap function.
• Operational Halting: Authority to alter halt levels to pause protocol operations at a granular level, including halting all withdrawals.
• Economic Parameter Control: Authority to modify the pool creation fee and external incentive collateral requirements.
• Token Registration: Authority to determine which tokens are permitted to operate within the AMM through the token registration system.
• Upgradeability: Authority to upgrade realm versions via the RBAC management system.

Upgradeable Architecture

The GnoSwap protocol attempts to implement a limited upgradeability model through the RBAC system. While individual realm code cannot be directly upgraded on Gno (due to lacking proxy patterns), the admin can modify role addresses to point to new contract implementations, theoretically allowing other realms to reference updated versions. However, this approach creates fundamental architectural problems that render it practically unusable and potentially catastrophic.

The most critical limitation is the absence of state migration. When role addresses are updated to point to new implementations, all existing user state, positions, liquidity, and accumulated fees remains trapped in the original contracts. This creates an impossible situation when addressing discovered vulnerabilities. If a critical bug is found in v1, it remains permanently exploitable since the vulnerable contract continues to hold all user funds and state. While pausing provides temporary mitigation against active exploitation, it merely delays the inevitable without offering a resolution path. Any attempt to remedy this would require coordinating all users to manually withdraw and migrate their positions under time pressure, creating a race condition with potential attackers who could exploit the vulnerability during the migration window.

Furthermore, the upgradeability pattern exhibits severe inconsistencies that could permanently lock user funds. Consider a scenario where the position realm is upgraded to v2: the NFT state remains in v1, but the pool realm's access control now dynamically loads the v2 address from the RBAC system. Any calls from v1 Position to Pool would fail access-control checks since the Pool no longer recognizes v1 as the authorized Position contract. Users' NFTs become orphaned, unable to interact with pools to modify liquidity or collect fees. Further complications arise from cached role addresses throughout the protocol, as various contracts cache role addresses at initialization or during operations. When the RBAC system updates these addresses, cached values become stale, creating a split-brain scenario where different operations of the protocol access different addresses for the same realm.

These architectural flaws make the upgradeability mechanism not just limited but actively dangerous. Any attempt to use this upgrade pattern could fragment the protocol, lock user funds, or create exploitable inconsistencies. The protocol effectively operates as non-upgradeable in practice, with the theoretical upgrade capability presenting more risk than benefit.

Production Readiness

The protocol in its current state is not suitable for production deployment. The audit identified multiple critical vulnerabilities that would result in immediate and catastrophic failure if deployed to mainnet. These are not edge cases or theoretical attack vectors, but fundamental breaks in core protocol functionality that affect normal user operations.

• Critical Functionality Failures: Basic and frequently used operations, including swaps via the canonical router, are fundamentally broken, potentially denying service or over/under-delivering tokens to users.
• Complete Protocol Compromise: Multiple pathways exist for complete drainage of protocol funds, including vulnerabilities that allow external actors to drain the entire staker realm.
• Irreversible Fund Locking: Several mechanisms can permanently trap user funds, including broken upgrade patterns, non-functional NFT transfers, incorrect state management, and fund-transfer errors that make assets permanently inaccessible.
• Systemic Testing Failures: The prevalence of these critical issues indicates a fundamental absence of adequate testing infrastructure. Comprehensive unit, fuzz, and integration tests would have caught the vast majority of these failures.
• Code Quality Concerns: The codebase exhibits inconsistent patterns, unnecessary type conversions, string-based arithmetic operations, and inconsistent boundary checks, requiring substantial refactoring before production consideration.

These issues represent systemic failures in the protocol's design and implementation. Comprehensive refactoring of core components, addressing key architectural concerns, implementation of rigorous testing frameworks, and additional rounds of security reviews will be necessary before this protocol can be considered safe for user funds.

Critical Severity

Exact-In Swaps Transfer Wrong Amount for Native Output

The ExactInSwapRoute function allows users to swap tokens specifying the exact amount they want to supply, and enforcing a minimum amount they want to get. When the output token is WUGNOT, the function attempts to unwrap and transfer the amount specified in params.exactAmount, which represents the input amount for exact-in swaps, and not the actual output amount received from the swap operation. This causes users swapping valuable tokens (worth more than GNOT) to receive far less tokens than expected.

Conversely, when swapping less valuable tokens, the transaction fails because the router attempts to unwrap more WUGNOT than it received. Any WUGNOT stranded in the router from underpaid transactions becomes vulnerable to theft by attackers who can craft specific swaps to claim these trapped funds. This mismatch causes severe financial discrepancies, effectively breaking all exact-in swaps to native GNOT, causing loss of user funds and creating attack vectors for draining router balances.

Consider modifying the unwrap call to use the absolute value of outputAmount after fees have been deducted, ensuring that users receive the correct amount of native tokens from their swaps.

Update: Resolved in pull request #834 at commits 052de9c and 3b46536. The team stated:

We have changed it to use the outputTokenAmount, which is the quantity for which the fee was collected.

WUGNOT Staker Funds Can Be Stolen via Incorrect Token Unwrapping

The CollectReward function takes an unwrapResult parameter that allows users to receive native tokens instead of wrapped tokens when claiming WUGNOT staking rewards. However, when unwrapResult is set to true, the function attempts to unwrap all external reward tokens by calling unwrapWithTransfer, regardless of whether the reward token is actually WUGNOT. This flawed logic treats every external reward token as if it were wrapped native currency, attempting to convert it to GNOT through the unwrapping mechanism.

An attacker can exploit this by creating an external incentive with a worthless token for a pool that also has legitimate WUGNOT incentives, then calling CollectReward with the flag set which causes the contract to send GNOT from its WUGNOT reserves that belong to other legitimate incentives. Since the staker realm holds WUGNOT from multiple external incentives in a commingled pool, the unwrapping operation drains funds indiscriminately, allowing the attacker to steal WUGNOT deposited by honest incentive creators.

Consider modifying the unwrapping logic to check if the reward token is specifically WUGNOT before attempting to unwrap and send native tokens to the user.

Update: Resolved in pull request #858 at commit c50eef3. The team stated:

Modified the condition to check if the token path is wugnot before calling the unwrapWithTransfer function, and checked that the described issue no longer occurs in the issue reproduction test.

Staked NFTs Become Permanently Locked After Pool Tier Removal

Users cannot recover their staked LP NFTs from pools that have been removed from incentive tiers and lack external incentives, creating a permanent fund lock situation. The UnStakeToken function calls applyUnStake, which invokes getPositionStakeTokenAmount to validate the unstaking operation. This validation inappropriately checks poolHasIncentives, which returns an error if the pool neither has internal tier status nor external incentives.

When governance removes a pool from the tier system by setting its tier to 0, and the pool has no active external incentives, this validation causes all unstaking attempts to revert with errNonIncentivizedPool, even though the positions had been legitimately staked when the pool was incentivized. The NFTs representing these liquidity positions remain permanently locked in the staker realm with no recovery mechanism, unless the pool is re-incentivized.

Consider modifying the unstaking flow to bypass incentive validation for existing deposits, ensuring that users can always recover their staked assets regardless of the pool's current incentive status.

Update: Resolved in pull request #849 at commits d1d62fa and 48bf0c6. The team stated:

Modified the applyUnstake function to remove unnecessary incentive verification and perform only ownership verification.

Key changes:

• Removed getPositionStakeTokenAmount call (including incentive verification)
• Maintained only hasTokenOwnership verification

External Incentive Creators Can Drain the staker Realm

The EndExternalIncentive function can be called by either the admin or the incentive creator to claim back the GNS deposit and any remaining rewards, but lacks any mechanism to prevent multiple invocations. After the initial call successfully refunds the depositGnsAmount and transfers remaining rewards, the incentive record is updated but never deleted or marked as fully claimed.

Subsequent calls continue to pass the validation checks, allowing unlimited refund claims of 1000 GNS per call plus any rewardLeft amount. An attacker can exploit this by creating a minimal external incentive, waiting for it to end, then repeatedly calling EndExternalIncentive to drain the staker realm's entire GNS balance, stealing funds belonging to other internal and external incentives.

Consider implementing a flag to mark incentives as ended, preventing multiple refund claims.

Update: Resolved in pull request #848 at commit 33fbf6e. The team stated:

This modification introduces a refunded flag to ensure that deposited GNS or rewards can only be received exactly once. This change allows rewards to continue being collected even after a refund.

The key modifications are as follows:

• external_incentive.gno: Added duplicate check to refund logic
• type.gno: Added refunded field to ExternalIncentive struct

GNFT Transfers Permanently Lock Liquidity

The protocol currently permits the transfer of LP-position GNFTs between addresses, contradicting the protocol's documentation. However, this functionality leads to a critical issue whereby funds become permanently locked due to the way ownership is tracked between the position and pool realms, specifically through the use of the positionKey. The positionKey that is generated upon position creation in the Mint function, incorporates the original owner's address, tick range, and pool parameters, effectively binding the position to the initial owner.

When an GNFT is transferred using the standard transfer functions, the GNFT realm updates the token's ownership, but the pool's internal mapping retains the original owner's address in the positionKey. This misalignment results in operational failures for the Burn and Collect functions, as they cannot identify the position under the new owner's address. Consequently, the original owner loses the ability to authorize operations, leading to the permanent inaccessibility of liquidity and accumulated fees.

Consider restricting position GNFT transfers to and from the staker realm only.

Update: Resolved in pull request #835 at commit b8ee5d8. The team stated:

Added permission checks to gnft's TransferFrom and SafeTransferFrom methods.

Broken Upgradeability Pattern

The protocol's upgrade mechanism creates an irreconcilable state fragmentation that permanently locks user funds. When realms, such as pool, router, staker, or position are upgraded by updating their addresses in the rbac, and subsequently, access realms, the upgrade only updates the address mappings without migrating the existing state data from the old realm to the new one. While the new realm correctly receives access-control permissions through dynamic IsAuthorized calls, realms retain cached references to the old contract address in their local variables, which are often used for token operations.

This creates a catastrophic split where the new contract has permission but no incoming calls, while the old contract receives all the calls but lacks permission to execute them. Critical operations like token transfers, approvals, and position modifications fail in both directions - the new contract cannot receive user interactions because other contracts do not know its address, and the old contract cannot execute operations because it fails dynamic access control checks. The result is complete denial of service with user funds trapped in v1 contracts.

Consider implementing a proxy pattern with delegate calls to enable true upgradeability, or accepting immutability as a design constraint and removing the upgrade functionality entirely.

Update: Resolved through architectural redesign. Acknowledged and reimplemented with a new upgradeability pattern that eliminates the original state fragmentation issue. The new upgradeability pattern was a primary focus of the extended audit, where additional vulnerabilities were identified. Please refer to the GnoSwap extended audit report for the complete assessment of the current implementation.

Router Fee Handling Causes Denial of Service (DoS) and Exact-Out Violations

The finalizeSwap function's fee-handling mechanism causes DoS for GRC-20 output swaps and violates exact-out semantics for native token swaps. When handleSwapFee is called to process the 0.15% router fee, the behavior differs critically between token types. For native token outputs, the router receives the swap output for unwrapping, deducts the fee, then applies slippage protection. In exact-out swaps, the exactOutAmount validation confirms the full amount was output by the pool's swap operation, but the fee is subsequently subtracted before sending the output to the user. The slippage validator also will not catch this exact-out underpayment, since it only checks that the amount in is not more than the maximum specified, directly violating the exact-out guarantee.

For GRC-20 outputs, the issue is more severe: the pool transfers tokens directly to the user, then handleSwapFee attempts to transfer fees from the pool address to the protocol. Since the pool has already sent all output tokens directly to the user and has no approval to the router to transfer any funds, this would fail with insufficient allowance, causing all GRC-20 output swaps to revert, resulting in complete DoS. This makes the router unusable for any swap where the output is a GRC-20 token, severely limiting protocol functionality.

Consider restructuring the swap flow to have all outputs first sent to the router, which then deducts fees before forwarding to users, ensuring that both exact-out guarantees are maintained and GRC-20 swaps function correctly.

Update: Resolved in pull request #834. However, the changes introduced another issue: the router was returning less than the expected amount for exact-out swaps. This issue has been resolved in pull request #902. The team stated:

We have changed the system so that fees are collected uniformly through the router and passed on to the user. Regarding this [new] issue, we’ve modified the exact out swap to internally request the swap amount including the router fee to ensure users receive the expected quantity.

High Severity

Incorrect TWAP Oracle Implementation

The TWAP oracle implementation in the pool realm contains multiple flaws that compromise price feed accuracy. In the Swap function, the oracle's updatePriceCumulatives is called after the swap state has already been applied, causing the tick cumulative to incorrectly use the post-swap tick value multiplied by the time delta, whereas it should reflect the pre-swap tick that was active during that period. In addition, when positions are modified through Mint or Burn operations where the current tick falls within the position's range, the pool's active liquidity is modified without updating the oracle, causing subsequent oracle calculations to operate on stale liquidity data.

The implementation also lacks the tracking of historical values and secondsPerLiquidity metrics that are essential for robust TWAP calculations, and only updates the oracle state after all swap steps have been performed instead of updating it during tick crosses when liquidity changes occur. These issues can lead to manipulable or inaccurate price feeds that could cause incorrect valuations for protocols relying on the oracle.

Consider implementing a comprehensive oracle-updating mechanism that captures the state before modifications, tracks all the required cumulative values including secondsPerLiquidity, updates upon every liquidity-affecting operation, and maintains proper historical records for accurate time-weighted calculations.

Update: Partially resolved in pull request #853. The team now successfully calls the oracle with the pre-swap value and updates the oracle when positions in range are modified. However, the PR introduced core logic changes to the oracle implementation (for example, in files pool/pool_type.gno, pool/oracle.gno) which are out-of-scope and we did not review.

Update (extended audit): During the extended audit, while the Oracle implementation remained out of scope, we reviewed the concerns raised in this issue:

• Concern: TWAP uses post-swap tick for the elapsed time; not updated at tick crosses during swaps.
  Fix: In v1/swap.gno, the oracle write is performed before applying the swap result, and it uses the pool’s pre-swap tick and liquidity; tick-cross liquidity changes during the swap are reflected in the post-swap state for subsequent intervals.

• Concern: Mint/Burn can change active liquidity (when in-range) without oracle update.
  Fix: In v1/position.gno, when the current tick is within the position range, the implementation writes an oracle observation before changing active liquidity.

• Concern: Lack of historical tracking and missing secondsPerLiquidity metric.
  Fix: The oracle uses an ObservationState circular buffer to track cumulative tick, cumulative liquidity, and implement secondsPerLiquidityCumulativeX128 with support for secondsAgo observations.

We therefore mark this issue as resolved. However, we emphasize again that we did not fully audit the TWAP oracle, as it was out of scope for the extended audit. Additionally, we identified a new Oracle issue during the extended audit; please refer to the GnoSwap extended audit report for details.

New Roles Cannot Be Added

When RegisterRole is called, it creates a new role instance using NewRole(roleName) which initializes the role with an empty address string. The function then attempts to synchronize role addresses in the access realm by calling updateAccessRoleAddresses, and passing in the map containing all the roles, including the newly created one with the empty string address. When this map is passed to SetRoleAddresses, the validation logic rejects the empty address and panics with an error message. This prevents the protocol from adding new privileged addresses or expanding administrative capabilities.

Consider modifying the registration flow to enable role registration by specifying the address of the new role in RegisterRole.

Update: Resolved in pull request #844 at commits d24dfbc, 56eabac, d880633, and d4bcca3. The team stated:

Modified RegisterRole to also receive the role address as a parameter.

Premature Update of lastCollectTime in CollectReward

The CollectReward function updates the lastCollectTime to the current time before verifying that external incentive payouts can actually be completed. If an external incentive has insufficient funds, the function emits an InsufficientExternalReward event and continues the execution flow, but the rewards for that period are permanently lost because lastCollectTime has already advanced. This issue becomes critical when combined with the WUGNOT unwrapping vulnerability, where attackers can drain WUGNOT reserves from the staker realm - legitimate users attempting to collect WUGNOT rewards find the balance depleted, and due to the premature timestamp update, cannot reclaim their earned rewards in future collection attempts. The time window between lastCollectTime and the current time represents permanently forfeited rewards with no recovery mechanism. Users who provided liquidity and earned rewards legitimately lose their earnings through no fault of their own, whether due to incentive creator mistakes, contract bugs, or malicious fund drainage.

Consider restructuring the reward collection flow to only update lastCollectTime after successfully transferring all calculated rewards, or implementing a per-incentive tracking mechanism that allows users to retry collection for specific failed incentives without losing the entire time period.

Update: Resolved in pull request #845 and pull request #892. The team stated:

We have modified the deposit object to track the last collected time for each incentive.

Unclaimed Rewards Become Unclaimable After Pool Tier Removal

When a pool is removed from incentive tiers through a governance action, stakers with uncollected historical rewards permanently lose access to their legitimately earned GNS tokens. The calculatePositionReward function checks if poolTier.CurrentTier(poolPath) != 0 before processing internal rewards, completely skipping GNS reward calculations for pools no longer in the tier system. This logic fails to distinguish between rewards earned during the pool's incentivized period and future rewards, treating all historical accumulations as invalid once the tier status is removed.

Stakers who provided liquidity while the pool was actively incentivized find their earned rewards trapped when attempting to collect through CollectReward, as the calculation returns zero for internal rewards despite the position having accumulated rewards during the incentivized period. The minted GNS tokens allocated for these rewards remain locked in the staker realm with no distribution mechanism, effectively burning protocol emissions that were intended to reward liquidity providers.

Consider implementing a checkpoint system that preserves historical reward accumulations up to the tier-removal timestamp, allowing users to claim rewards earned during incentivized periods while preventing new accumulation after removal.

Update: Resolved in pull request #846 and pull request #903. The team stated:

Even if a PoolTier is removed, reward collection should still be possible, so we modified the code to not remove incentive information even when RemovePoolTier is called.

Stale Emission Rate Cache on Governance Changes

The staker realm's reward-distribution mechanism relies on cached emission rates that can become stale when governance updates allocation parameters. The PoolTier structure maintains a currentEmission field that stores the GNS emission rate allocated to stakers, which is updated when a pool changes tier and through the cacheReward function called during user interactions like staking, unstaking, and collection of rewards. When governance modifies the staker's allocation percentage through the emission module, the cached rates continue using outdated values until the next triggering event occurs, creating a temporal inconsistency where reward calculations do not reflect current governance or admin decisions. This delay can last indefinitely for inactive pools with no user interactions, causing users to receive incorrect reward amounts based on obsolete emission rates. Users staking or collecting during this window receive rewards calculated with outdated rates, potentially leading to the over- or under-distribution of protocol incentives.

Consider implementing a governance callback mechanism that forces cache reward updates when emission parameters change.

Update: Resolved in pull request #847 and pull request #884. The team stated:

We fixed this issue by adding a callback to update the cache in the ChangeDistributionPct function. The added test was designed to compare calculations before and after the fix, and I confirmed that it continued to use the cached value before the fix, and that it calculated values using the updated cache after the fix. Additionally, PR #884 is work to adjust the callback invocation location, as there was a possibility of calculating past periods with changed values depending on when the callback was called.

Unchecked Transfer of Funds

The codebase exhibits multiple instances where transfers of funds via tokenTeller are performed without validation of their success. This lack of checks introduces a risk of state inconsistencies within contracts, potentially leading to financial losses for users. These instances are notably found with the use of grc20reg_helper, where tokenTeller transfers are invoked without subsequent validation to ensure their completion. The CollectReward function transfers rewards to users without checking success, while handleUnstakingFee sends protocol fees without validation.

Within the handleSwapFee function, both user transfers and protocol fee transfers proceed unchecked. The decreaseLiquidity function performs unchecked transfers when returning token0 and token1 to users. Similarly, the collectFee function executes multiple unchecked transfers for wrapped token0, wrapped token1, and the protocol fees. The absence of validation for transfer operations creates a critical accounting vulnerability: when transfers fail silently, the contract continues operating with incorrect balance assumptions, creating a divergence between the contract's internal state and actual token holdings. This lack of transfer validation transforms recoverable errors into permanent state corruption that directly threatens user funds.

Consider implementing explicit checks for invocations of tokenTeller transfers (e.g., validating return values or using safeGRC20 wrappers) and reverting state changes if transfers fail, ensuring atomicity.

Update: Resolved in pull request #854 at commits 96e03f9 and f0502a1. The team stated:

When handling GRC20 assets such as SafeGRC20Transfer, SafeGRC20TransferFrom, and SafeGRC20Approve, we have modified the behavior to trigger a panic and revert the state when an error occurs.

Incorrect Unclaimable Period Tracking in processUnclaimableReward

The processUnclaimableReward function unconditionally sets lastUnclaimableTime = endTime without checking the pool's current liquidity state, creating two exploitable scenarios. When CollectReward is called during an unclaimable period (staked liquidity is zero), the function overwrites the original unclaimable start time with the current timestamp, causing all rewards accumulated from the actual start of the unclaimable period until the collection call to be permanently lost.

Conversely, when called during a claimable period (staked liquidity is non-zero), the function incorrectly initiates an unclaimable period tracking by setting lastUnclaimableTime, even though the pool is actively claimable. This causes the next actual unclaimable period to inherit this incorrect start time, overestimating the unclaimable duration and accumulating more unclaimable rewards than it should. Attackers can exploit this by strategically timing CollectReward calls to either minimize unclaimable rewards (keeping the excess in the staker realm) or maximize unclaimable rewards, forcing the staker to drain its GNS funds by sending them to the protocol_fee realm and causing subsequent reward collection to fail for all users.

Consider checking the unclaimable state before overwriting lastUnclaimableTime, and appropriately handling unclaimable periods and their reward calculations.

Update: Resolved in pull request #871. The fix properly addresses the unclaimable period tracking issue. Initial concerns about zero-liquidity initialization were found to be handled correctly upon further review during the extended audit.

Medium Severity

Native Tokens Permanently Stuck in WUGNOT Mint Operations

The Mint function fails to validate native token transfers when users specify WUGNOT as the input token, causing permanent fund loss. When users specify token paths as WUGNOT, but mistakenly send native GNOT via std.OriginSend, the processTokens function only checks if any of the token strings are GNOT to trigger the wrapping logic. Thus, the native tokens bypass all processing and they are neither wrapped nor returned. If the user has previously given sufficient WUGNOT allowance to the pool, the operation proceeds using the approved WUGNOT while the sent GNOT remains permanently trapped in the position realm with no recovery mechanism. This results in particularly confusing user experience where positions in native-token pools actually use WUGNOT internally, leading users to reasonably, but incorrectly, assume that they can send GNOT for automatic wrapping while specifying the token path as handled in the pool realm (gno.land/r/demo/wugnot). In contrast, when increasing liquidity, native tokens must be sent, even though the underlying position is partly denominated in WUGNOT.

Consider implementing consistent validation across all realms that reverts with clear error messages when native tokens are detected in non-native operations, or standardizing token handling to automatically wrap any received GNOT when WUGNOT pools are targeted, preventing this category of permanent native-token fund loss throughout the protocol.

Update: Resolved in pull request #857, pull request #859 at commit 7b836c3, pull request #900 at commit 7077c67, and pull request #905 at commit 643b459.

Native Tokens Permanently Stuck in Non-Native Swaps

The router's handleNativeTokenWrapping function fails to validate or handle native tokens sent with non-native swaps, causing permanent fund loss when users accidentally include GNOT with WUGNOT transactions. When inputToken != gnot && outputToken != gnot, the function immediately returns without checking std.OriginSend for attached native tokens.

If a user mistakenly sends GNOT while specifying WUGNOT as inputToken, the native funds bypass all processing logic. They are neither wrapped for use in the swap nor returned to the sender, becoming permanently trapped in the router. The swap proceeds normally using the user's WUGNOT allowance, giving no indication that native funds were lost. This issue is exacerbated due to the incomplete documentation of the router interface, which could confuse users, front-end developers, or integrating protocols.

Consider implementing strict validation that reverts transactions when unexpected native tokens are detected with non-native swaps, or automatically returning any unexpected native tokens to the sender before proceeding with the swap.

Update: Resolved in pull request #857, pull request #859 at commit 7b836c3, pull request #900 at commit 7077c67, and pull request #905 at commit 643b459. The team stated:

This is the same PR as M-01. When input tokens are present, it performs a quantity comparison with gnot. When not handling native tokens, it restricts native token input.

Users Can Stake in Non-Incentivized Pools

The staker realm fails to properly clean up ended external incentives, allowing users to stake positions in pools that have no active reward programs. When EndExternalIncentive is called, the incentive is updated but never removed from the pool's incentives collection. This causes IsExternallyIncentivizedPool to permanently return true for any pool that ever had an external incentive, regardless of whether any incentives remain active. The poolHasIncentives validation in StakeToken therefore passes for these non-incentivized pools, allowing users to stake their LP NFTs without any possibility of earning rewards.

This is troublesome for the protocol and its users, since stakers could unknowingly lock their positions in non-earning pools based on false incentive signals. The staker realm processes these positions during every reward calculation despite zero reward potential, and the accumulation of ended incentives causes increasing computational overhead as GetAllInTimestamps must iterate through all historical incentives to filter active ones. The issue compounds over time as more incentives end but remain in the data structures, degrading performance and misleading users.

Consider removing incentive records entirely when ended, or modifying IsExternallyIncentivizedPool to check for active incentives instead of only ensuring that they exist in the collection.

Update: Resolved in pull request #852 and pull request #894. These PRs include updates to core staker logic that extend beyond the original issue. We recommend including these changes in the re-audit scope to ensure all modifications work together as intended. The team stated:

This modification moved the ended incentives to the archive so that rewards would no longer be calculated. However, PR #852 itself had an issue where it couldn't retrieve items that had ended but whose rewards had not yet been claimed. So in the follow-up commits, I refined the conditional statements to allow unclaimed rewards to be retrieved even from archived data. Commit a14f88a specifically addresses this.

Potential Permanent Lock of Rewards Due to Warmup Modification

The modifyWarmup function allows for the adjustment of warmup period durations by governance entities. However, this function currently lacks critical validation checks, particularly concerning the configuration of the final warmup tier. The protocol's design specifies that the last warmup period should have a TimeDuration set to math.MaxInt64, effectively creating an indefinite tier that ensures rewards remain accessible indefinitely.

Misconfiguration of this tier through modifyWarmup to a finite timestamp introduces the risk of reaching a NextWarmupTime that, once surpassed, could lead to the failure of the rewardPerWarmup function due to an underflow in calculateRewardForPosition, causing both calculateInternalReward and calculateExternalReward to fail. This scenario would result in a permanent DoS for the affected token, preventing stakeholders from claiming rewards or unstaking their tokens.

Consider adding validation in the modifyWarmup function to ensure that the last warmup tier always maintains math.MaxInt64 duration.

Update: Resolved in pull request #851, at commit b73cece. The team stated:

Added validation logic to the modifyWarmup function to ensure that the last warmup tier always maintains a math.MaxInt64 duration. Additionally, during an internal meeting, there was an opinion that it would be good to set the time duration that can be configured with ModifyWarmup to 1 year, so that logic has also been reflected.

Weak Ownership Transfer Model Due to Reliance on OriginCaller

The rbac realm's TransferOwnership and AcceptOwnership functions rely on std.OriginCaller for access control. This design assumes that the rbac package adequately ensures access control. However, reliance on the transaction origin is susceptible to manipulation by malicious contracts. If the owner interacts with a seemingly benign contract for unrelated functionality, that contract could silently execute ownership transfer, stealing control of the entire protocol's access control system. This vulnerability fundamentally undermines the security of the ownership transfer mechanism, potentially allowing unauthorized parties to gain control over critical functionalities or assets within the protocol.

Consider modifying the ownership-transfer mechanism to verify the immediate caller by passing it from the rbac package to the realm.

Update: Resolved in pull request #855 at commits 117d4f5, 4b7dfe0, and 4b92160. The team stated:

Package RBAC explicitly handles the owner and pending owner. It receives the previous realm from the realm area's rbac and passes it on.

Withdrawal Fees From CollectFee Get Locked in the protocol_fee Realm

The collectFee function transfers withdrawal fees to the protocol_fee realm without properly tracking them, causing these fees to become permanently inaccessible.

Unlike other protocol fee collections throughout the codebase that call AddToProtocolFee to track accumulated amounts, these transfers bypass the tracking mechanism entirely. The DistributeProtocolFee function only distributes amounts that are properly tracked in the tokenListWithAmount mapping, meaning any tokens directly transferred to the protocol fee address without registration become permanently locked with no distribution mechanism.

Consider modifying the collectFee function to call AddToProtocolFee after transferring withdrawal fees, ensuring they are properly tracked and can be distributed through the existing protocol fee distribution mechanism.

Update: Resolved in pull request #856 at commit 1aecbd9. The team stated:

When calling the CollectFee function, we have modified it to call the protocolFee ledger update function so that the protocol fee ledger can be updated.

Low Severity

Incorrect Encoding of Negative Integers in EncodeInt64

The EncodeInt64 function fails to correctly encode negative int64 values. This issue stems from the use of strconv.FormatInt(num, 10) within the function, which includes the minus sign (-) in the length count when encoding integers. Consequently, a negative integer such as -123 is encoded as 0000000000000000-123, which deviates from the expected format for correct parsing and handling of negative values.

While, in the current scope, EncodeInt64 is exclusively applied to encode timestamp values and does not encode any negative integers, the function's inability to correctly handle negative values restricts its utility and could pose challenges in future scenarios where negative integers might need to be encoded.

Consider enhancing the versatility and correctness of the EncodeInt64 function by modifying it to handle negative integers.

Update: Resolved in pull request #862 at commit e8a1732. The team stated:

The EncodeInt64 currently only accepts timestamps as input, so we modified it to cause a panic when negative values are passed in. Also, it performs type conversion to uint64 internally and the starting type is int64, safe type conversion is possible.

Restrictive Zero-Amount Validation in DrySwap

The DrySwap function contains an overly restrictive validation check that requires both result.Amount0 and result.Amount1 to be non-zero. This validation rejects the swap if either amount equals zero, which fails to account for legitimate edge cases where one token amount may round down to zero while the other remains positive. Such scenarios can occur with very small swap amounts, or extreme price ratios. The actual Swap function does not enforce this same restriction and would successfully execute these swaps, creating an inconsistency between simulation and execution. This discrepancy could cause exact-out multi-hop swaps relying on DrySwap for pre-flight checks to incorrectly reject valid transactions.

Consider modifying the validation to only reject swaps where both amounts are zero.

Update: Resolved in pull request #872 at commit 4927b24. The team stated:

The condition for intentionally setting a zero value in the DrySwap function has been relaxed.

Missing Underflow Check in updateAmounts

The updateAmounts function contains an incomplete bounds validation that only checks for positive overflow while ignoring potential negative underflow. The function correctly validates that amountCalculated.Gt(maxInt64), but fails to implement a corresponding check for the minimum boundary. During exact-in swaps, amountCalculated is decremented at each step by subtracting amountOut, causing it to move strictly downward throughout the swap execution. When a swap traverses many tick boundaries or involves very large amounts, the cumulative negative value could exceed the minimum int64 limit, resulting in an underflow that would cause transaction failure during the subsequent token transfer operations.

Consider adding a validation check to ensure that amountCalculated does not fall below the minimum int64 value, returning an appropriate underflow error.

Update: Resolved in pull request #870 at commit 73298ff. The team stated:

Added underflow check in updateAmounts function.

Inconsistent Return Value in handleExactIn and handleExactOut

The handleExactIn and handleExactOut helper functions in the swap_math library return semantically inconsistent values in their second return position, creating a confusing interface that undermines code maintainability. In handleExactIn, when the target price is reached, the function returns the actual amountIn needed for the swap, but when the target is not reached, it returns amountRemainingLessFee which represents the fee-adjusted input to the price calculation rather than the actual amount consumed.

Similarly, handleExactOut returns amountOut when reaching the target but returns amountRemainingAbs otherwise. This breaks the implicit promise that the second return value should consistently represent the actual amount consumed or produced in the step. While the current implementation works because the calling code correctly overwrites these provisional values with properly calculated amounts, this semantic inconsistency creates a maintenance hazard where future developers may misinterpret the return values and introduce subtle bugs.

Consider refactoring the aforementioned functions to consistently return either the actual amount or a provisional value and clearly documenting the return value semantics to prevent future confusion.

Update: Resolved in pull request #877 at commit 9124cdd. The team stated:

Updated the comment and changed the response values ​​to provisional amount.

SetRoleAddresses Replaces the Entire Role Map

The SetRoleAddresses function of the access realm replaces the entire role address mapping instead of updating individual entries, creating a severe security risk if the RBAC validation can be bypassed. The function performs a wholesale replacement, allowing a single malicious call to overwrite all system-critical roles including admin, governance, pool, and router addresses.

While the function is protected by assertIsRBAC(caller), if this check can be circumvented, for example due to vulnerabilities in DerivePkg, an attacker will gain the ability to hijack the entire protocol's access-control system in a single transaction. This design violates the principle of least privilege by granting unnecessarily broad modification powers when most legitimate operations only need to update individual role addresses.

Consider refactoring the logic to provide granular role-update functions that modify individual addresses while also implementing additional validation that prevents modification of immutable system roles.

Update: Resolved in pull request #882 at commits fc38fd1 and 7955e1e. The team stated:

Modified to perform individual updates instead of bulk updating addresses

Exact-Out Swap Routes Allow for More Output Amount Than Specified

The ExactOutSwapRoute function violates the semantic contract of exact-output swaps by potentially delivering more tokens than the user had specified. In exact-out swaps, users specify precisely how many output tokens they want to receive and provide a maximum input amount they are willing to spend. However, while the swap execution logic calculates the required input based on pool liquidity and price impact, it does not enforce the condition that the output remains exactly as specified.

If a vulnerability exists in the underlying AMM math logic that returns more tokens than what the user had specified, the router will not reject them in the exactOutAmount validation function, potentially leading to a loss for the pool. The function only checks that the output is, at most, one less than the amount specified, but no upper bound is enforced, allowing excess tokens to pass through. Tolerance due to rounding from both directions breaks the fundamental promise of exact-output swaps and can cause issues for integrating protocols that rely on precise output amounts for accounting or automated strategies.

Consider enforcing strict validation of exact-output swaps in the router, identifying and rejecting swaps that do not yield to the correct exact output. If any exists, it would signify underlying issues with the pool implementation.

Update: Resolved in pull request #878 at commit 3dad6c2. The team stated:

Changed to allow a rounding error of up to 1 per swap.

Unnecessary Approvals in Single-Hop Swaps

The swapInner function performs a token approval granting the pool permission to spend tokens on behalf of the router. This is essential for multi-hop swaps, where the router acts as an intermediary holding tokens between pools. However, for single-hop swaps, the tokens flow directly from the user to the pool without the router taking custody, making this approval redundant. This not only increases the gas costs for single-hop swaps but also leaves residual approvals that could allow the pool to spend router funds if a vulnerability exists in the pool realm.

Consider implementing separate execution paths for single-hop swaps that bypass the approval step.

Update: Resolved in pull request #834 at commit 0e9c3e6. The team stated:

When swap, we changed it so that Approve is called only when necessary.

Multi-Hop, Native-Token, Round-Trip Swaps Fail

The router's handleNativeTokenWrapping function contains a logical flaw that prevents multi-hop swaps where both input and output tokens are native GNOT. When the function detects that the outputToken == gnot, it sets the withUnwrap flag and immediately returns, bypassing the subsequent logic that would wrap the input GNOT to WUGNOT.

This causes the multi-hop swap to fail because the first pool expects to receive WUGNOT, but since the native tokens never got wrapped, it results in insufficient allowance errors when the pool attempts to transfer tokens from the user. Even if users have pre-existing WUGNOT allowances, the swap would still fail later in HandleInputSwap due to balance mismatches. This limitation blocks legitimate use cases such as arbitrage opportunities where traders need to start and end with native currency while routing through intermediate tokens.

Consider refactoring the native-token handling logic to check both input and output tokens independently, ensuring that input GNOT is wrapped regardless of the output token type.

Update: Resolved at commit in pull request #873 at commit 3f1072d. The team stated:

When handling with gnot in a swap route, both the input token and the output token are checked.

Unexpected Unwrapping of WUGNOT in Multi-Hop Swaps

The router incorrectly unwraps WUGNOT to native GNOT in multi-hop swaps where users explicitly request WUGNOT as the output token. When executing exact-in or exact-out swaps where the input is the native token and the output is its wrapped representation, the handleNativeTokenWrapping function correctly wraps the input GNOT to WUGNOT for the initial swap, but the router's output-handling logic fails to distinguish between cases where the user wants native GNOT instead of wrapped WUGNOT as the final output.

The finalizeSwap function calls HandleInputSwap if the input token is GNOT to handle the unwrapping of any leftover input funds, which unwraps the user's received WUGNOT back to native GNOT, solely based on its new balance, regardless of whether the new balance is partly due to wrapped tokens received from the swap. This unwrapping is likely to fail, preventing such swaps due to insufficient allowance from the user to the router and is particularly problematic for smart contract integrations that expect to receive WUGNOT for composability with other protocols.

Consider distinguishing between the leftover wrapped tokens that should be unwrapped and the WUGNOT tokens that the user received as part of the swap operation, preserving the user's explicit output token choice.

Update: Resolved in pull request #834. All tokens now flow from the pool to the router first, and since HandleInputSwap is called before tokens are sent to the user, only the expected WUGNOT amount is unwrapped.

Outdated Token Amounts Returned in Staker

The StakeToken and UnStakeToken functions return stale token amounts that do not reflect the current composition of liquidity positions. Both functions call getTokenPairBalanceFromPosition, which retrieves the stored Token0Balance and Token1Balance values from the position struct maintained in the position realm.

These balance fields are only updated during liquidity modifications like minting or burning, but remain static as market prices change. Since concentrated liquidity positions experience continuous rebalancing as the price moves through their range, the actual token composition can differ significantly from these cached values. This discrepancy could mislead integrating contracts and users who rely on these return values for decision-making.

Consider either calculating real-time token amounts using the current pool price and the position's liquidity, or removing the returned token amounts if they are not intended to be used.

Update: Resolved in pull request #865 at commit 21af34b. The team stated:

The problematic data has been confirmed as definitely not being used, so it has been removed. Additionally, unnecessary functions and parameters have also been removed.

Unexpected Unwrapping of WUGNOT Rewards

The EndExternalIncentive function incorrectly unwraps WUGNOT refunds to native GNOT, violating user expectations about token consistency. When users create external incentives with WUGNOT as the reward token by sending wrapped tokens directly, they expect to receive any refunds in the same WUGNOT format. However, the function checks if incentive.rewardToken == WUGNOT_PATH and calls unwrapWithTransfer to convert the refund to native GNOT.

This creates an asymmetric token flow where users deposit WUGNOT but receive native GNOT back, breaking the principle that refunds should match the original token type. The forced unwrapping is particularly problematic for smart contracts or protocols that create incentives programmatically, as they may not be equipped to handle native-token receipts and expect to maintain WUGNOT throughout their operations for composability with other protocols.

Consider modifying the refund logic to preserve the original token format, and only unwrapping when the user initially sent native GNOT that was wrapped by the contract, while maintaining the WUGNOT format when WUGNOT was originally deposited.

Update: Resolved in pull request #866 at commits 32c5f16 and 025087f. The team stated:

The EndExternalIncentive function was incorrectly handling WUGNOT refunds, causing refunds to be issued in a different token type than the originally deposited tokens.

The following modifications were made:

1. Added isRequestUnwrap flag: Added a new field to the ExternalIncentive struct to track the original deposit type
2. Modified refund logic:
   • GNOT deposit --> GNOT refund
   • WUGNOT deposit --> WUGNOT refund

Tests were also added to verify that users receive refunds in the same token type (GNOT or WUGNOT) as they originally deposited, ensuring token type consistency is maintained.

Fees Can Be Set Too High

The protocol lacks upper-bound validation on critical fee parameters, allowing governance or admin to set unreasonably high fees and effectively confiscate user funds. The router swap fee mechanism allows fee percentages to be configured up to 100%, enabling complete appropriation of swap outputs. Similarly, the staker realm's unstaking fee can be set to consume all staking rewards, while the pool's withdrawal fee applied during decreasing liquidity or collecting rewards can capture 100% of withdrawn liquidity and accumulated fees. While these parameters are admin- and governance-controlled, the maximum limit of 100% creates significant centralization risk where a compromised admin key or malicious governance proposal could drain all user funds under the guise of fee collection.

Consider reducing immutable maximum fee constants to reasonable amounts, ensuring that users retain the majority of their funds even in worst-case scenarios.

Update: Resolved in pull request #864 at commits 75b7757, 441a6ff, 7a8a95d, and b954e47. The team stated:

We have established a policy to set the upper limit for configurable fees at 10%.

Inconsistency in Position-Burn and Liquidity-Increase Logic

The protocol exhibits a design choice where burning a position clears its liquidity, tokensOwed0, tokensOwed1, and sets position.burned to true, but does not burn the associated GNFT. This functionality is complemented by the reposition feature, which allows for the "re-activation" of a position within the same or a different tick range. However, a significant inconsistency arises with the IncreaseLiquidity function, which can still be called to increase the liquidity of burned positions, despite the documented prerequisite that positions should have liquidity.

This oversight creates a logical inconsistency in the position's lifecycle. While the reposition function is the intended mechanism for reactivating a burned position, the IncreaseLiquidity function provides an unintended backdoor to modify it. This could lead to user confusion and states where a "burned" position holds liquidity without being formally repositioned, undermining the integrity of the protocol's state management.

Consider adding validation in the IncreaseLiquidity function to enforce the condition that the position is not burned.

Update: Resolved in pull request #874 at commit 1f27b58. The team stated:

When increasing liquidity, we intended to activate closed positions as well. A miswritten comment has been corrected.

Precision Loss in rewardPerSecond Calculation Leading to Dust Rewards

The external incentive mechanism in the staker realm suffers from precision loss that permanently locks residual reward tokens. When CreateExternalIncentive is called, the reward distribution rate is calculated by dividing the total rewardAmount by the incentive duration through integer division to determine rewardPerSecond. This integer division truncates the decimal part, rounding down, leaving a small amount of residual of tokens that are neither distributed to incentivized positions nor tracked to be returned to the external incentive creator.

Consider implementing a mechanism to refund dust to the external incentive creator.

Update: Resolved in pull request #852 and pull request #894. These PRs include updates to core staker logic that extend beyond the original issue. We recommend including these changes in the re-audit scope to ensure all modifications work together as intended. The team stated:

Previously, we calculated and processed the reward distribution amount using integer division, but now we have modified it to calculate and process the exact reward distribution amount. In the first fix, we handled it by calculating the dust amount and adding it to the rewards, but there was still a slight precision loss, so we implemented the current method that requires more computation but can obtain accurate values.

Incorrect Parameter Passed to cacheReward in StakeToken

The StakeToken function inaccurately passes currentTime as the currentHeight parameter to the cacheReward function. This misalignment between the expected block height and the provided timestamp value leads to the improper setting of lastRewardCacheHeight within the cacheReward function. The documentation of cacheReward suggests that it updates the reward cache based on the block height difference. However, the implementation does not align with this description, as the reward calculations rely solely on timestamps, rendering the height parameter unused.

Consider updating the StakeToken function so that it properly passes the currentHeight parameter, and updating the docstrings of cacheReward to correctly describe the function's reliance on timestamps instead of block heights.

Update: Resolved in pull request #862 at commit a692963.

Unstaking Fee Is Applied When Collecting Rewards

The staker realm applies unstaking fees when users collect rewards from their staked positions, even though the GNFT remains staked. In the CollectReward function, when processing internal rewards, the code calls handleUnStakingFee with the internal reward amount, which applies a 1% fee despite no unstaking occurring. The same inappropriate fee application happens for external rewards. This misnamed and misapplied fee reduces users' legitimate reward earnings by 1% on every collection, effectively acting as a tax on staking rewards rather than an unstaking fee on the position's principal.

Consider renaming this unstaking fee to a staking reward fee if the intention is to charge on reward distributions instead of unstaking.

Update: Resolved in pull request #862 at commit 38b226b.

Unchecked Success Boolean in TryRegister May Lead to Inconsistent Referrals

The Mint function takes a referrer address that is attempting to register a referral. However, the success boolean returned by the TryRegister function is not checked or handled, diverging from the standard practice observed across the rest of the codebase. When a referral registration fails, the function proceeds silently without applying the necessary fallback logic, potentially causing inconsistent referral states.

Consider validating the success boolean returned by the TryRegister function and applying the protocol's standard fallback logic to ensure consistent behavior and prevent silent failures.

Update: Resolved in pull request #875 at commit 96c07ab. The team stated:

We have unified the pattern of using the TryRegister function of the referrer within the position Mint function.

Unchecked Type Assertions

The staker realm contains multiple instances of unchecked type assertions that could cause runtime panics when invalid data is encountered. In the CurrentCount function, the code performs a direct type assertion (value.(uint64)) without checking if the conversion is valid, which will panic if the stored value is not actually a uint64. Similar unsafe patterns exist in the IterateAll function of the Pools type where values are asserted to *Pool without validation, and also in the calculateUnclaimableReward function.

Consider implementing safe type assertions using the two-value form (value, ok := v.(uint64)) and handling the failure case appropriately.

Update: Resolved in pull request #862 at commit c1e1dc4. The team stated:

Modified to explicitly handle errors that occur during type assertions.

Misleading Documentation

Throughout the codebase, multiple instances of inaccurate, incomplete, or misleading documentation were identified:

• In the router documentation, the route paths are described to be of the format POOL_PATH,TOKEN0,TOKEN1,FEE:NEXT_POOL..., while in the ExactInSwapRoute and the ExactOutSwapRoute route paths are described to be of the format TOKEN0:TOKEN1:FEE,TOKEN1:TOKEN2:FEE,.... However, the correct format is TOKEN0:TOKEN1:FEE*POOL*TOKEN1:TOKEN2:FEE*POOL*....
• In the router documentation, the ExactInSwapRoute and the ExactOutSwapRoute functions mention that swap routes of a maximum of 7 hops are supported. However, the implementation restricts the maximum hops to 3.
• The liquidity field of the Pool struct is referred to as the total liquidity instead of being referred to as the total active liquidity.
• The setPosition function returns true if the position existed before and has now been updated. This is in contrast to the comment.
• The safeMulInt64 and the safeAddInt64 functions document a panic on overflow, whereas safeSubInt64 documents a panic on underflow. However, all three functions panic on both overflows and underflows.
• The documentation for the update function is incorrect.

Incomplete or inaccurate documentation increases the learning curve for developers and auditors, raises the likelihood of misinterpretation, and makes long-term maintenance more difficult.

Consider reviewing and updating both in-code documentation and README files to ensure that function behavior, parameters, return values, and design decisions are accurately and consistently described, while keeping project-level documentation aligned with the current implementation.

Update: Resolved in pull request #878 at commit 16fa069. The team stated:

Corrected incorrectly written comments and documentation.

Redundant Code and Opaque Patterns

Throughout the codebase, multiple instances of redundant code and opaque patterns were identified:

• There is redundant logic in the RewardSpent and the RewardLeft functions of the staker realm.
• There is redundant logic in the processForwardSwap function of the router realm.
• There is redundant logic in the cacheInternalReward function, since it always calls cacheReward first.
• The safeTransfer function called from the swapTransfers function of the pool realm expects a negative amount, which is unintuitive.
• There are two implementations of the liquidityMathAddDelta [1] [2] function.
• The GetOrCreate function is designed as a getter but can also mutate state by creating a new pool if the requested pool path does not exist.
• The decreaseLiquidity function calls collectFee, accumulating and distributing all accrued fee rewards, and then proceeds to recalculate them.
• The calcPositionReward function contains variable shadowing where the inner loop variable shadows the outer loop variable with the same name but different type.

Consider removing unused elements and consolidating duplicate logic to simplify the codebase, reduce potential maintenance issues, and improve the overall clarity.

Update: Resolved in pull request #880.

Missing or Weak Validation

Throughout the codebase, multiple instances of functions that lack additional input validation or defensive checks were identified. While these omissions do not currently lead to incorrect behavior, they may result in reduced processing efficiency or unnecessary computations, and increased difficulty in debugging:

• The TickMathGetTickAtSqrtRatio and TickMathGetSqrtRatioAtTick functions lack nil-pointer checks.
• The router realm does not verify whether the pools exist before attempting swaps.
• The swapInner function does not validate that the amounts returned from the swap are non-zero.
• The UpdateRoleAddress function does not ensure the address is valid.
• The rewardPerWarmup function does not return early when startTime equals the endTime. Such a scenario could occur when calculating rewards for collection at the same timestamp as an emission rate change, leading to unnecessary computation.
• The modifyPosition function does not validate whether liqDelta is 0, such as when called via collectFee. This results in unnecessary computation and gas usage.
• Several operations in the codebase check or modify state but do not verify the returned values to ensure that the intended state change actually succeeded:
  • Pools.tree.Set operations in the staker realm
  • Pool.ticks.Set operations in the pool realm
  • Pool.tickBitmaps.Set operations in the pool realm
  • Pool.rewardCache.set operations in the staker realm
  • Callers of the function setPosition in the pool realm
  • Callers of the function setPosition in the position realm

Consider adding lightweight validation and defensive checks to ensure that the assumptions about parameters and state are explicit. Doing so would help improve the processing efficiency while making debugging and maintenance easier.

Update: Partially resolved in pull request #881 at commits dc5bb67, 4e7f075, f62be5e, 80b9148, and a2a1132. The team stated:

We've applied the content from the report as a priority. The last item in the report was a recommendation to check the boolean value for the avl.Set method. However, this field indicates whether new data was added, not whether the update succeeded or failed. In practice, when we added logic to check this field, problems occurred such as the data structure not being created or not being initialized. Therefore, we did not implement this item.

Unchecked Errors Across Codebase

Numerous function calls throughout the codebase ignore returned errors. This practice can lead to silent failures, where an operation fails without any warning or interruption, causing the system to continue running in an unpredictable or incorrect state. This not only masks the underlying bugs and complicates debugging, but it also poses a significant risk to the protocol's stability, potentially leading to incorrect state transitions or data corruption.

• The Approve call in the swapInner function is not being checked against approval error.
• The multiSwap function calls the processForwardSwap function but ignores the returned error. Similarly, multiSwapNegative calls the processBackwardSwap function but ignores the returned error. Although none are currently returned, errors from failed approvals in the swapInner function should be propagated upwards.
• The initRbac function calls RegisterRole and UpdateRoleAddress without checking errors in the registration of roles and their respective addresses.
• The decreaseLiquidity function calls access.GetAddress but ignores the returned error in the retrieval of the relevant role. This pattern is consistent across many of the access.GetAddress calls.

Consider reviewing all instances where errors could occur but are not currently checked. For each of these instances, consider implementing appropriate error handling and panics accordingly.

Update: Resolved in pull request #868 at commit 80a3391, pull request #854 at commit f0502a1, and pull request #844 at commit d880633. The team stated:

We are continuously adding error checks to areas where they were missing, and we have fixed the parts you suggested in the report.

Misleading Errors Returned

The updateAmounts function contains a confusing error-handling issue whereby an overflow condition incorrectly returns an underflow error. The incorrect error message significantly hampers debugging efforts, as developers encountering this error would investigate underflow scenarios when the actual issue is an overflow condition. Also, the error message in tickCross incorrectly references the fee-growth global values for token0 instead of token1.

Consider updating the errors returned to accurately reflect the erroneous conditions, thereby maintaining consistency and improving maintainability.

Update: Resolved in pull request #862 at commit 067755c. Additionally, the erroneous underflow errors in tickCross have been removed in pull request #825. Note that while these errors were addressed, the underlying pool tick implementation where these changes occur is out of scope for this audit. The team stated:

Returns an appropriate error within the updateAmounts function.

MintAndStake Referrer Loss

The MintAndStake function mishandles the referral parameter causing the staking referral to be lost. The function correctly passes the referrer value to the Mint function, ensuring referral attribution for the minting portion of the operation. However, when calling StakeToken, the function passes an empty string instead of the provided referrer.

Consider passing the referrer value to the StakeToken function.

Update: Resolved in pull request #862 at commit 2e1722a. The team stated:

We modified the MintAndStake function to pass the referral value received as an argument to the StakeToken function. This fix prevents the situation where referral information was being lost by passing an empty string previously.

Uncaught Overflows and Underflows, Unsafe Casting, and Truncated Values

The codebase exhibits multiple instances where arithmetic operations are vulnerable to overflow and underflow conditions. There are also cases of potentially unsafe casting and value truncation, which may cause unexpected results or incorrect values to be processed. While not necessarily critical, these issues could affect the reliability of calculations and should be reviewed to ensure safe handling and consistent system behavior:

• There is a potential overflow in the Burn function when adding the amounts from the burned liquidity to tokens owed.
• There is a potential overflow in the Collect function when subtracting amount0 from the tokens owed, and from the pool balances.
• There is an instance of unsafe casting of amounts from uint256 to uint128 which could truncate values.
• There is a potential overflow in the positionUpdate function when accumulating owed tokens for the position.
• There is a potential overflow in the updateFeeProtocol function when accumulating protocol fees.
• There is a potential overflow in the calculateFees function when adding fees to the tokens owed.
• There is a potential overflow in the increaseLiquidity function when updating the position's liquidity and token balances.
• There is a potential overflow in the processRoutes function when accumulating the amounts in and out of the swap.
• There is a potential underflow in the handleSwapFee function as it returns 0 instead of reverting.
• There is a potential overflow in the HandleInputSwap function when adding the balance before with the wrapped amount.

The above issues were identified by the GnoSwap team during the audit and were addressed in pull request #825. However, additional instances of uncaught overflows and underflows were also identified:

• There is a potential overflow in the Collect function when subtracting amount1 from the tokens owed, and from the pool balances.
• The Mul functions of the Uint and Int objects do not handle overflows and underflows.
• The lsh functions of the Uint and Int objects do not handle overflows and underflows.
• There is a potential underflow in the updateAmounts function when calculating the amountSpecifiedRemaining [1] [2].
• There are potential underflows in the increaseLiquidity function when calculating the fee growth [1] [2].
• There is a potential overflow in the updateTokensOwed function when accumulating the total tokens owed.
• Potential underflow in the calculateTokensOwed function when calculating tokens owed to the position.
• The abs function overflows on input -2147483648, although such value is impossible due to the assertValidTickRange validation.

There are several additional instances where underflows or overflows could theoretically occur. While these do not currently present a critical vulnerability, it is important as a defensive security measure to detect and handle them to prevent unexpected state corruption.

Consider ensuring that all arithmetic operations are safe from underflows and overflows, that values are not inadvertently truncated, and that type conversions are performed safely.

Update: Partially resolved in pull request #825, pull request #885 at commits ce1089e, 9bbb00c, and 79fc52c. The Mul and lsh functions of the Uint and Int objects, and the abs function have not be resolved. The team stated:

We've also prioritized fixing the parts suggested in the report. I'm currently reviewing the codebase with a focus on the mul and add functions, and those parts will be updated later as well. Regarding your suggestions, there was a recommendation to check internally for functions that could cause overflow, such as the Mul function in the int256 and uint256 packages. However, after internal review, I decided not to modify those packages because I believe the side effects of directly modifying the internals of those functions would be significant. For the abs function, I didn't add separate overflow checks since the range is already being checked before the function is called. Finally, I excluded functions that use modular operations from the modifications, as I determined that they don't need separate overflow handling.

Notes & Additional Information

Unaddressed TODO Comments

The codebase contains a TODO comment indicating unfinished work. Such comments do not provide clear traceability and may be forgotten or overlooked. In particular, there is a TODO comment in the calculateTokensOwed function.

Consider removing all TODO comments and tracking unfinished tasks in the project’s issues backlog. Alternatively, consider linking each inline TODO to its corresponding issue to ensure visibility and maintainability.

Update: Resolved in pull request #887 at commits d383252, d92e804, and e4e944c. The team stated:

We removed all TODO comments.

The code was modified in the following two parts: 1. q128: This was a comment written for the purpose of storing repeated string conversions as constants and reusing them, and I have implemented this modification. 2. In the newApiStake function, unified the calculation for obtaining stakeDuration to be time-based.

Native-Token Handling Complexity and Documentation Gaps

The protocol requires users to approve WUGNOT spending when performing operations with native GNOT tokens, but this requirement is undocumented and could causes transaction failures. When users send native GNOT for swaps or position operations, the contracts internally wrap it to WUGNOT for processing. If excess GNOT needs to be refunded, the unwrapWithTransferFrom function in the router attempts to transfer WUGNOT from the user before unwrapping it back to native tokens. Similarly, the position contract's unwrap function requires WUGNOT transfer approval for refund operations.

In addition, the protocol creates interface confusion by requiring users to specify different token identifiers for the same native token depending on the context. When swapping GNOT, users must specify the inputToken as (gnot) to trigger native token handling, but routes must include the wrapped version (gno.land/r/demo/wugnot) instead, because pools only operate on WUGNOT tokens. This dual-identifier system lacks validation and documentation, leading to failed transactions when users intuitively specify gnot in routes.

The lack of documentation around both the WUGNOT approval requirement and the route specification format could lead to confusion amongst frontend developers and integrating protocols, leading to poor developer or user experience.

Consider clearly documenting these requirements, including the mandatory WUGNOT approval for refund operations and the correct route format specification.

Update: Resolved in pull request #886 at commit d4bdc55. The team stated:

We have updated the Readme with information regarding the Router's GNOT processing, use cases, refunds, and more.

Recoverable Pool-Initialization Griefing Attack

The CreatePool function allows pool creators to set an arbitrary initial sqrtPriceX96 value without any validation of price reasonableness, enabling a low-cost griefing attack against the protocol. An attacker can front-run legitimate pool creation by initializing pools for all fee tiers of important token pairs at extreme prices, such as setting 1 GNO equal to 0.000001 USDC when the market price is significantly higher.

Once a pool is created with an incorrect price through the newPool function, it effectively becomes unusable because no rational liquidity provider will deposit funds at these distorted prices due to immediate arbitrage losses, and the price cannot naturally correct through trading since swaps require existing liquidity to execute against, creating a deadlock situation where the pool exists but cannot function. While the 100 GNS creation fee provides some deterrent, it may be insufficient against motivated attackers targeting high-value pairs.

In such scenarios, the pools can be restored permissionlessly using an atomic transaction that adds wide-range liquidity, swaps to correct the price, then removes liquidity. Since the executor will both be the liquidity provider (losing value due to slippage) and the arbitrageur (gaining value from the swap towards the correct market price), the losses largely cancel out, minus gas and protocol fees.

Consider documenting the above mechanism to restore pools in case of griefing attacks. Alternatively, consider implementing price-oracle validation during pool creation to ensure that initial prices fall within reasonable bounds of market rates.

Update: Resolved in pull request #879 at commit caf2c98. The team stated:

Added documentation on how users can restore normal pricing through position mint and swaps when malicious values are set with CreatePool.

Missing User-Specified Price Limit Support in Router Interface

The router interface lacks the ability for users to specify custom price limits during swaps, reducing control for advanced integration patterns and MEV protection strategies. The router consistently calls swapInner with sqrtPriceLimitX96 hardcoded to zero, which causes calculateSqrtPriceLimitF2orSwap to automatically set extreme values - MIN_SQRT_RATIO + 1 for zeroForOne swaps or MAX_SQRT_RATIO - 1 otherwise.

While the router provides slippage protection through amountOutMin and amountInMax parameters that validate final swap outputs, it prevents users from leveraging the pool's native price limit functionality that enables controlled partial execution within acceptable price boundaries.

This limitation impacts MEV protection strategies where protocols need to accept partial fills rather than allowing price to move beyond specified thresholds, as the current implementation forces all-or-nothing execution. Advanced trading strategies including arbitrage bots and AMMs lose the ability to specify acceptable price ranges for partial execution, forcing them to choose between risking excessive price impact or setting conservative slippage limits that may prevent beneficial trades. The absence of price-bounded partial fills also reduces capital efficiency, as protocols cannot optimize execution by accepting available liquidity within favorable price ranges when full amounts are not available at the desired prices.

Protocols implementing sophisticated order execution strategies must rely solely on binary pass/fail slippage validation rather than leveraging the underlying pool's graduated price limit mechanism that allows optimal partial fills. However, exposing price limits would require modifying the router's exact-out validation logic, as the current exactOutAmount check would reject legitimate under-delivery caused by price limit termination.

Consider implementing price-limit-aware validation alongside optional sqrtPriceLimitX96 parameters, allowing sophisticated integrators to specify custom price boundaries for partial execution while maintaining current zero-default behavior for standard users.

Update: Resolved through architectural redesign. Acknowledged and reimplemented via the new callback mechanism that allows custom price limit specification. The new architectural pattern was a primary focus of the extended audit, where additional vulnerabilities were identified. Please refer to the GnoSwap extended audit report for a complete assessment of the current implementation.

Liquidity Operations Might Revert for Large Amounts

The protocol's reliance on int64 for token transfers creates a limitation when handling large liquidity positions. While the DecreaseLiquidity function and internal calculations use uint256 for precision, the underlying GRC-20 token standard restricts transfer amounts to int64. Liquidity positions can exceed this limit through multiple liquidity increases that each add up to 63 bits of liquidity, or through swap-induced composition shifts that concentrate value in one token. When users attempt to withdraw such positions, the Collect function calls safeConvertToInt64 for the approval amount, which panics when the value exceeds int64 maximum.

This issue particularly affects tokens with large supplies or many decimals, and becomes more likely as positions accumulate fees over time. The same vulnerability exists in the CollectProtocol function, where accumulated protocol fees might exceed transfer limits. While users can work around this by calling decrease liquidity with smaller amounts multiple times, this creates poor user experience, increases gas costs, and could temporarily prevent users from accessing their funds if they are unaware of the limitation.

Consider batching large withdrawals into multiple int64-compliant transfers, or extending the GRC-20 standard to use uint256 for balances, allowances, and transfers to safely accommodate high-decimal or high-supply tokens in the AMM.

Update: Acknowledged, will resolve. The team stated:

We are aware that we are using the int64 data type designed in the Gno token system, which creates certain limitations. It seems difficult to resolve immediately, and we plan to share this issue with the Gno team to work on a solution.

Router-Pool Integration Limitation Restricts Composability

The pool's assertPayerIsPreviousRealmOrOriginCaller validation in the Swap function restricts the payer to either std.PreviousRealm (the immediate caller, typically the canonical router) or std.OriginCaller (the end user). While this prevents unauthorized fund transfers, it blocks legitimate integration scenarios where wrapper contracts, aggregators, or vault strategies need to perform intermediate swaps.

In a flow where a third contract attempts to use the canonical router, and attempts to perform a swap as the payer, the assertPayerIsPreviousRealmOrOriginCaller validation fails because the payer is neither the router nor the user. This forces wrappers to bypass the router and obtain direct pool whitelisting by governance or the admin, restricting themselves to direct pool calls without router features such as multi-hop routing or native-token handling. The result is duplicated logic and additional governance overhead to maintain integrator whitelists.

This differs from industry standards like Uniswap v3’s callback model, where pools send output tokens first and then require input via a callback, enabling any contract to integrate seamlessly with router infrastructure. Without such flexibility, GnoSwap integrations face reduced composability, blocking advanced aggregator optimizations, yield strategies, and multi-contract compositions.

Consider revising the architecture to support router-mediated calls from legitimate wrappers while maintaining security.

Update: Resolved through architectural redesign. Acknowledged and reimplemented via the new callback mechanism that enables flexible integration patterns. The callback implementation was a primary focus of the extended audit, where additional vulnerabilities were identified. Please refer to the GnoSwap extended audit report for a complete assessment of the current implementation.

Users Cannot Increase Liquidity With Wrapped Tokens

The IncreaseLiquidity function contains a design flaw that prevents users from utilizing existing WUGNOT tokens when adding liquidity to positions. When the function detects that a position contains WUGNOT through isWrappedToken checks, it automatically calls safeWrapNativeToken which expects to receive native GNOT tokens for wrapping. This assumption forces users who already hold WUGNOT to first unwrap their tokens to native GNOT, only to have them immediately re-wrapped by the function, incurring unnecessary gas costs and adding friction to the user experience.

Consider implementing dual-path logic that detects whether the user is sending native GNOT or WUGNOT tokens, and handles each case appropriately without forcing unnecessary wrapping operations.

Update: Resolved in pull request #857, pull request #859 at commit 7b836c3, pull request #900 at commit 7077c67, and pull request #905 at commit 643b459.

Token Ordering Inconsistency Between Router and Pool Interfaces

The router exhibits a fundamental token-ordering inconsistency that creates confusion between swap-direction-based routing and pool-canonical alphabetical ordering, manifesting as both an interface-design issue and a user-experience problem. The architecture employs two distinct token-ordering paradigms that operate independently: the pool layer uses strict alphabetical ordering for canonical pool identification where tokens are always arranged lexicographically, while the router layer uses swap-direction ordering for route specification where tokens are positioned based on the intended swap flow (tokenIn:tokenOut:fee).

This dual-paradigm approach creates confusion because the router's getDataForSinglePath function parses routes by treating the first token as tokenIn and the second as tokenOut, directly mapping these to swap parameters without any validation of the intended swap direction, yet the documentation and user expectations often assume alphabetical ordering similar to pool identifiers.

When users specify routes like "GNS:USDC:3000", the router interprets this as a GNS → USDC swap regardless of alphabetical ordering, but users familiar with pool conventions might expect routes to follow the same alphabetical format, potentially leading them to specify "GNS:USDC:3000" when they actually intend a USDC → GNS swap if they are thinking in pool terms. The router then passes these tokens directly to the pool's Swap function as tokenIn and tokenOut parameters, relying entirely on the pool's mustGetPoolBy function to handle token reordering and pool existence validation.

While this delegation approach functionally works because the pool layer correctly handles token normalization and the swap direction is determined by the zeroForOne boolean calculated from lexicographic comparison, it creates an architectural disconnect where the router does not validate route format consistency or enforce any relationship between user-specified inputToken and outputToken parameters and the route string format.

Consider enhancing the documentation to explicitly distinguish between route format (tokenIn:tokenOut:fee following swap direction) and pool format (token0:token1:fee following alphabetical ordering), along with optional route validation which verifies that the route string matches the user-specified input and output tokens.

Update: Resolved in pull request #878 at commit 16fa069. The team has successfully addressed the core issue by enhancing documentation and comments to clearly distinguish between route format and pool format. Additionally, the team implemented on-chain validation for routes to prevent inconsistencies. However, this validation logic is out of scope for this audit and has not been reviewed. The team stated:

The changes for this issue are as follows: - Added comments and documentation about the format and rules for router and pool path mapping - Added validation functions for router and pool paths

Unutilized Artifacts

Throughout the codebase, multiple instances of unused variables, function parameters and duplicate or unnecessary struct fields were identified. These can cause confusion, reduce readability, and may indicate incomplete or legacy logic that was not fully removed:

• The _q96 and q192 variables are unused.
• The recipient address in the Mint function is unused.
• The positionId and poolPath parameters in the handleUnStakingFee function are unused.
• The stakeTimestamp and stakeTime are both set to the current time during the deposit creation upon staking.
• The nonce field of the Position struct is set to zero during minting but not otherwise utilized.

Consider removing all unused variables, parameters, and unutilized struct fields to improve code clarity and maintainability. If any are intended for future use, add explicit comments to document their purpose.

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

We cleaned up unused variables and fields. For stakeTimestamp and stakeTime, I consolidated them into stakeTime, and removed all the others.

Client Reported

Division By Zero in Incentive Reward Tier Calculation

The reward calculation system in the staker realm contains multiple division-by-zero vulnerabilities that can cause the protocol to panic when tier counts become zero. The CurrentReward function calculates per-pool rewards by dividing the emission amount by self.CurrentCount(tier) without checking if the count is zero. This vulnerability also appears in the CurrentRewardPerPool function, where the reward is calculated by dividing by counts[tier].

Consider implementing validations which ensure that any potential divisions by zero during reward calculations are caught and handled appropriately.

Update: Resolved in pull request #825 at commit 5f4df93. The team stated:

We've added zero check for this case.

Conclusion

GnoSwap represents an ambitious implementation of concentrated liquidity market making on the emerging Gno blockchain network, incorporating established Uniswap V3 conventions while adding comprehensive staking and incentive mechanisms. The protocol demonstrates significant improvements compared to its initial assessment, with noteworthy enhancements in its overall security posture. However, the present audit identified numerous critical- and high-severity issues among dozens of distinct security vulnerabilities, revealing an extensive attack surface throughout the protocol.

The audit uncovered significant vulnerabilities across multiple components, including critical issues in fund handling, position management, and access control mechanisms. These findings, in the context of the Gno infrastructure’s active development and refinement, presented security risks that required comprehensive remediation.

The fix review confirmed that most vulnerabilities were resolved through major development efforts. However, the scope of changes introduced further security considerations and complexity. The remediation process revealed additional issues, involving substantial architectural modifications, including a new callback mechanism for fund transfers, and extending into important out-of-scope components. With one critical upgradeability issue and two high-severity vulnerabilities remaining unresolved, alongside other findings requiring caveats, a further round of review is necessary before production deployment.

While the code quality has improved from previous iterations, persistent issues such as unnecessary type conversions, string-based integer operations, unclear encapsulation boundaries, and computational inefficiencies indicate areas for continued refinement. The implementation would benefit from comprehensive unit, integration, and fuzz testing to validate the extensive changes made during remediation.

Furthermore, several critical components — particularly those related to fund transfers, native token handling, and tick state management — were outside this audit’s scope and should undergo a dedicated review in a subsequent engagement. These modules, along with the underlying Gno infrastructure, represent potential risk vectors that merit focused security assessment as the ecosystem matures.

Given the extensive remediation efforts that introduced architectural changes, the modifications to core components extending beyond the original audit scope, and the interconnected nature of the fixes, a comprehensive re-audit of the codebase is essential. The significant updates in the router and staker realms, combined with the new callback pattern and pending upgradeability improvements, require a holistic review to ensure all components function securely together.

In light of the above, it is recommended to establish a structured remediation plan that prioritizes pending issues, implements progressive security measures throughout the protocol, and establishes comprehensive monitoring and incident response procedures. The protocol should undergo an additional audit as the Gno ecosystem matures. Subsequently, a phased launch with limited functionality is advised, expanding features only after thorough battle-testing of core components.

The GnoSwap team is commended for their ambition in building sophisticated DeFi infrastructure on an emerging blockchain platform. Throughout the audit process, the team demonstrated exceptional knowledge of concentrated liquidity mechanics, deep understanding of DeFi primitives, and remarkable responsiveness to audit inquiries. Extensive protocol documentation and thoughtful architectural decisions reflect strong technical expertise in adapting complex protocols to a new runtime environment.

Despite the significant challenges inherent in building DeFi infrastructure on a nascent blockchain, the GnoSwap team has shown strong technical proficiency and dedication throughout the audit. With full remediation, strengthened testing, and continued iteration, GnoSwap is well-positioned to become a cornerstone of the emerging Gno DeFi ecosystem.

Update: Following the completion of this audit, the GnoSwap team undertook significant remediation efforts, including architectural redesigns of the upgradeability framework and implementation of a callback-based swap mechanism. A subsequent audit was conducted focusing on these architectural changes, core AMM components, and in-scope fixes for issues in this audit. Readers are referred to the GnoSwap extended audit report for a comprehensive assessment of the current implementation and its security posture.