fix: Update `noripple_check` to exclude `transactions` field on error responses by mvadari · Pull Request #6303 · XRPLF/rippled

mvadari · 2026-01-30T15:13:48Z

High Level Overview of Change

This PR updates the noripple_check RPC to exclude the transactions field on error responses. It also does some other cleanup of the file, to match modern rippled code.

Context of Change

Fixes #4616

Type of Change

Bug fix (non-breaking change which fixes an issue)

API Impact

Public API: Fix

Before / After

Before: an rpcACT_MALFORMED error message return would have an empty transactions array attached to it.

After: Now it does not.

Test Plan

Added a test. CI passes.

…r responses

Copilot

Pull request overview

This PR fixes issue #4616 by ensuring that the transactions field is not included in error responses from the noripple_check RPC handler. Additionally, it modernizes the code by replacing string literals with jss:: constants for consistency with current rippled conventions.

Changes:

Moved account validation earlier in the flow to fail fast before ledger lookup
Changed transactions array from being conditionally created to always being created as a local variable and only added to the result at the end, ensuring error paths don't include it
Replaced string literals with jss:: constants throughout the handler (e.g., "Account" → jss::Account, "gateway" → jss::gateway)
Added test case to verify transactions field is absent in error responses

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
src/xrpld/rpc/handlers/NoRippleCheck.cpp	Refactored to exclude `transactions` field from error responses, moved account validation earlier, and replaced string literals with jss:: constants
src/test/rpc/NoRippleCheck_test.cpp	Added test to verify `transactions` field is not present on error response and adjusted test structure slightly
include/xrpl/protocol/jss.h	Added `gateway`, `problems`, and `user` constants for use in noripple_check handler
API-CHANGELOG.md	Documented the API change for the unreleased version

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

API-CHANGELOG.md

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

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

Copilot · 2026-01-30T15:48:51Z

src/xrpld/rpc/handlers/NoRippleCheck.cpp

        return false;
    });

+    if (transactions)


According to the API-CHANGELOG.md, the transactions field should not be included in the response if there are no problems. However, this code only checks if the transactions parameter is true, not whether there are any problems. This means an empty transactions array will still be returned when transactions=true but there are no problems, which contradicts the documented behavior. The condition should check both that transactions is true AND that jvTransactions is not empty, or check if problems.size() is greater than 0.

Suggested change

if (transactions)

if (transactions && !jvTransactions.empty())

This is intentional

src/xrpld/rpc/handlers/NoRippleCheck.cpp

codecov · 2026-01-30T16:03:12Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.9%. Comparing base (677758b) to head (726c077).

Additional details and impacted files

@@           Coverage Diff           @@
##           develop   #6303   +/-   ##
=======================================
  Coverage     79.9%   79.9%           
=======================================
  Files          840     840           
  Lines        65483   65484    +1     
  Branches      7255    7251    -4     
=======================================
+ Hits         52311   52320    +9     
+ Misses       13172   13164    -8

Files with missing lines	Coverage Δ
src/xrpld/rpc/handlers/NoRippleCheck.cpp	`100.0% <100.0%> (ø)`

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

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

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

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

Copilot · 2026-02-03T15:27:14Z

src/xrpld/rpc/handlers/NoRippleCheck.cpp

            "experimenting");
    }
-    else if (roleGateway & !bDefaultRipple)
+    else if (roleGateway & !defaultRipple)


Incorrect operator used: bitwise AND (&) is used instead of logical AND (&&). The expression roleGateway & !defaultRipple performs bitwise AND between two boolean values, which can lead to incorrect behavior. This should be roleGateway && !defaultRipple to perform logical AND.

Copilot · 2026-02-03T15:27:14Z

src/test/rpc/NoRippleCheck_test.cpp

+            if (!BEAST_EXPECT(result[jss::transactions].isArray()))
+                return;


The API changelog states that the transactions field should not be included when there are no problems. However, the test expectation on line 228 checks txs.size() == 0, which implicitly expects an empty array rather than the absence of the field. This test should be updated to check !result.isMember(jss::transactions) when there are no problems, even if the transactions parameter was set to true. Note that line 206 also unsafely accesses result[jss::transactions] without checking if the field exists first - this should be guarded with a membership check.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

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

Copilot · 2026-02-05T18:40:10Z

src/xrpld/rpc/handlers/NoRippleCheck.cpp

+    Json::Value jvTransactions = Json::arrayValue;

-    if (bDefaultRipple & !roleGateway)
+    if (defaultRipple & !roleGateway)


This uses bitwise AND (&) instead of logical AND (&&). Since defaultRipple is a boolean, this should use && to avoid unexpected behavior. The same issue appears on line 117.

Copilot · 2026-02-05T18:40:11Z

src/xrpld/rpc/handlers/NoRippleCheck.cpp

            std::string problem;
            bool needFix = false;
-            if (bNoRipple & roleGateway)
+            if (noRipple && roleGateway)


This correctly uses logical AND (&&), but line 110 and 117 in the same function use bitwise AND (&) for the same type of boolean comparison. For consistency and correctness, all boolean comparisons should use logical operators.

feat: Update noripple_check to exclude transactions field on erro…

3367bce

…r responses

mvadari requested review from a team and Copilot January 30, 2026 15:13

Copilot started reviewing on behalf of mvadari January 30, 2026 15:14 View session

Copilot AI reviewed Jan 30, 2026

View reviewed changes

API-CHANGELOG.md Outdated Show resolved Hide resolved

mvadari added 2 commits January 30, 2026 10:34

another if(transactions)

4807dae

only add field if needed

6c3bc27

Copilot AI review requested due to automatic review settings January 30, 2026 15:44

Copilot started reviewing on behalf of mvadari January 30, 2026 15:44 View session

Copilot AI reviewed Jan 30, 2026

View reviewed changes

respond to comments

b896342

mvadari added API Change Added to API Changelog API changes have been documented in API-CHANGELOG.md labels Jan 30, 2026

Merge branch 'develop' into nrc-transactions

9b5ad16

Copilot AI review requested due to automatic review settings February 3, 2026 15:21

Copilot started reviewing on behalf of mvadari February 3, 2026 15:21 View session

Copilot AI reviewed Feb 3, 2026

View reviewed changes

mvadari added 2 commits February 4, 2026 17:06

Merge remote-tracking branch 'upstream/develop' into nrc-transactions

52d09a6

Merge remote-tracking branch 'upstream/develop' into nrc-transactions

ab1f99c

Copilot AI review requested due to automatic review settings February 5, 2026 18:33

Copilot started reviewing on behalf of mvadari February 5, 2026 18:34 View session

Copilot AI reviewed Feb 5, 2026

View reviewed changes

Merge remote-tracking branch 'upstream/develop' into nrc-transactions

726c077

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Update `noripple_check` to exclude `transactions` field on error responses#6303

fix: Update `noripple_check` to exclude `transactions` field on error responses#6303
mvadari wants to merge 8 commits intoXRPLF:developfrom
mvadari:nrc-transactions

mvadari commented Jan 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 30, 2026

Uh oh!

mvadari Jan 30, 2026

Uh oh!

Uh oh!

codecov bot commented Jan 30, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 3, 2026

Uh oh!

Copilot AI Feb 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 5, 2026

Uh oh!

Copilot AI Feb 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	if (transactions)
	if (transactions && !jvTransactions.empty())

		if (!BEAST_EXPECT(result[jss::transactions].isArray()))
		return;

Conversation

mvadari commented Jan 30, 2026

High Level Overview of Change

Context of Change

Type of Change

API Impact

Before / After

Test Plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

mvadari Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

codecov bot commented Jan 30, 2026 •

edited

Loading