From 31180f94c23476626703ae6e47c842bdd171104e Mon Sep 17 00:00:00 2001 From: Vito Tumas <5780819+Tapanito@users.noreply.github.com> Date: Thu, 30 Apr 2026 15:05:09 +0200 Subject: [PATCH 1/2] fix: Prevents overwriting a bool value in an invariant (#6609) Co-authored-by: Ed Hennis --- include/xrpl/protocol/Rules.h | 15 ++ src/libxrpl/protocol/Rules.cpp | 12 +- src/libxrpl/tx/invariants/InvariantCheck.cpp | 41 ++++- src/test/app/AMM_test.cpp | 4 +- src/test/app/Batch_test.cpp | 3 +- src/test/app/Invariants_test.cpp | 161 +++++++++++++++++-- src/test/app/LoanBroker_test.cpp | 4 +- src/test/app/Loan_test.cpp | 4 +- src/test/app/Vault_test.cpp | 45 +++--- src/test/rpc/GatewayBalances_test.cpp | 4 +- 10 files changed, 240 insertions(+), 53 deletions(-) diff --git a/include/xrpl/protocol/Rules.h b/include/xrpl/protocol/Rules.h index 7faf602cfd..604bb24700 100644 --- a/include/xrpl/protocol/Rules.h +++ b/include/xrpl/protocol/Rules.h @@ -8,6 +8,21 @@ namespace xrpl { +/** Check whether a feature is enabled in the current ledger rules + * + * @param feature The feature to be tested. + * @param resultIfNoRules What to return if called from outside a Transactor context. + */ +bool +isFeatureEnabled(uint256 const& feature, bool resultIfNoRules); + +/** Check whether a feature is enabled in the current ledger rules + * + * @param feature The feature to be tested. + * + * Returns false if no global Rules object is available. i.e. Outside of + * a Transactor context + */ bool isFeatureEnabled(uint256 const& feature); diff --git a/src/libxrpl/protocol/Rules.cpp b/src/libxrpl/protocol/Rules.cpp index 3e4649a27f..cf261a631b 100644 --- a/src/libxrpl/protocol/Rules.cpp +++ b/src/libxrpl/protocol/Rules.cpp @@ -141,10 +141,18 @@ Rules::operator!=(Rules const& other) const } bool -isFeatureEnabled(uint256 const& feature) +isFeatureEnabled(uint256 const& feature, bool resultIfNoRules) { auto const& rules = getCurrentTransactionRules(); - return rules && rules->enabled(feature); + if (!rules) + return resultIfNoRules; + return rules->enabled(feature); +} + +bool +isFeatureEnabled(uint256 const& feature) +{ + return isFeatureEnabled(feature, false); } } // namespace xrpl diff --git a/src/libxrpl/tx/invariants/InvariantCheck.cpp b/src/libxrpl/tx/invariants/InvariantCheck.cpp index e97501f129..2f12c7ce1e 100644 --- a/src/libxrpl/tx/invariants/InvariantCheck.cpp +++ b/src/libxrpl/tx/invariants/InvariantCheck.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -337,9 +338,11 @@ NoZeroEscrow::visitEntry( auto checkAmount = [this](std::int64_t amount) { if (amount > maxMPTokenAmount || amount < 0) - bad_ = true; + bad_ |= true; }; + bool const overwriteFixEnabled = isFeatureEnabled(fixSecurity3_1_3, true); + if (after && after->getType() == ltMPTOKEN_ISSUANCE) { auto const outstanding = (*after)[sfOutstandingAmount]; @@ -347,7 +350,15 @@ NoZeroEscrow::visitEntry( if (auto const locked = (*after)[~sfLockedAmount]) { checkAmount(*locked); - bad_ = outstanding < *locked; + bool const isBad = outstanding < *locked; + if (overwriteFixEnabled) + { + bad_ |= isBad; + } + else + { + bad_ = isBad; + } } } @@ -367,7 +378,7 @@ NoZeroEscrow::finalize( STTx const& txn, TER const, XRPAmount const, - ReadView const& rv, + ReadView const&, beast::Journal const& j) const { if (bad_) @@ -617,13 +628,23 @@ NoXRPTrustLines::visitEntry( std::shared_ptr const&, std::shared_ptr const& after) { + bool const overwriteFixEnabled = isFeatureEnabled(fixSecurity3_1_3, true); + if (after && after->getType() == ltRIPPLE_STATE) { // checking the issue directly here instead of // relying on .native() just in case native somehow // were systematically incorrect - xrpTrustLine_ = after->getFieldAmount(sfLowLimit).asset() == xrpIssue() || + bool const isXrp = after->getFieldAmount(sfLowLimit).asset() == xrpIssue() || after->getFieldAmount(sfHighLimit).asset() == xrpIssue(); + if (overwriteFixEnabled) + { + xrpTrustLine_ |= isXrp; + } + else + { + xrpTrustLine_ = isXrp; + } } } @@ -652,6 +673,8 @@ NoDeepFreezeTrustLinesWithoutFreeze::visitEntry( { if (after && after->getType() == ltRIPPLE_STATE) { + bool const overwriteFixEnabled = isFeatureEnabled(fixSecurity3_1_3, true); + std::uint32_t const uFlags = after->getFieldU32(sfFlags); bool const lowFreeze = (uFlags & lsfLowFreeze) != 0u; bool const lowDeepFreeze = (uFlags & lsfLowDeepFreeze) != 0u; @@ -659,7 +682,15 @@ NoDeepFreezeTrustLinesWithoutFreeze::visitEntry( bool const highFreeze = (uFlags & lsfHighFreeze) != 0u; bool const highDeepFreeze = (uFlags & lsfHighDeepFreeze) != 0u; - deepFreezeWithoutFreeze_ = (lowDeepFreeze && !lowFreeze) || (highDeepFreeze && !highFreeze); + bool const bad = (lowDeepFreeze && !lowFreeze) || (highDeepFreeze && !highFreeze); + if (overwriteFixEnabled) + { + deepFreezeWithoutFreeze_ |= bad; + } + else + { + deepFreezeWithoutFreeze_ = bad; + } } } diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index d79bd667dd..b25705becc 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -101,7 +101,7 @@ private: {}, 0, {}, - {testable_amendments() | featureSingleAssetVault}); + {testable_amendments()}); #endif // XRP to IOU, without featureSingleAssetVault @@ -6733,7 +6733,7 @@ private: }; testCase("tecDUPLICATE", testable_amendments() - featureSingleAssetVault); - testCase("terADDRESS_COLLISION", testable_amendments() | featureSingleAssetVault); + testCase("terADDRESS_COLLISION", testable_amendments()); } void diff --git a/src/test/app/Batch_test.cpp b/src/test/app/Batch_test.cpp index 6d8e1f2de3..56d398f8f2 100644 --- a/src/test/app/Batch_test.cpp +++ b/src/test/app/Batch_test.cpp @@ -2663,8 +2663,7 @@ class Batch_test : public beast::unit_test::suite using namespace test::jtx; - test::jtx::Env env{ - *this, features | featureSingleAssetVault | featureLendingProtocol | featureMPTokensV1}; + test::jtx::Env env{*this, features}; Account const issuer{"issuer"}; // For simplicity, lender will be the sole actor for the vault & diff --git a/src/test/app/Invariants_test.cpp b/src/test/app/Invariants_test.cpp index c7fe90daa7..36ba3e98ec 100644 --- a/src/test/app/Invariants_test.cpp +++ b/src/test/app/Invariants_test.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -93,8 +94,7 @@ class Invariants_test : public beast::unit_test::suite static FeatureBitset defaultAmendments() { - return xrpl::test::jtx::testable_amendments() | featureInvariantsV1_1 | - featureSingleAssetVault; + return xrpl::test::jtx::testable_amendments() | featureInvariantsV1_1 | fixSecurity3_1_3; } /** Run a specific test case to put the ledger into a state that will be @@ -180,6 +180,10 @@ class Invariants_test : public beast::unit_test::suite beast::Journal const jlog{sink}; ApplyContext ac{env.app(), ov, tx, tesSUCCESS, env.current()->fees().base, tapNONE, jlog}; + // Invariants normally run in the Transaction's "apply" (operator()) context, and can always + // access global Rules. + CurrentTransactionRulesGuard const rg(ov.rules()); + BEAST_EXPECT(precheck(A1, A2, ac)); auto transactor = makeTransactor(ac); @@ -934,7 +938,7 @@ class Invariants_test : public beast::unit_test::suite return false; auto sleNew = std::make_shared(keylet::escrow(A1, (*sle)[sfSequence] + 2)); - MPTIssue const mpt{MPTIssue{makeMptID(1, AccountID(0x4985601))}}; + MPTIssue const mpt{makeMptID(1, AccountID(0x4985601))}; STAmount const amt(mpt, -1); sleNew->setFieldAmount(sfAmount, amt); ac.view().insert(sleNew); @@ -950,7 +954,7 @@ class Invariants_test : public beast::unit_test::suite if (!sle) return false; - MPTIssue const mpt{MPTIssue{makeMptID(1, AccountID(0x4985601))}}; + MPTIssue const mpt{makeMptID(1, AccountID(0x4985601))}; auto sleNew = std::make_shared(keylet::mptIssuance(mpt.getMptID())); sleNew->setFieldU64(sfOutstandingAmount, -1); ac.view().insert(sleNew); @@ -966,7 +970,7 @@ class Invariants_test : public beast::unit_test::suite if (!sle) return false; - MPTIssue const mpt{MPTIssue{makeMptID(1, AccountID(0x4985601))}}; + MPTIssue const mpt{makeMptID(1, AccountID(0x4985601))}; auto sleNew = std::make_shared(keylet::mptIssuance(mpt.getMptID())); sleNew->setFieldU64(sfLockedAmount, -1); ac.view().insert(sleNew); @@ -982,7 +986,7 @@ class Invariants_test : public beast::unit_test::suite if (!sle) return false; - MPTIssue const mpt{MPTIssue{makeMptID(1, AccountID(0x4985601))}}; + MPTIssue const mpt{makeMptID(1, AccountID(0x4985601))}; auto sleNew = std::make_shared(keylet::mptIssuance(mpt.getMptID())); sleNew->setFieldU64(sfOutstandingAmount, 1); sleNew->setFieldU64(sfLockedAmount, 10); @@ -999,7 +1003,7 @@ class Invariants_test : public beast::unit_test::suite if (!sle) return false; - MPTIssue const mpt{MPTIssue{makeMptID(1, AccountID(0x4985601))}}; + MPTIssue const mpt{makeMptID(1, AccountID(0x4985601))}; auto sleNew = std::make_shared(keylet::mptoken(mpt.getMptID(), A1)); sleNew->setFieldU64(sfMPTAmount, -1); ac.view().insert(sleNew); @@ -1015,7 +1019,7 @@ class Invariants_test : public beast::unit_test::suite if (!sle) return false; - MPTIssue const mpt{MPTIssue{makeMptID(1, AccountID(0x4985601))}}; + MPTIssue const mpt{makeMptID(1, AccountID(0x4985601))}; auto sleNew = std::make_shared(keylet::mptoken(mpt.getMptID(), A1)); sleNew->setFieldU64(sfLockedAmount, -1); ac.view().insert(sleNew); @@ -3968,7 +3972,7 @@ class Invariants_test : public beast::unit_test::suite if (!sle) return false; - MPTIssue const mpt{MPTIssue{makeMptID(sle->getFieldU32(sfSequence), A1)}}; + MPTIssue const mpt{makeMptID(sle->getFieldU32(sfSequence), A1)}; auto sleNew = std::make_shared(keylet::mptIssuance(mpt.getMptID())); sleNew->setFieldU64(sfOutstandingAmount, 110); sleNew->setFieldU64(sfMaximumAmount, 100); @@ -3985,7 +3989,7 @@ class Invariants_test : public beast::unit_test::suite if (!sle) return false; - MPTIssue const mpt{MPTIssue{makeMptID(sle->getFieldU32(sfSequence), A1)}}; + MPTIssue const mpt{makeMptID(sle->getFieldU32(sfSequence), A1)}; auto sleNew = std::make_shared(keylet::mptIssuance(mpt.getMptID())); sleNew->setFieldU64(sfOutstandingAmount, 100); sleNew->setFieldU64(sfMaximumAmount, 100); @@ -4056,7 +4060,7 @@ class Invariants_test : public beast::unit_test::suite auto seq = sle->getFieldU32(sfSequence); for (int i = 0; i < nTokens; ++i) { - MPTIssue const mpt{MPTIssue{makeMptID(seq + i, A1)}}; + MPTIssue const mpt{makeMptID(seq + i, A1)}; auto sleNew = std::make_shared(keylet::mptIssuance(mpt.getMptID())); ac.view().insert(sleNew); @@ -4101,6 +4105,139 @@ class Invariants_test : public beast::unit_test::suite } } + // Test the invariant overwrite fix for both pre- and post-amendment + // behavior. With the fix enabled, |= accumulates violations across + // entries so a later valid entry cannot clear an earlier violation. + // Without the fix, = assignment means the last-visited entry wins. + void + testInvariantOverwrite(FeatureBitset features) + { + using namespace test::jtx; + bool const fixEnabled = features[fixSecurity3_1_3]; + std::initializer_list const failTers = {tecINVARIANT_FAILED, tefINVARIANT_FAILED}; + std::initializer_list const passTers = {tesSUCCESS, tesSUCCESS}; + + // Insert two trust line SLEs in hash-sorted order, with the "bad" + // entry at the lower-sorting key so it is visited first by + // ApplyStateTable::visit(). The configurer callables receive the + // SLE and the Issue corresponding to that side's keylet currency. + auto const insertOrderedTrustLinePair = [](ApplyContext& ac, + Account const& A1, + Account const& A2, + Account const& A3, + auto const& badConfig, + auto const& goodConfig) { + char const* const c1 = "USD"; + char const* const c2 = "EUR"; + auto const k1 = keylet::line(A1, A2, A1[c1].currency); + auto const k2 = keylet::line(A1, A3, A1[c2].currency); + + bool const k1First = k1.key < k2.key; + auto const& badKey = k1First ? k1 : k2; + auto const& goodKey = k1First ? k2 : k1; + Issue const badIss{k1First ? A1[c1].currency : A1[c2].currency, A1.id()}; + Issue const goodIss{k1First ? A1[c2].currency : A1[c1].currency, A1.id()}; + + auto const sleBad = std::make_shared(badKey); + badConfig(*sleBad, badIss); + ac.view().insert(sleBad); + + auto const sleGood = std::make_shared(goodKey); + goodConfig(*sleGood, goodIss); + ac.view().insert(sleGood); + }; + + // Regression: bad XRP trust line followed by a valid trust line. + // With the fix, the invariant catches the violation. Without it, + // the valid entry overwrites the flag to false. The keylet + // currencies are non-XRP (the invariant inspects sfLowLimit / + // sfHighLimit issue, not the keylet currency). + testcase << "overwrite: NoXRPTrustLines" + std::string(fixEnabled ? " fix" : ""); + doInvariantCheck( + Env(*this, features), + fixEnabled ? std::vector{{"an XRP trust line was created"}} + : std::vector{}, + [&insertOrderedTrustLinePair](Account const& A1, Account const& A2, ApplyContext& ac) { + Account const A3{"A3"}; + insertOrderedTrustLinePair( + ac, + A1, + A2, + A3, + [](SLE& sle, Issue const& iss) { + // sfLowLimit has xrpIssue, making isXrp = true + sle.setFieldAmount(sfLowLimit, STAmount{xrpIssue(), 0}); + sle.setFieldAmount(sfHighLimit, STAmount{iss, 0}); + }, + [](SLE& sle, Issue const& iss) { + sle.setFieldAmount(sfLowLimit, STAmount{iss, 0}); + sle.setFieldAmount(sfHighLimit, STAmount{iss, 0}); + }); + return true; + }, + XRPAmount{}, + STTx{ttACCOUNT_SET, [](STObject&) {}}, + fixEnabled ? failTers : passTers); + + // Regression: bad deep-freeze trust line followed by a valid one. + testcase << "overwrite: NoDeepFreeze" + std::string(fixEnabled ? " fix" : ""); + doInvariantCheck( + Env(*this, features), + fixEnabled ? std::vector{{"a trust line with deep freeze flag without " + "normal freeze was created"}} + : std::vector{}, + [&insertOrderedTrustLinePair](Account const& A1, Account const& A2, ApplyContext& ac) { + Account const A3{"A3"}; + insertOrderedTrustLinePair( + ac, + A1, + A2, + A3, + [](SLE& sle, Issue const& iss) { + sle.setFieldAmount(sfLowLimit, STAmount{iss, 0}); + sle.setFieldAmount(sfHighLimit, STAmount{iss, 0}); + sle.setFieldU32(sfFlags, lsfLowDeepFreeze); + }, + [](SLE& sle, Issue const& iss) { + sle.setFieldAmount(sfLowLimit, STAmount{iss, 0}); + sle.setFieldAmount(sfHighLimit, STAmount{iss, 0}); + sle.setFieldU32(sfFlags, 0u); + }); + return true; + }, + XRPAmount{}, + STTx{ttACCOUNT_SET, [](STObject&) {}}, + fixEnabled ? failTers : passTers); + + // Regression: MPT OutstandingAmount exceeds max, but locked <= + // outstanding. Plain assignment would overwrite bad_ = true. + // With the fix, NoZeroEscrow catches it. + // Without the fix, NoZeroEscrow passes but ValidMPTIssuance + // still fires ("a MPT issuance was created"). + testcase << "overwrite: NoZeroEscrow MPT" + std::string(fixEnabled ? " fix" : ""); + doInvariantCheck( + Env(*this, features), + fixEnabled ? std::vector{{"escrow specifies invalid amount"}} + : std::vector{{"a MPT issuance was created"}}, + [](Account const& A1, Account const&, ApplyContext& ac) { + auto const sle = ac.view().peek(keylet::account(A1.id())); + if (!sle) + return false; + + MPTIssue const mpt{makeMptID(1, AccountID(0x4985601))}; + auto sleNew = std::make_shared(keylet::mptIssuance(mpt.getMptID())); + // outstanding exceeds maxMPTokenAmount -> checkAmount sets bad_ + sleNew->setFieldU64(sfOutstandingAmount, maxMPTokenAmount + 1); + // locked is valid and <= outstanding -> must NOT clear bad_ + sleNew->setFieldU64(sfLockedAmount, 10); + ac.view().insert(sleNew); + return true; + }, + XRPAmount{}, + STTx{ttACCOUNT_SET, [](STObject&) {}}, + failTers); + } + void testVaultComputeCoarsestScale() { @@ -4253,6 +4390,8 @@ public: testValidLoanBroker(); testVault(); testMPT(); + testInvariantOverwrite(defaultAmendments()); + testInvariantOverwrite(defaultAmendments() - fixSecurity3_1_3); testVaultComputeCoarsestScale(); } }; diff --git a/src/test/app/LoanBroker_test.cpp b/src/test/app/LoanBroker_test.cpp index 1ffc34bdcc..3bdd346877 100644 --- a/src/test/app/LoanBroker_test.cpp +++ b/src/test/app/LoanBroker_test.cpp @@ -63,9 +63,7 @@ class LoanBroker_test : public beast::unit_test::suite { // Ensure that all the features needed for Lending Protocol are included, // even if they are set to unsupported. - FeatureBitset const all{ - jtx::testable_amendments() | featureMPTokensV1 | featureSingleAssetVault | - featureLendingProtocol}; + FeatureBitset const all{jtx::testable_amendments()}; void testDisabled() diff --git a/src/test/app/Loan_test.cpp b/src/test/app/Loan_test.cpp index f50a7f71d3..ad87603661 100644 --- a/src/test/app/Loan_test.cpp +++ b/src/test/app/Loan_test.cpp @@ -87,9 +87,7 @@ class Loan_test : public beast::unit_test::suite protected: // Ensure that all the features needed for Lending Protocol are included, // even if they are set to unsupported. - FeatureBitset const all{ - jtx::testable_amendments() | featureMPTokensV1 | featureSingleAssetVault | - featureLendingProtocol}; + FeatureBitset const all{jtx::testable_amendments()}; std::string const iouCurrency{"IOU"}; diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 0e6b680ff3..775d579b9b 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -536,7 +536,7 @@ class Vault_test : public beast::unit_test::suite auto testCases = [&, this]( std::string prefix, std::function setup) { - Env env{*this, testable_amendments() | featureSingleAssetVault}; + Env env{*this, testable_amendments()}; Vault vault{env}; env.fund(XRP(1000), issuer, owner, depositor, charlie, dave); @@ -589,7 +589,7 @@ class Vault_test : public beast::unit_test::suite struct CaseArgs { - FeatureBitset features = testable_amendments() | featureSingleAssetVault; + FeatureBitset features = testable_amendments(); }; auto testCase = [&, this]( @@ -802,8 +802,7 @@ class Vault_test : public beast::unit_test::suite env(tx, ter{temDISABLED}); } }, - {.features = - (testable_amendments() | featureSingleAssetVault) - featurePermissionedDomains}); + {.features = (testable_amendments()) - featurePermissionedDomains}); testCase([&](Env& env, Account const& issuer, @@ -1114,10 +1113,11 @@ class Vault_test : public beast::unit_test::suite Account const& depositor, Asset const& asset, Vault& vault)> test) { - Env env{*this, testable_amendments() | featureSingleAssetVault}; + Env env{*this, testable_amendments()}; Account const issuer{"issuer"}; Account const owner{"owner"}; Account const depositor{"depositor"}; + env.fund(XRP(1000), issuer, owner, depositor); env.close(); Vault vault{env}; @@ -1278,8 +1278,7 @@ class Vault_test : public beast::unit_test::suite { { testcase("IOU fail because MPT is disabled"); - Env env{ - *this, (testable_amendments() - featureMPTokensV1) | featureSingleAssetVault}; + Env env{*this, (testable_amendments() - featureMPTokensV1)}; Account const issuer{"issuer"}; Account const owner{"owner"}; env.fund(XRP(1000), issuer, owner); @@ -1295,7 +1294,7 @@ class Vault_test : public beast::unit_test::suite { testcase("IOU fail create frozen"); - Env env{*this, testable_amendments() | featureSingleAssetVault}; + Env env{*this, testable_amendments()}; Account const issuer{"issuer"}; Account const owner{"owner"}; env.fund(XRP(1000), issuer, owner); @@ -1313,7 +1312,7 @@ class Vault_test : public beast::unit_test::suite { testcase("IOU fail create no ripling"); - Env env{*this, testable_amendments() | featureSingleAssetVault}; + Env env{*this, testable_amendments()}; Account const issuer{"issuer"}; Account const owner{"owner"}; env.fund(XRP(1000), issuer, owner); @@ -1330,7 +1329,7 @@ class Vault_test : public beast::unit_test::suite { testcase("IOU no issuer"); - Env env{*this, testable_amendments() | featureSingleAssetVault}; + Env env{*this, testable_amendments()}; Account const issuer{"issuer"}; Account const owner{"owner"}; env.fund(XRP(1000), owner); @@ -1348,7 +1347,7 @@ class Vault_test : public beast::unit_test::suite { testcase("IOU fail create vault for AMM LPToken"); - Env env{*this, testable_amendments() | featureSingleAssetVault}; + Env env{*this, testable_amendments()}; Account const gw("gateway"); Account const alice("alice"); Account const carol("carol"); @@ -1410,7 +1409,7 @@ class Vault_test : public beast::unit_test::suite Account const& depositor, Asset const& asset, Vault& vault)> test) { - Env env{*this, testable_amendments() | featureSingleAssetVault}; + Env env{*this, testable_amendments()}; Account const issuer{"issuer"}; Account const owner{"owner"}; Account const depositor{"depositor"}; @@ -1469,7 +1468,7 @@ class Vault_test : public beast::unit_test::suite { using namespace test::jtx; - Env env{*this, testable_amendments() | featureSingleAssetVault}; + Env env{*this, testable_amendments()}; Account const issuer{"issuer"}; Account const owner{"owner"}; Account const depositor{"depositor"}; @@ -1582,7 +1581,7 @@ class Vault_test : public beast::unit_test::suite Vault& vault, MPTTester& mptt)> test, CaseArgs args = {}) { - Env env{*this, testable_amendments() | featureSingleAssetVault}; + Env env{*this, testable_amendments()}; Account const issuer{"issuer"}; Account const owner{"owner"}; Account const depositor{"depositor"}; @@ -2206,7 +2205,7 @@ class Vault_test : public beast::unit_test::suite { testcase("MPT shares to a vault"); - Env env{*this, testable_amendments() | featureSingleAssetVault}; + Env env{*this, testable_amendments()}; Account const owner{"owner"}; Account const issuer{"issuer"}; env.fund(XRP(1000000), owner, issuer); @@ -2338,7 +2337,7 @@ class Vault_test : public beast::unit_test::suite PrettyAsset const& asset, std::function issuanceId)> test, CaseArgs args = {}) { - Env env{*this, testable_amendments() | featureSingleAssetVault}; + Env env{*this, testable_amendments()}; Account const owner{"owner"}; Account const issuer{"issuer"}; Account const charlie{"charlie"}; @@ -3020,7 +3019,7 @@ class Vault_test : public beast::unit_test::suite testcase("private vault"); - Env env{*this, testable_amendments() | featureSingleAssetVault}; + Env env{*this, testable_amendments()}; Account const issuer{"issuer"}; Account const owner{"owner"}; Account const depositor{"depositor"}; @@ -3268,7 +3267,7 @@ class Vault_test : public beast::unit_test::suite testcase("private XRP vault"); - Env env{*this, testable_amendments() | featureSingleAssetVault}; + Env env{*this, testable_amendments()}; Account const owner{"owner"}; Account const depositor{"depositor"}; Account const alice{"charlie"}; @@ -3365,7 +3364,7 @@ class Vault_test : public beast::unit_test::suite using namespace test::jtx; testcase("fail pseudo-account allocation"); - Env env{*this, testable_amendments() | featureSingleAssetVault}; + Env env{*this, testable_amendments()}; Account const owner{"owner"}; Vault const vault{env}; env.fund(XRP(1000), owner); @@ -3408,7 +3407,7 @@ class Vault_test : public beast::unit_test::suite auto testCase = [&, this]( std::uint8_t scale, std::function test) { - Env env{*this, testable_amendments() | featureSingleAssetVault}; + Env env{*this, testable_amendments()}; Account const owner{"owner"}; Account const issuer{"issuer"}; Account const depositor{"depositor"}; @@ -4241,7 +4240,7 @@ class Vault_test : public beast::unit_test::suite using namespace test::jtx; testcase("RPC"); - Env env{*this, testable_amendments() | featureSingleAssetVault}; + Env env{*this, testable_amendments()}; Account const owner{"owner"}; Account const issuer{"issuer"}; Vault const vault{env}; @@ -5369,7 +5368,7 @@ class Vault_test : public beast::unit_test::suite } }; - Account owner{"alice"}; + Account const owner{"alice"}; Account const depositor{"bob"}; Account const issuer{"issuer"}; @@ -5477,7 +5476,7 @@ class Vault_test : public beast::unit_test::suite using namespace test::jtx; - Env env{*this, testable_amendments() | featureSingleAssetVault}; + Env env{*this, testable_amendments()}; Account const owner{"owner"}; Account const issuer{"issuer"}; diff --git a/src/test/rpc/GatewayBalances_test.cpp b/src/test/rpc/GatewayBalances_test.cpp index be51f60f02..95c7d425c1 100644 --- a/src/test/rpc/GatewayBalances_test.cpp +++ b/src/test/rpc/GatewayBalances_test.cpp @@ -239,8 +239,8 @@ public: using namespace std::chrono_literals; using namespace jtx; - // Ensure MPT is enabled - FeatureBitset const features = testable_amendments() | featureMPTokensV1; + // testable_amendments() includes MPT + FeatureBitset const features = testable_amendments(); Env env(*this, features); Account const alice{"alice"}; From c6053f5d640a66c21c5704b321ae427dc1a84b1b Mon Sep 17 00:00:00 2001 From: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> Date: Thu, 30 Apr 2026 14:33:33 +0100 Subject: [PATCH 2/2] fix: Gate -mcmodel flags to x86_64 in sanitizer builds (#7049) Signed-off-by: Pratik Mankawde <3397372+pratikmankawde@users.noreply.github.com> --- cmake/XrplSanitizers.cmake | 12 ++++++++---- conan/profiles/sanitizers | 9 +++++++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/cmake/XrplSanitizers.cmake b/cmake/XrplSanitizers.cmake index f9630f6856..2228381286 100644 --- a/cmake/XrplSanitizers.cmake +++ b/cmake/XrplSanitizers.cmake @@ -34,10 +34,12 @@ * -fsanitize=: Links sanitizer runtime libraries * -mcmodel=large/medium: (GCC only) Matches compile-time code model - - SANITIZERS_RELOCATION_FLAGS: (GCC only) Code model flags for linking. + - SANITIZERS_RELOCATION_FLAGS: (GCC only, x86_64 only) Code model flags for linking. Used to handle large instrumented binaries on x86_64: * -mcmodel=large: For AddressSanitizer (prevents relocation errors) * -mcmodel=medium: For ThreadSanitizer (large model is incompatible) + On ARM64, these flags are omitted since GCC does not support + -mcmodel=large with -fPIC, and -mcmodel=medium does not exist. #]===================================================================] include(CompilationEnv) @@ -151,9 +153,11 @@ if(is_gcc) elseif(enable_tsan) # GCC doesn't support atomic_thread_fence with tsan. Suppress warnings. list(APPEND SANITIZERS_COMPILE_FLAGS "-Wno-tsan") - message(STATUS " Using medium code model (-mcmodel=medium)") - list(APPEND SANITIZERS_COMPILE_FLAGS "-mcmodel=medium") - list(APPEND SANITIZERS_RELOCATION_FLAGS "-mcmodel=medium") + if(is_amd64) + message(STATUS " Using medium code model (-mcmodel=medium)") + list(APPEND SANITIZERS_COMPILE_FLAGS "-mcmodel=medium") + list(APPEND SANITIZERS_RELOCATION_FLAGS "-mcmodel=medium") + endif() endif() # Join sanitizer flags with commas for -fsanitize option diff --git a/conan/profiles/sanitizers b/conan/profiles/sanitizers index 073fcf3072..800e6eb48c 100644 --- a/conan/profiles/sanitizers +++ b/conan/profiles/sanitizers @@ -1,5 +1,6 @@ include(default) {% set compiler, version, compiler_exe = detect_api.detect_default_compiler() %} +{% set arch = detect_api.detect_arch() %} {% set sanitizers = os.getenv("SANITIZERS") %} [conf] @@ -13,12 +14,16 @@ include(default) {% if "address" in sanitizers %} {% set _ = sanitizer_list.append("address") %} - {% set model_code = "-mcmodel=large" %} + {% if arch == "x86_64" %} + {% set model_code = "-mcmodel=large" %} + {% endif %} {% set _ = defines.append("BOOST_USE_ASAN")%} {% set _ = defines.append("BOOST_USE_UCONTEXT")%} {% elif "thread" in sanitizers %} {% set _ = sanitizer_list.append("thread") %} - {% set model_code = "-mcmodel=medium" %} + {% if arch == "x86_64" %} + {% set model_code = "-mcmodel=medium" %} + {% endif %} {% set _ = extra_cxxflags.append("-Wno-tsan") %} {% set _ = defines.append("BOOST_USE_TSAN")%} {% set _ = defines.append("BOOST_USE_UCONTEXT")%}