⚡ Quick Guide: Modularise RelationalDB by a1q123456 · Pull Request #6224 · XRPLF/rippled · GitHub

a1q123456 · 2026-01-15T13:30:50Z

High Level Overview of Change

This PR modularise xrpld/rdb

Context of Change

The rdb module was not properly designed. We have 3 classes:

The abstract class RelationalDB
The abstract class SQLiteDatabase which inherits from RelationalDB and adds some more pure virtual methods
The concrete class SQLiteDatabaseImp which inherits from SQLiteDatabase and implements all the methods.

The factory function RelationalDB::init method checks the config and only returns an instance of SQLiteDatabaseImp, we can see in the code base that we are doing dynamic_cast<SQLiteDatabase*>(app.getRelationalDb()) everywhere.

To address this problem and help modularise app/tx, the rdb module needs to be refactored and moved to libxrpl.

This PR is part of the modularisation of the transactors. See all PRs here:
#6222
#6223
#6224
#6225
#6226
#6227
#6228

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

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)

codecov · 2026-01-15T16:31:46Z

Codecov Report

❌ Patch coverage is 78.65169% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.9%. Comparing base (ef28469) to head (86ba05b).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
...rc/xrpld/app/rdb/backend/detail/SQLiteDatabase.cpp	69.8%	16 Missing ⚠️
src/xrpld/app/main/Application.cpp	50.0%	2 Missing ⚠️
src/xrpld/rpc/handlers/AccountTx.cpp	80.0%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           develop   #6224   +/-   ##
=======================================
  Coverage     79.9%   79.9%           
=======================================
  Files          840     840           
  Lines        65522   65508   -14     
  Branches      7251    7250    -1     
=======================================
- Hits         52355   52353    -2     
+ Misses       13167   13155   -12

Files with missing lines	Coverage Δ
include/xrpl/rdb/RelationalDatabase.h	`100.0% <100.0%> (ø)`
src/xrpld/app/ledger/Ledger.cpp	`82.4% <100.0%> (+0.1%)`	⬆️
src/xrpld/app/ledger/detail/LedgerMaster.cpp	`43.2% <ø> (ø)`
src/xrpld/app/misc/SHAMapStoreImp.cpp	`75.7% <100.0%> (+0.2%)`	⬆️
src/xrpld/app/misc/Transaction.h	`96.1% <ø> (ø)`
src/xrpld/app/misc/detail/Transaction.cpp	`83.3% <100.0%> (+1.1%)`	⬆️
src/xrpld/app/rdb/backend/SQLiteDatabase.h	`100.0% <100.0%> (ø)`
src/xrpld/app/rdb/backend/detail/Node.cpp	`58.6% <ø> (ø)`
src/xrpld/overlay/detail/OverlayImpl.cpp	`32.9% <ø> (ø)`
src/xrpld/overlay/detail/PeerReservationTable.cpp	`16.3% <ø> (ø)`
... and 7 more

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

kuznetsss · 2026-01-23T17:08:27Z

+        std::uint32_t minLedger;
+        std::uint32_t maxLedger;


Maybe use LedgerRange?

This is just moved from src/xrpld/app/rdb/RelationalDatabase.h. I think we can have a separate PR for optimisations like this.

Will be fixed in RIPD-4880.

kuznetsss · 2026-01-23T17:09:49Z

+    using AccountTx =
+        std::pair<std::shared_ptr<Transaction>, std::shared_ptr<TxMeta>>;
+    using AccountTxs = std::vector<AccountTx>;
+    using txnMetaLedgerType = std::tuple<Blob, Blob, std::uint32_t>;


Better to avoid pairs and tuples if possible because they decrease readability

Will be fixed in RIPD-4880.

kuznetsss · 2026-01-23T17:14:09Z

+    closeTransactionDB() = 0;
+};
+
+template <class T, class C>


Please add concepts on T and C to improve compilation errors on misuse. Probably std::is_arithmetic and that std::is_convertible.

Will be fixed in RIPD-4880.

godexsoft

Looks fine from refactoring perspective except for the questions i left. I agree with @kuznetsss and there are more obvious things we should fix but i'd leave that for another pr.

godexsoft · 2026-02-10T19:05:12Z

-    auto const db = dynamic_cast<SQLiteDatabase*>(&app.getRelationalDatabase());
-    if (!db)
-        Throw<std::runtime_error>("Failed to get relational database");
+    auto& db = app.getRelationalDatabase();


Are we always guaranteed to have a db now? is there no way to start xrpld without a relational db? why was it possible before, do you know?

We're always guaranteed to have a db even before the change. You can see from the code that we actually used to return a reference instead of a pointer.

The reason why we used to check if db is null is that we had to perform a dyanmic_cast on it, and the reason why we needed to dynamic_cast was the bad design.

We had 3 classes before the change, RelationDatabase, SQLiteDatabase, SQLiteDatabaseImp. The first two are abstract classes, and the last one is a concrete class. RelationDatabase defines some abstract methods, and then SQLiteDatabase inherits from it, adding more abstract methods, and finally, SQLiteDatabaseImp implements all the methods. We should just merge SQLiteDatabase into RelationDatabase because in my opinion, it makes no sense to have a SQLiteDatabase interface that adds some methods but hide that interface under the hood if the business logic needs those methods, and the name creates more confusion when putting it together with SQLiteDatabaseImp as SQLiteDatabase already sounds like the implementation of RelationDatabase.

Another funny thing is that RelationalDatabase::init checks if the rdb backend is sqlite, and it throws an exception if not, which further suggests that there's no chance to have anything other than SQLiteDatabase.

Ok i see the reasoning. It makes more sense to store and return by interface in the service locator. This will allow us to add another implementation in the future, if we ever need one 👍

kuznetsss

👍

Signed-off-by: JCW <a1q123456@users.noreply.github.com>

godexsoft

Good for modularization 👍

Signed-off-by: JCW <a1q123456@users.noreply.github.com>

The rdb module was not properly designed, which is fixed in this change. The module had three classes: 1) The abstract class `RelationalDB`. 2) The abstract class `SQLiteDatabase`, which inherited from `RelationalDB` and added some pure virtual methods. 3) The concrete class `SQLiteDatabaseImp`, which inherited from `SQLiteDatabase` and implemented all methods. The updated code simplifies this as follows: * The `SQLiteDatabaseImp` has become `SQLiteDatabase`, and * The former `SQLiteDatabase `has merged with `RelationalDatabase`.

a1q123456 requested review from a team, kuznetsss and vlntb January 15, 2026 13:30

a1q123456 force-pushed the a1q123456/modularise-relational-db branch 3 times, most recently from dd1a1a9 to cf67641 Compare January 15, 2026 14:19

kuznetsss reviewed Jan 23, 2026

View reviewed changes

a1q123456 force-pushed the a1q123456/modularise-wallet-db-and-manifest branch from c9dbea1 to 8dc9483 Compare February 3, 2026 16:43

a1q123456 force-pushed the a1q123456/modularise-relational-db branch from 922752a to 17020f9 Compare February 3, 2026 17:20

a1q123456 force-pushed the a1q123456/modularise-wallet-db-and-manifest branch 2 times, most recently from 572f5b2 to 650dc4f Compare February 4, 2026 11:27

a1q123456 force-pushed the a1q123456/modularise-relational-db branch 2 times, most recently from eebaec1 to 860e22d Compare February 4, 2026 13:30

a1q123456 force-pushed the a1q123456/modularise-wallet-db-and-manifest branch from 0dd8b72 to b3f0361 Compare February 4, 2026 15:11

a1q123456 force-pushed the a1q123456/modularise-relational-db branch from 860e22d to 39f6e3a Compare February 4, 2026 15:16

a1q123456 force-pushed the a1q123456/modularise-wallet-db-and-manifest branch from b3f0361 to c0bbbb2 Compare February 4, 2026 15:30

a1q123456 force-pushed the a1q123456/modularise-relational-db branch from 39f6e3a to e436dff Compare February 4, 2026 15:37

godexsoft reviewed Feb 10, 2026

View reviewed changes

kuznetsss reviewed Feb 11, 2026

View reviewed changes

Comment thread src/xrpld/app/main/Application.cpp

Comment thread src/xrpld/app/main/Application.cpp Outdated

Base automatically changed from a1q123456/modularise-wallet-db-and-manifest to develop February 11, 2026 13:42

kuznetsss approved these changes Feb 11, 2026

View reviewed changes

a1q123456 added the Needs additional review PR requires at least one more code review approval before it can be merged label Feb 11, 2026

Move LedgerShortcut to libxrpl.protocol as it's a protocol thing

c5d56a5

Signed-off-by: JCW <a1q123456@users.noreply.github.com>

a1q123456 added 5 commits February 11, 2026 15:01

Modularise RelationalDatabase

e8f62e9

Fix levelisation

9ce5145

Signed-off-by: JCW <a1q123456@users.noreply.github.com>

Fix issues

9778aa6

Signed-off-by: JCW <a1q123456@users.noreply.github.com>

Address PR comments

5bc3459

Signed-off-by: JCW <a1q123456@users.noreply.github.com>

Address PR comments

6ea2646

Signed-off-by: JCW <a1q123456@users.noreply.github.com>

a1q123456 force-pushed the a1q123456/modularise-relational-db branch from 9dbe6b9 to 6ea2646 Compare February 11, 2026 15:02

godexsoft approved these changes Feb 11, 2026

View reviewed changes

a1q123456 removed the Needs additional review PR requires at least one more code review approval before it can be merged label Feb 11, 2026

Fix errors

86ba05b

Signed-off-by: JCW <a1q123456@users.noreply.github.com>

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 Feb 11, 2026

bthomee enabled auto-merge (squash) February 11, 2026 16:03

bthomee merged commit 9f17d10 into develop Feb 11, 2026
1 check passed

bthomee deleted the a1q123456/modularise-relational-db branch February 11, 2026 16:22

a1q123456 added a commit that referenced this pull request Feb 19, 2026

Address PR comments in #6224

1f84174

Signed-off-by: JCW <a1q123456@users.noreply.github.com>

mvadari added this to the 3.2.0 milestone May 20, 2026

Uh oh!

Conversation

a1q123456 commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

High Level Overview of Change

Context of Change

Type of Change

API Impact

Uh oh!

codecov Bot commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

godexsoft left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kuznetsss left a comment

Choose a reason for hiding this comment

Uh oh!

godexsoft left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

a1q123456 commented Jan 15, 2026 •

edited

Loading

codecov Bot commented Jan 15, 2026 •

edited

Loading