Skip to content

Hook Administrator Amendment [DO NOT MERGE]#662

Open
tequdev wants to merge 25 commits into
devfrom
HookAdministrator
Open

Hook Administrator Amendment [DO NOT MERGE]#662
tequdev wants to merge 25 commits into
devfrom
HookAdministrator

Conversation

@tequdev

@tequdev tequdev commented Jan 5, 2026

Copy link
Copy Markdown
Member

High Level Overview of Change

Context of Change

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Tests (you added tests for code that already exists, or your new feature included in this PR)
  • Documentation update
  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
  • Release

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

@tequdev tequdev added Amendment New feature or fix amendment feature Feature Request labels Jan 5, 2026
@tequdev tequdev linked an issue Jan 5, 2026 that may be closed by this pull request
tequdev and others added 20 commits January 10, 2026 11:42
- Add owner count and account index to AccountRoot
- Increment account count at FeeSetting
* 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`.
Backport of XRPLF/rippled#6325. The python version runs ~80x faster.
@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 64.28571% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.3%. Comparing base (c55420b) to head (b5fdae6).

Files with missing lines Patch % Lines
src/xrpld/app/tx/detail/SetHook.cpp 63.3% 29 Missing ⚠️
src/xrpld/app/tx/detail/DeleteAccount.cpp 50.0% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             dev    #662     +/-   ##
=======================================
- Coverage   77.3%   77.3%   -0.0%     
=======================================
  Files        837     837             
  Lines      78575   78654     +79     
  Branches   11551   11568     +17     
=======================================
+ Hits       60748   60784     +36     
- Misses     17817   17860     +43     
  Partials      10      10             
Files with missing lines Coverage Δ
include/xrpl/protocol/detail/ledger_entries.macro 100.0% <ø> (ø)
include/xrpl/protocol/detail/transactions.macro 100.0% <ø> (ø)
src/xrpld/app/tx/detail/InvariantCheck.cpp 90.5% <100.0%> (+<0.1%) ⬆️
src/xrpld/app/tx/detail/DeleteAccount.cpp 83.0% <50.0%> (-0.4%) ⬇️
src/xrpld/app/tx/detail/SetHook.cpp 80.2% <63.3%> (-1.3%) ⬇️

... and 9 files with indirect coverage changes

Impacted file tree graph

🚀 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.

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.

Idea: Install Hook on a new account with the SetHook transaction

6 participants