Skip to content

Conversation

a1q123456
Copy link
Collaborator

High Level Overview of Change

This PR decouples ledger from xrpld/app, and therefore fully clears the path to the modularisation of the ledger component.

Context of Change

Before this change, View.cpp relies on MPTokenAuthorize::authorize, and this change moves MPTokenAuthorize::authorize to View.cpp to invert the dependency, making ledger a standalone module.

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)
  • Performance (increase or change in throughput and/or latency)
  • 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

This change doesn't affect any API

  • 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)

Test Plan

As usual, regression test just for our sanity

Future Tasks

Modularise ledger

@a1q123456 a1q123456 marked this pull request as ready for review July 3, 2025 14:38
@a1q123456 a1q123456 changed the title [DRAFT] Decouple ledger from xrpld/app Decouple ledger from xrpld/app Jul 3, 2025
@a1q123456 a1q123456 requested a review from yinyiqian1 July 3, 2025 14:39
Copy link

codecov bot commented Jul 3, 2025

Codecov Report

❌ Patch coverage is 92.64706% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.8%. Comparing base (6419f9a) to head (bf2a9a9).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
src/xrpld/ledger/detail/View.cpp 92.5% 4 Missing ⚠️
src/xrpld/app/tx/detail/MPTokenAuthorize.cpp 80.0% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #5492     +/-   ##
=========================================
- Coverage     78.8%   78.8%   -0.0%     
=========================================
  Files          814     814             
  Lines        71270   71269      -1     
  Branches      8343    8348      +5     
=========================================
- Hits         56172   56159     -13     
- Misses       15098   15110     +12     
Files with missing lines Coverage Δ
src/xrpld/app/tx/detail/MPTokenAuthorize.h 100.0% <ø> (ø)
src/xrpld/app/tx/detail/VaultDeposit.cpp 96.4% <100.0%> (ø)
src/xrpld/ledger/View.h 100.0% <ø> (ø)
src/xrpld/app/tx/detail/MPTokenAuthorize.cpp 96.3% <80.0%> (+0.9%) ⬆️
src/xrpld/ledger/detail/View.cpp 90.8% <92.5%> (+<0.1%) ⬆️

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

@bthomee bthomee removed request for Bronek, vlntb and ximinez July 25, 2025 15:15
Copy link
Collaborator

@yinyiqian1 yinyiqian1 left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

@a1q123456 a1q123456 added the Needs second review PR requires at least one more code review approval before it can be merged label Aug 5, 2025
@vvysokikh1 vvysokikh1 removed the Needs second review PR requires at least one more code review approval before it can be merged label Aug 5, 2025
@a1q123456 a1q123456 added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Aug 5, 2025
@bthomee bthomee enabled auto-merge (squash) August 5, 2025 15:00
@bthomee bthomee disabled auto-merge August 5, 2025 15:02
@bthomee bthomee enabled auto-merge (squash) August 5, 2025 15:27
@bthomee bthomee merged commit b5a63b3 into develop Aug 5, 2025
27 checks passed
@bthomee bthomee deleted the a1q123456/decouple-ledger branch August 5, 2025 15:28
mvadari pushed a commit to mvadari/rippled that referenced this pull request Aug 5, 2025
This change decouples `ledger` from `xrpld/app`, and therefore fully clears the path to the modularisation of the ledger component. Before this change, `View.cpp` relied on `MPTokenAuthorize::authorize; this change moves `MPTokenAuthorize::authorize` to `View.cpp` to invert the dependency, making ledger a standalone module.
ximinez added a commit that referenced this pull request Aug 6, 2025
…to ximinez/lending-XLS-66

* XRPLF/ximinez/lending-refactoring-4:
  Fix formatting
  fix: Ensures canonical order for `PriceDataSeries` upon `PriceOracle` creation (#5485)
  Add code coverage for STParsedJSON edge cases
  refactor: Decouple ledger from xrpld/app (#5492)
This was referenced Aug 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants