Skip to content

Named Hook#718

Open
tequdev wants to merge 34 commits intoXahau:devfrom
tequdev:named-hook
Open

Named Hook#718
tequdev wants to merge 34 commits intoXahau:devfrom
tequdev:named-hook

Conversation

@tequdev
Copy link
Copy Markdown
Member

@tequdev tequdev commented Mar 16, 2026

High Level Overview of Change

Add support for named hooks via a new optional HookName field on hook definitions/usages.

This change introduces the featureNamedHooks amendment, allows SetHook to create/install/update hooks with an optional UTF-8 hook name, and updates hook execution/fee calculation so named hooks only run when the incoming transaction includes the matching HookName. The PR also adds coverage for invalid names, create/install/update flows, strong/weak execution paths, and callback/emitted transaction behavior.

Context of Change

Today hooks are selected only by install position and existing hook matching rules. This change adds a lightweight name-based gate so an installed hook can require an explicit transaction opt-in through HookName.

The implementation keeps HookName on the installed ltHook object rather than the shared hook definition, which avoids changing hook bytecode identity and lets different accounts install the same hook definition with or without a name. Validation enforces that HookName is either empty or a valid UTF-8 string between 4 and 16 bytes (8 to 32 hex chars in JSON form). Transactions carrying HookName are preflight-validated.

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Tests (you added tests for code that already exists, or your new feature included in this PR)

Before / After

Before:

  • SetHook did not accept HookName.
  • Transactions could not declare a hook name target.

After:

  • SetHook accepts optional HookName when featureNamedHooks is enabled.
  • Transactions may include optional HookName.
  • Installed hooks with a non-empty HookName execute only when the transaction carries the same name.
  • Hook chain fee calculation now follows the same name-matching rule.
  • Empty HookName on update removes the stored name from the installed hook.

RichardAH and others added 12 commits February 24, 2026 16:07
* Add AMM bid/create/deposit/swap/withdraw/vote invariants:
  - Deposit, Withdrawal invariants: `sqrt(asset1Balance * asset2Balance) >= LPTokens`.
  - Bid: `sqrt(asset1Balance * asset2Balance) > LPTokens` and the pool balances don't change.
  - Create: `sqrt(asset1Balance * assetBalance2) == LPTokens`.
  - Swap: `asset1BalanceAfter * asset2BalanceAfter >= asset1BalanceBefore * asset2BalanceBefore`
     and `LPTokens` don't change.
  - Vote: `LPTokens` and pool balances don't change.
  - All AMM and swap transactions: amounts and tokens are greater than zero, except on withdrawal if all tokens
    are withdrawn.
* Add AMM deposit and withdraw rounding to ensure AMM invariant:
  - On deposit, tokens out are rounded downward and deposit amount is rounded upward.
  - On withdrawal, tokens in are rounded upward and withdrawal amount is rounded downward.
* Add Order Book Offer invariant to verify consumed amounts. Consumed amounts are less than the offer.
* Fix Bid validation. `AuthAccount` can't have duplicate accounts or the submitter account.
Due to rounding, the LPTokenBalance of the last LP might not match the LP's trustline balance. This was fixed for `AMMWithdraw` in `fixAMMv1_1` by adjusting the LPTokenBalance to be the same as the trustline balance. Since `AMMClawback` is also performing a withdrawal, we need to adjust LPTokenBalance as well in `AMMClawback.`

This change includes:
1. Refactored `verifyAndAdjustLPTokenBalance` function in `AMMUtils`, which both`AMMWithdraw` and `AMMClawback` call to adjust LPTokenBalance.
2. Added the unit test `testLastHolderLPTokenBalance` to test the scenario.
3. Modify the existing unit tests for `fixAMMClawbackRounding`.
@tequdev tequdev added Amendment New feature or fix amendment feature Feature Request labels Mar 16, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 29, 2026

Codecov Report

❌ Patch coverage is 78.33333% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.59%. Comparing base (9b50b68) to head (b106559).

Files with missing lines Patch % Lines
src/xrpld/app/tx/detail/SetHook.cpp 77.77% 1 Missing and 7 partials ⚠️
src/xrpld/app/tx/detail/Transactor.cpp 79.16% 0 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #718      +/-   ##
==========================================
+ Coverage   66.57%   66.59%   +0.02%     
==========================================
  Files         837      837              
  Lines       78540    78593      +53     
  Branches    44600    44642      +42     
==========================================
+ Hits        52288    52342      +54     
+ Misses      17859    17855       -4     
- Partials     8393     8396       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/xrpld/app/tx/detail/SetHook.cpp Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Amendment New feature or fix amendment feature Feature Request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants