Skip to content

Fix hooks#156

Merged
jubeira merged 11 commits intoreclamm-2.1from
dev/fix-hooks
Mar 3, 2026
Merged

Fix hooks#156
jubeira merged 11 commits intoreclamm-2.1from
dev/fix-hooks

Conversation

@jubeira
Copy link
Contributor

@jubeira jubeira commented Feb 27, 2026

Description

Add protections for non-view hooks (onBefore...) so that they cannot be called by attackers using fake pools pointing to legit pools as hooks.

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Dependency changes
  • Code refactor / cleanup
  • Optimization: [ ] gas / [ ] bytecode
  • Documentation or wording changes
  • Other

Checklist:

  • The diff is legible and has no extraneous changes
  • Complex code has been commented, including external interfaces
  • Tests have 100% code coverage
  • The base branch is either main, or there's a description of how to merge

Issue Resolution

N/A

@jubeira jubeira changed the title Dev/fix hooks Fix hooks Feb 27, 2026
@jubeira jubeira requested review from EndymionJkb and joaobrunoah and removed request for EndymionJkb March 2, 2026 14:21
@jubeira jubeira marked this pull request as ready for review March 2, 2026 15:03
Copy link
Collaborator

@joaobrunoah joaobrunoah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Collaborator

@EndymionJkb EndymionJkb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good; just some minor comments. Also have a follow-on

}

// This hook makes sure that the virtual balances are decreased in the same proportion as the real balances
// after removing liquidity. This is needed to keep the pool centeredness and price ratio constant.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in diff, but we'd discussed adding modifiers to start/stop ratio updates, and decided against it, mainly because of the low impact and potential griefing vector. Requiring the lock would make governance operations fragile in a way that has no offsetting security benefit.
We should document that decision, though. Could add this comment to both functions:

/**
 * @inheritdoc IReClammPool
 *
 * @dev Note: this function does not require `onlyWhenVaultIsLocked`, unlike other functions that update virtual
 * balances. The price ratio update is gradual, with a minimum duration, and bounded in rate, so any imprecision
 * in the virtual balance snapshot taken here is temporary and corrected on the next swap. Requiring the vault
 * to be locked would introduce a griefing vector: a watcher could front-run a governance call by unlocking the
 * vault, causing the transaction to revert.
 */

@EndymionJkb EndymionJkb mentioned this pull request Mar 2, 2026
12 tasks
@jubeira jubeira merged commit 41b5476 into reclamm-2.1 Mar 3, 2026
3 checks passed
@jubeira jubeira deleted the dev/fix-hooks branch March 3, 2026 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants