🎯 Practical: refactor: Clean up NetworkOPs by a1q123456 · Pull Request #6575 · XRPLF/rippled · GitHub

a1q123456 · 2026-03-18T13:44:54Z

High Level Overview of Change

This PR cleans up a code path that checks the database type because never going to hit, and a dynamic cast that casts the database type from RelationalDatabase to SqlLiteDatabase in NetworkOPsImp::addAccountHistoryJob.

Detailed change list

Removed the lines from 3486 to 3510. Removed the databaseType capture from the lambda at the line 3513, and extracted case Sqlite from the switch-case block.

Context of Change

We simplified the inheritance hierarchy for RelationalDatabase in a previous PR and most of the services don't need to downcast the database instance to SqlLiteDatabase anymore, also, a good design should never care about what the actual implementation is.

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)

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

codecov · 2026-03-18T14:07:19Z

Codecov Report

❌ Patch coverage is 74.56140% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.5%. Comparing base (7c7c189) to head (4db6b4b).
⚠️ Report is 9 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/xrpld/app/misc/NetworkOPs.cpp	74.6%	29 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #6575     +/-   ##
=========================================
- Coverage     82.5%   82.5%   -0.0%     
=========================================
  Files         1010    1010             
  Lines        79246   79232     -14     
  Branches      7529    7531      +2     
=========================================
- Hits         65389   65366     -23     
- Misses       13857   13866      +9

Files with missing lines	Coverage Δ
src/xrpld/app/misc/NetworkOPs.cpp	`71.5% <74.6%> (-0.1%)`	⬇️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

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

…eanup-for-network-ops Signed-off-by: JCW <a1q123456@users.noreply.github.com> # Conflicts: # src/xrpld/app/misc/NetworkOPs.cpp

vlntb · 2026-03-26T13:57:33Z

+                auto const& txns = dbResult->first;
+                marker = dbResult->second;
+                size_t num_txns = txns.size();
+                for (size_t i = 0; i < num_txns; ++i)


[nit] while we're touching this code, could we move away from the index-based loop and use a range-based or iterator-style loop instead? Not a blocker though — I’m also fine leaving this kind of mechanical cleanup to clang-tidy.

Because what we're doing at the line 3741, we can't convert this for loop until we upgrade to c++ 23, which will give us std::views::chunk_by

vlntb

This change looks good to me. I like the simplification here: it removes handling for an impossible case and avoids downcasting by relying on the shared interface instead. Approving.

xrplf-ai-reviewer

Had a look at this

Potential data race flagged inline: concurrent history jobs for the same account share mutable index_ fields with no synchronization — see line 3553.

Review by ReviewBot 🤖

Review by Claude Opus 4.6 · Prompt: V12

Copilot

Pull request overview

This PR refactors NetworkOPsImp::addAccountHistoryJob to avoid depending on a specific relational DB implementation (SQLite) and removes a previously-unreachable database-type check/downcast, aligning the code with the simplified RelationalDatabase hierarchy.

Changes:

Removes SQLiteDatabase include and eliminates database-type probing via dynamic_cast.
Updates account history paging to call RelationalDatabase::newestAccountTxPage() directly.
Restructures the job lambda to remove the SQLite-specific switch/case and related unreachable paths.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-actions · 2026-03-30T15:46:28Z

This PR has conflicts, please resolve them in order for the PR to be reviewed.

github-actions · 2026-03-31T12:48:42Z

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

…eanup-for-network-ops

xrplf-ai-reviewer

Two issues flagged inline: missing null guard on subInfo.index_ and a potential data race on stopHistorical_.

Review by Claude Opus 4.6 · Prompt: V12

github-actions · 2026-03-31T17:50:31Z

This PR has conflicts, please resolve them in order for the PR to be reviewed.

…eanup-for-network-ops

github-actions · 2026-04-01T13:44:58Z

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

…eanup-for-network-ops

xrplf-ai-reviewer

No issues.

Review by Claude Opus 4.6 · Prompt: V14

godexsoft

Looks fine overall. You can likely add const in many places in the changed code. But since it's not new code and clang-tidy can do that later too, it's ok to leave it out.

github-actions · 2026-04-21T15:59:08Z

This PR has conflicts, please resolve them in order for the PR to be reviewed.

…eanup-for-network-ops

github-actions · 2026-04-22T13:02:43Z

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

xrplf-ai-reviewer

No issues.

Review by Claude Opus 4.6 · Prompt: V15

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

Clean up NetworkOPs

beceec3

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

a1q123456 requested review from godexsoft and vlntb March 18, 2026 13:44

a1q123456 changed the title ~~Clean up NetworkOPs~~ refactor: Clean up NetworkOPs Mar 18, 2026

Merge remote-tracking branch 'origin/develop' into a1q123456/minor-cl…

22cc6f8

…eanup-for-network-ops Signed-off-by: JCW <a1q123456@users.noreply.github.com> # Conflicts: # src/xrpld/app/misc/NetworkOPs.cpp

vlntb reviewed Mar 26, 2026

View reviewed changes

vlntb approved these changes Mar 26, 2026

View reviewed changes

kuznetsss requested a review from Copilot March 27, 2026 14:02

Copilot started reviewing on behalf of kuznetsss March 27, 2026 14:03 View session

xrplf-ai-reviewer Bot reviewed Mar 27, 2026

View reviewed changes

Comment thread src/xrpld/app/misc/NetworkOPs.cpp

Copilot AI reviewed Mar 27, 2026

View reviewed changes

Comment thread src/xrpld/app/misc/NetworkOPs.cpp Outdated

Comment thread src/xrpld/app/misc/NetworkOPs.cpp

github-actions Bot added the PR: has conflicts label Mar 30, 2026

github-actions Bot removed the PR: has conflicts label Mar 31, 2026

Merge remote-tracking branch 'origin/develop' into a1q123456/minor-cl…

3174777

…eanup-for-network-ops

a1q123456 force-pushed the a1q123456/minor-cleanup-for-network-ops branch from 6862d87 to 3174777 Compare March 31, 2026 12:50

xrplf-ai-reviewer Bot reviewed Mar 31, 2026

View reviewed changes

Comment thread src/xrpld/app/misc/NetworkOPs.cpp

Comment thread src/xrpld/app/misc/NetworkOPs.cpp

github-actions Bot added the PR: has conflicts label Mar 31, 2026

Merge remote-tracking branch 'origin/develop' into a1q123456/minor-cl…

c44eb59

…eanup-for-network-ops

github-actions Bot removed the PR: has conflicts label Apr 1, 2026

Merge remote-tracking branch 'origin/develop' into a1q123456/minor-cl…

9213f43

…eanup-for-network-ops

xrplf-ai-reviewer Bot approved these changes Apr 14, 2026

View reviewed changes

godexsoft approved these changes Apr 20, 2026

View reviewed changes

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 Apr 21, 2026

github-actions Bot added the PR: has conflicts label Apr 21, 2026

Merge remote-tracking branch 'origin/develop' into a1q123456/minor-cl…

4db6b4b

…eanup-for-network-ops

github-actions Bot removed the PR: has conflicts label Apr 22, 2026

xrplf-ai-reviewer Bot approved these changes Apr 22, 2026

View reviewed changes

bthomee added this pull request to the merge queue Apr 23, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Apr 23, 2026

bthomee added this pull request to the merge queue Apr 23, 2026

Merged via the queue into develop with commit 7a449ed Apr 23, 2026
2 of 3 checks passed

bthomee deleted the a1q123456/minor-cleanup-for-network-ops branch April 23, 2026 18:17

marek-foss-neti pushed a commit to marek-foss-neti/rippled that referenced this pull request May 5, 2026

refactor: Clean up NetworkOPs (XRPLF#6575)

e34bf5b

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

mvadari added this to the 3.2.0 milestone May 20, 2026

beartec-jpg pushed a commit to beartec-jpg/FalconLedger that referenced this pull request Jun 1, 2026

refactor: Clean up NetworkOPs (XRPLF#6575)

12a4ef3

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

Uh oh!

Conversation

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

Uh oh!

High Level Overview of Change

Detailed change list

Context of Change

API Impact

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

vlntb Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

a1q123456 Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

vlntb left a comment

Choose a reason for hiding this comment

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Mar 30, 2026

Uh oh!

github-actions Bot commented Mar 31, 2026

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Mar 31, 2026

Uh oh!

github-actions Bot commented Apr 1, 2026

Uh oh!

xrplf-ai-reviewer Bot 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!

github-actions Bot commented Apr 21, 2026

Uh oh!

github-actions Bot commented Apr 22, 2026

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

a1q123456 commented Mar 18, 2026 •

edited

Loading

codecov Bot commented Mar 18, 2026 •

edited

Loading