fix: Prevents overwritting a bool value in an invariant (#6609) by ximinez · Pull Request #6961 · XRPLF/rippled

ximinez · 2026-04-16T21:50:15Z

High Level Overview of Change

This PR backports the changes from #6609 to 3.1.x.

github-actions · 2026-04-16T21:51:45Z

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

github-actions · 2026-04-16T22:34:46Z

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

ximinez · 2026-04-16T22:52:02Z

/ai-review

xrplf-ai-reviewer

Critical correctness bug: line 416 (bad_ = outstanding < *locked) was not removed as part of this fix and silently overwrites bad_ before the new |= logic runs, defeating the fix entirely. See inline comments.

Review by Claude Opus 4.6 · Prompt: V15

Copilot

Pull request overview

Backports the fix from #6609 to prevent invariant state booleans from being overwritten across multiple visitEntry calls, and adds a safer isFeatureEnabled helper for contexts where transaction rules may be unset.

Changes:

Add amendment-gated “overwrite fix” logic to multiple invariants by accumulating (|=) rather than overwriting boolean state.
Extend isFeatureEnabled to support a caller-chosen default when no CurrentTransactionRules are set.
Update/add tests to exercise the overwrite fix both enabled and disabled, and simplify feature bitsets to rely on testable_amendments().

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/xrpld/app/tx/detail/InvariantCheck.cpp	Adds overwrite-fix gating for invariant booleans (but currently contains a regression in one branch).
src/libxrpl/protocol/Rules.cpp	Implements new `isFeatureEnabled(feature, resultIfNoRules)` behavior.
include/xrpl/protocol/Rules.h	Updates `isFeatureEnabled` declaration and documents the new parameter (potential ABI/API considerations).
src/test/app/Invariants_test.cpp	Adds helper amendments bitset + rules guard and runs overwrite tests for both enabled/disabled fix (currently has a namespace issue).
src/test/app/Vault_test.cpp	Test cleanups and a type change in callbacks (currently uses an apparently invalid `xrpl::Keylet` type).
src/test/rpc/GatewayBalances_test.cpp	Makes the feature bitset const (no behavior change).
src/test/app/Loan_test.cpp	Simplifies feature set to `testable_amendments()`.
src/test/app/LoanBroker_test.cpp	Simplifies feature set to `testable_amendments()`.
src/test/app/Batch_test.cpp	Simplifies Env feature composition to use the passed-in feature bitset.
src/test/app/AMM_test.cpp	Simplifies feature composition to rely on `testable_amendments()`.

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

xrplf-ai-reviewer

Critical bug: the old bad_ = outstanding < *locked; on line 416 was never removed, so it overwrites bad_ unconditionally before the new |= guarded logic runs — the fix is a no-op. The other two invariant checks (NoXRPTrustLines, NoDeepFreezeTrustLinesWithoutFreeze) correctly dropped the old assignment; this one did not. See inline comment.

Review by Claude Opus 4.6 · Prompt: V15

- Brings over a function overload from `develop` that the new test uses to run with different amendments

xrplf-ai-reviewer

ODR violation flagged inline — function body in header needs inline.

Review by ReviewBot 🤖

Review by Claude Opus 4.6 · Prompt: V15

xrplf-ai-reviewer

No issues.

Review by Claude Opus 4.6 · Prompt: V15

github-actions · 2026-04-17T21:13:52Z

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

github-actions · 2026-04-20T21:29:54Z

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

…invariant-bugfix-313 * XRPLF/ripple/staging-313: fix: Improve invariant check for Permissioned Domains (6134) (6958)

github-actions · 2026-04-20T21:49:33Z

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

xrplf-ai-reviewer

Compilation error from split string literal — see inline comment.

Review by ReviewBot 🤖

Review by Claude Opus 4.6 · Prompt: V15

vlntb

The change looks correct.
Happy to approve once the erroneous line split spotter by xrplf-ai-reviewer is fixed.

pratikmankawde

LGTM

pratikmankawde · 2026-04-21T13:06:15Z

        using namespace test::jtx;

-        Env env{*this, testable_amendments() | featureSingleAssetVault};
+        Env env{*this, testable_amendments()};


Do we need to clean | featureSingleAssetVault at other places in this file and other files as well?

Do we need to clean | featureSingleAssetVault at other places in this file and other files as well?

No. This one slipped through, and while it doesn't break anything, I'm not a fan.

…iminez/invariant-bugfix-313

xrplf-ai-reviewer

No issues.

Review by Claude Opus 4.6 · Prompt: V15

Copilot

Pull request overview

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

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

xrplf-ai-reviewer

No issues.

Review by Claude Opus 4.6 · Prompt: V15

Copilot

Pull request overview

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

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

Co-authored-by: Vito <[email protected]>

ximinez mentioned this pull request Apr 16, 2026

fix: Prevents overwriting a bool value in an invariant #6609

Merged

4 tasks

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

ximinez force-pushed the ximinez/invariant-bugfix-313 branch from 85985ba to 43daf4e Compare April 16, 2026 22:34

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

ximinez added this to the 3.1.3 milestone Apr 16, 2026

ximinez changed the title ~~fix: prevents overwritting a bool value in an invariant (#6609)~~ fix: Prevents overwritting a bool value in an invariant (#6609) Apr 16, 2026

ximinez requested review from Copilot, mvadari and pratikmankawde April 16, 2026 22:51

ximinez marked this pull request as ready for review April 16, 2026 22:51

Copilot started reviewing on behalf of ximinez April 16, 2026 22:52 View session

xrplf-ai-reviewer Bot reviewed Apr 16, 2026

View reviewed changes

Comment thread src/xrpld/app/tx/detail/InvariantCheck.cpp Outdated

Comment thread src/xrpld/app/tx/detail/InvariantCheck.cpp

Copilot AI reviewed Apr 16, 2026

View reviewed changes

Comment thread src/test/app/Invariants_test.cpp Outdated

Comment thread src/test/app/Invariants_test.cpp Outdated

Comment thread src/test/app/Vault_test.cpp Outdated

Comment thread include/xrpl/protocol/Rules.h Outdated

Comment thread src/xrpld/app/tx/detail/InvariantCheck.cpp Outdated

fix: Prevents overwritting a bool value in an invariant

7e12277

ximinez force-pushed the ximinez/invariant-bugfix-313 branch from 43daf4e to 7e12277 Compare April 16, 2026 22:58

ximinez marked this pull request as draft April 16, 2026 22:59

xrplf-ai-reviewer Bot reviewed Apr 16, 2026

View reviewed changes

Comment thread src/xrpld/app/tx/detail/InvariantCheck.cpp Outdated

ximinez added 2 commits April 16, 2026 19:05

AI review feedback: function overload, merge error, clarify comment

01b98df

Restore unintentionally deleted testInvariantOverwrite unit test

94034cc

ximinez marked this pull request as ready for review April 17, 2026 16:45

Fix invariant test compilation

651b6ee

- Brings over a function overload from `develop` that the new test uses to run with different amendments

xrplf-ai-reviewer Bot reviewed Apr 17, 2026

View reviewed changes

Comment thread include/xrpl/protocol/Rules.h

AI review feedback: mark header function as inline

ffa8661

ximinez requested a review from vlntb April 17, 2026 20:48

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

View reviewed changes

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

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

Merge remote-tracking branch 'XRPLF/ripple/staging-313' into ximinez/…

d64c2e7

…invariant-bugfix-313 * XRPLF/ripple/staging-313: fix: Improve invariant check for Permissioned Domains (6134) (6958)

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

xrplf-ai-reviewer Bot reviewed Apr 20, 2026

View reviewed changes

Comment thread src/test/app/Invariants_test.cpp Outdated

vlntb approved these changes Apr 21, 2026 •

edited

Loading

View reviewed changes

vlntb self-requested a review April 21, 2026 11:49

vlntb requested changes Apr 21, 2026

View reviewed changes

pratikmankawde approved these changes Apr 21, 2026

View reviewed changes

pratikmankawde reviewed Apr 21, 2026

View reviewed changes

fixup! Merge remote-tracking branch 'XRPLF/ripple/staging-313' into x…

21ffecb

…iminez/invariant-bugfix-313

ximinez requested a review from vlntb April 21, 2026 14:42

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

View reviewed changes

vlntb approved these changes Apr 21, 2026

View reviewed changes

bthomee requested a review from Copilot April 21, 2026 17:32

Copilot started reviewing on behalf of bthomee April 21, 2026 17:33 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

Comment thread include/xrpl/protocol/Rules.h Outdated

Comment thread src/libxrpl/protocol/Rules.cpp

fixup! AI review feedback: mark header function as inline

4097950

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

View reviewed changes

ximinez 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

bthomee requested a review from Copilot April 22, 2026 00:15

Copilot started reviewing on behalf of bthomee April 22, 2026 00:15 View session

Copilot AI reviewed Apr 22, 2026

View reviewed changes

Comment thread src/test/app/AMM_test.cpp

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

Merged via the queue into ripple/staging-313 with commit 31d4d08 Apr 22, 2026
3 checks passed

bthomee deleted the ximinez/invariant-bugfix-313 branch April 22, 2026 13:14

ximinez added a commit that referenced this pull request Apr 22, 2026

fix: Prevents overwritting a bool value in an invariant (#6609) (#6961)

2794a75

Co-authored-by: Vito <[email protected]>

LeJamon mentioned this pull request Jul 7, 2026

Rippled 3.1.3 parity — tracking issue (fixCleanup3_1_3 + fix wave) LeJamon/go-xrpl#1248

Open

32 tasks

Uh oh!

Conversation

ximinez commented Apr 16, 2026

High Level Overview of Change

Uh oh!

github-actions Bot commented Apr 16, 2026

Uh oh!

github-actions Bot commented Apr 16, 2026

Uh oh!

ximinez commented Apr 16, 2026

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 17, 2026

Uh oh!

github-actions Bot commented Apr 20, 2026

Uh oh!

github-actions Bot commented Apr 20, 2026

Uh oh!

xrplf-ai-reviewer Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vlntb left a comment

Choose a reason for hiding this comment

Uh oh!

pratikmankawde left a comment

Choose a reason for hiding this comment

Uh oh!

pratikmankawde Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

ximinez Apr 21, 2026

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

xrplf-ai-reviewer Bot left a comment

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!

Uh oh!

Uh oh!

Reviewers

Assignees