Skip to content

Base hook contracts: review notes #144

Description

@luiz-lvj

Grouping a set of observations about the base hook contracts in one place, for tracking and to reduce duplicate reports. Items are summarized briefly and are not all confirmed. No severity is assigned here.

BaseCustomAccounting

  • Position salt derived from msg.sender — The position salt is keccak256(sender, salt) with sender fixed to the depositor, so the ability to withdraw is bound to the depositor address rather than to any receipt-share ownership.
  • User-controlled ranges with per-sender salt — If an inheriting hook forwards user-controlled tick ranges and salt, many small distinct positions can be created that others cannot remove, which can keep tick-crossing costs elevated.
  • Single-pool binding at initialization — The first beforeInitialize permanently stores the observed PoolKey with no validation, so an unintended pool can be bound if it is initialized first.
  • Zero-amount native refundaddLiquidity calls the native refund transfer even when the refund is zero, which can revert for some contract callers; the refund also runs as a trailing external call.

BaseCustomCurve

  • Swap fee computed but not applied_beforeSwap computes and emits a swap fee but builds the deltas without it, so an overridden fee is not collected on-chain.
  • HookSwap sign conventionHookSwap is emitted with unsigned amounts, so the output leg is not negated as IHookEvents expects.

BaseAsyncSwap

  • Missing pool binding — Initialization callbacks are disabled and _beforeSwap does not validate the pool id, so a permissionlessly created pool referencing the hook can reach shared claim state.
  • Emitted LP fee not charged — A computed LP fee is emitted in HookSwap but never settled in the async flow.
  • Documentation on claim redemption — The documentation describes redeeming ERC-6909 claims via CurrencySettler.settle, which does not pay out; correct redemption uses settle(burn=true) then take.

BaseHook

  • onlyValidPools scope — The modifier checks key.hooks == address(this) but not the pool identity, so inheritors that assume a single pool or share state across pools are not protected by it.
  • Non-forwarding _beforeSwap overrides — Some mixins implement _beforeSwap without calling super, so composing them can silently skip another module's logic.

Sources: src/base/.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions