From 311719daeb10887eca3f4b049e46013cee68fd43 Mon Sep 17 00:00:00 2001 From: Vito <5780819+Tapanito@users.noreply.github.com> Date: Sat, 21 Mar 2026 14:36:35 +0100 Subject: [PATCH] fix: amendment-gate invariant bool overwrite fix (fixInvariantOverwrite) Three invariant checks used = instead of |= to accumulate their boolean flag across multiple visited entries. A later "good" entry could overwrite a violation detected by an earlier "bad" entry. Gate the corrected |= behavior behind fixInvariantOverwrite to preserve pre-amendment consensus. Each affected class tracks both the fixed (|=) and legacy (=) values; finalize picks which to use based on the amendment. Affected invariants: - NoZeroEscrow (bad_ for MPT locked vs outstanding) - NoXRPTrustLines (xrpTrustLine_) - NoDeepFreezeTrustLinesWithoutFreeze (deepFreezeWithoutFreeze_) --- include/xrpl/protocol/detail/features.macro | 1 + include/xrpl/tx/invariants/InvariantCheck.h | 3 ++ src/libxrpl/tx/invariants/InvariantCheck.cpp | 43 +++++++++++++++----- src/test/app/Invariants_test.cpp | 6 +-- 4 files changed, 40 insertions(+), 13 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index a40d524c70..fa07b9c2c3 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -16,6 +16,7 @@ // Add new amendments to the top of this list. // Keep it sorted in reverse chronological order. +XRPL_FIX (InvariantOverwrite, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (PermissionedDomainInvariant, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (ExpiredNFTokenOfferRemoval, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (BatchInnerSigs, Supported::no, VoteBehavior::DefaultNo) diff --git a/include/xrpl/tx/invariants/InvariantCheck.h b/include/xrpl/tx/invariants/InvariantCheck.h index e0ad65f14c..cdc2d5f847 100644 --- a/include/xrpl/tx/invariants/InvariantCheck.h +++ b/include/xrpl/tx/invariants/InvariantCheck.h @@ -227,6 +227,7 @@ public: class NoXRPTrustLines { bool xrpTrustLine_ = false; + bool xrpTrustLineLegacy_ = false; public: void @@ -246,6 +247,7 @@ public: class NoDeepFreezeTrustLinesWithoutFreeze { bool deepFreezeWithoutFreeze_ = false; + bool deepFreezeWithoutFreezeLegacy_ = false; public: void @@ -281,6 +283,7 @@ public: class NoZeroEscrow { bool bad_ = false; + bool badLegacy_ = false; public: void diff --git a/src/libxrpl/tx/invariants/InvariantCheck.cpp b/src/libxrpl/tx/invariants/InvariantCheck.cpp index 15af8a486d..4cd58da64b 100644 --- a/src/libxrpl/tx/invariants/InvariantCheck.cpp +++ b/src/libxrpl/tx/invariants/InvariantCheck.cpp @@ -306,14 +306,25 @@ NoZeroEscrow::visitEntry( }; if (before && before->getType() == ltESCROW) - bad_ |= isBad((*before)[sfAmount]); + { + auto const b = isBad((*before)[sfAmount]); + bad_ |= b; + badLegacy_ |= b; + } if (after && after->getType() == ltESCROW) - bad_ |= isBad((*after)[sfAmount]); + { + auto const b = isBad((*after)[sfAmount]); + bad_ |= b; + badLegacy_ |= b; + } auto checkAmount = [this](std::int64_t amount) { if (amount > maxMPTokenAmount || amount < 0) + { bad_ = true; + badLegacy_ = true; + } }; if (after && after->getType() == ltMPTOKEN_ISSUANCE) @@ -324,6 +335,7 @@ NoZeroEscrow::visitEntry( { checkAmount(*locked); bad_ |= outstanding < *locked; + badLegacy_ = outstanding < *locked; } } @@ -346,7 +358,9 @@ NoZeroEscrow::finalize( ReadView const& rv, beast::Journal const& j) const { - if (bad_) + bool const effectiveBad = rv.rules().enabled(fixInvariantOverwrite) ? bad_ : badLegacy_; + + if (effectiveBad) { JLOG(j.fatal()) << "Invariant failed: escrow specifies invalid amount"; return false; @@ -598,8 +612,10 @@ NoXRPTrustLines::visitEntry( // checking the issue directly here instead of // relying on .native() just in case native somehow // were systematically incorrect - xrpTrustLine_ |= after->getFieldAmount(sfLowLimit).issue() == xrpIssue() || + bool const isXrp = after->getFieldAmount(sfLowLimit).issue() == xrpIssue() || after->getFieldAmount(sfHighLimit).issue() == xrpIssue(); + xrpTrustLine_ |= isXrp; + xrpTrustLineLegacy_ = isXrp; } } @@ -608,10 +624,13 @@ NoXRPTrustLines::finalize( STTx const&, TER const, XRPAmount const, - ReadView const&, + ReadView const& rv, beast::Journal const& j) const { - if (!xrpTrustLine_) + bool const bad = + rv.rules().enabled(fixInvariantOverwrite) ? xrpTrustLine_ : xrpTrustLineLegacy_; + + if (!bad) return true; JLOG(j.fatal()) << "Invariant failed: an XRP trust line was created"; @@ -635,8 +654,9 @@ NoDeepFreezeTrustLinesWithoutFreeze::visitEntry( bool const highFreeze = uFlags & lsfHighFreeze; bool const highDeepFreeze = uFlags & lsfHighDeepFreeze; - deepFreezeWithoutFreeze_ |= - (lowDeepFreeze && !lowFreeze) || (highDeepFreeze && !highFreeze); + bool const bad = (lowDeepFreeze && !lowFreeze) || (highDeepFreeze && !highFreeze); + deepFreezeWithoutFreeze_ |= bad; + deepFreezeWithoutFreezeLegacy_ = bad; } } @@ -645,10 +665,13 @@ NoDeepFreezeTrustLinesWithoutFreeze::finalize( STTx const&, TER const, XRPAmount const, - ReadView const&, + ReadView const& rv, beast::Journal const& j) const { - if (!deepFreezeWithoutFreeze_) + bool const bad = rv.rules().enabled(fixInvariantOverwrite) ? deepFreezeWithoutFreeze_ + : deepFreezeWithoutFreezeLegacy_; + + if (!bad) return true; JLOG(j.fatal()) << "Invariant failed: a trust line with deep freeze flag " diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index 8de06c01cc..fad172f2c4 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -41,7 +41,7 @@ class Invariants_test : public beast::unit_test::suite defaultAmendments() { return xrpl::test::jtx::testable_amendments() | featureInvariantsV1_1 | - featureSingleAssetVault; + featureSingleAssetVault | fixInvariantOverwrite; } /** Run a specific test case to put the ledger into a state that will be @@ -987,9 +987,9 @@ class Invariants_test : public beast::unit_test::suite MPTIssue const mpt{MPTIssue{makeMptID(1, AccountID(0x4985601))}}; auto sleNew = std::make_shared(keylet::mptIssuance(mpt.getMptID())); - // outstanding exceeds maxMPTokenAmount → checkAmount sets bad_ + // outstanding exceeds maxMPTokenAmount -> checkAmount sets bad_ sleNew->setFieldU64(sfOutstandingAmount, maxMPTokenAmount + 1); - // locked is valid and <= outstanding → must NOT clear bad_ + // locked is valid and <= outstanding -> must NOT clear bad_ sleNew->setFieldU64(sfLockedAmount, 10); ac.view().insert(sleNew); return true;