fix: Fix wrong hybrid offer orderbook placement and update LedgerStateFix to amend ExchangeRate meta - #7087
Merged
Merged
Conversation
shawnxie999
marked this pull request as ready for review
May 6, 2026 20:41
shawnxie999
changed the title
LedgerStateFix to amend ExchangeRate meta
shawnxie999
changed the title
LedgerStateFix to amend ExchangeRate metaLedgerStateFix to amend ExchangeRate meta
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #7087 +/- ##
=======================================
Coverage 82.1% 82.1%
=======================================
Files 1010 1011 +1
Lines 76268 76343 +75
Branches 7412 7412
=======================================
+ Hits 62610 62683 +73
- Misses 13658 13660 +2
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
yinyiqian1
reviewed
May 7, 2026
Kassaking7
reviewed
May 7, 2026
|
This PR has conflicts, please resolve them in order for the PR to be reviewed. |
gregtatcam
approved these changes
May 13, 2026
gregtatcam
left a comment
gregtatcam
left a comment
Contributor
There was a problem hiding this comment.
👍 LGTM
I left two minor comments for your consideration.
|
All conflicts have been resolved. Assigned reviewers can now start or resume their review. |
Kassaking7
approved these changes
May 20, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a legacy hybrid-offer placement bug where the open-book directory key quality could diverge from the directory’s sfExchangeRate metadata after partial crossing, and adds tooling + invariants to detect/repair mismatched book-directory exchange-rate metadata.
Changes:
- Adjust
OfferCreatehybrid placement so the open-book directory uses the original placement rate (post-fixCleanup3_2_0) instead of a post-crossing-derived rate. - Extend
LedgerStateFixwith a newBookExchangeRatefix type to repairsfExchangeRateon existing book root directory pages. - Add a new invariant (
ValidBookDirectory) to enforcesfExchangeRatemetadata consistency for newly-created book directories, plus add/extend tests and jtx helpers.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libxrpl/tx/transactors/dex/OfferCreate.cpp | Uses feature-gated openRate for hybrid open-book directory placement to prevent key/metadata divergence. |
| include/xrpl/tx/transactors/dex/OfferCreate.h | Updates applyHybrid signature to accept the precomputed open-book quality rate. |
| src/libxrpl/tx/transactors/system/LedgerStateFix.cpp | Adds preflight/preclaim/doApply handling for FixType::BookExchangeRate. |
| include/xrpl/tx/transactors/system/LedgerStateFix.h | Adds FixType::BookExchangeRate enum value (public header change). |
| include/xrpl/protocol/detail/transactions.macro | Adds optional sfBookDirectory field to LedgerStateFix transaction format. |
| include/xrpl/protocol_autogen/transactions/LedgerStateFix.h | Autogen wrapper/builder support for optional sfBookDirectory. |
| src/tests/libxrpl/protocol_autogen/transactions/LedgerStateFixTests.cpp | Extends round-trip tests to cover sfBookDirectory. |
| src/test/jtx/ledgerStateFix.h | Adds jtx helper declaration for bookExchangeRate. |
| src/test/jtx/impl/ledgerStateFixes.cpp | Implements ledgerStateFix::bookExchangeRate JSON constructor. |
| src/libxrpl/tx/invariants/DirectoryInvariant.cpp | Introduces ValidBookDirectory invariant implementation. |
| include/xrpl/tx/invariants/DirectoryInvariant.h | Declares ValidBookDirectory invariant class. |
| include/xrpl/tx/invariants/InvariantCheck.h | Registers ValidBookDirectory in the invariant tuple. |
| src/test/app/Invariants_test.cpp | Adds invariant tests for book-directory exchange rate consistency and legacy tolerance. |
| src/test/app/PermissionedDEX_test.cpp | Adds regression test for hybrid crossing quality + LedgerStateFix repair-path test. |
| src/test/app/FixNFTokenPageLinks_test.cpp | Adds negative test ensuring fix-type-specific fields are rejected. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
This PR has conflicts, please resolve them in order for the PR to be reviewed. |
|
All conflicts have been resolved. Assigned reviewers can now start or resume their review. |
Contributor
|
/ai-review |
![xrplf-ai-reviewer[bot]](https://avatars.githubusercontent.com/u/67929741?s=60&v=4){kind=small}
ximinez
reviewed
May 21, 2026
ximinez
left a comment
ximinez
left a comment
Contributor
There was a problem hiding this comment.
@shawnxie999 @bthomee The below issue needs to be resolved before the release. You should also add some test cases that delete an order book directory entry with the bad value to ensure you don't get any false hits in the invariant.
Comment on lines
+52
to
+53
| if (badBookDirectory_ || before || !after || after->getType() != ltDIR_NODE) | ||
| return; |
Contributor
There was a problem hiding this comment.
You should probably also return early if isDelete is true.
Contributor
Merged
4 tasks
Kassaking7
pushed a commit
to Kassaking7/rippled
that referenced
this pull request
Jun 2, 2026
…teFix` to amend `ExchangeRate` meta (XRPLF#7087) Co-authored-by: Peter Chen <ychen@ripple.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
High Level Overview of Change
1. Fix hybrid offer placement
For hybrid offers,
sfBookDirectorypoints to the permissioned domain book, andsfAdditionalBooks[0].sfBookDirectorypoints to the open book.This fixes
OfferCreateso the open-book directory insfAdditionalBooksuses the original offer quality after partial crossing, instead of the post-crossing quality.2. Add legacy repair support
Adds
LedgerStateFixsupport to repair legacy book-directorysfExchangeRatemetadata when it does not match the quality encoded in the directory key.Also adds invariant coverage for new book directories to make sure
ExchangeRatemeta matches the real qualityAPI Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)