From fa6991812471bdde0d771754e8b7e688d774c81f Mon Sep 17 00:00:00 2001 From: yinyiqian1 Date: Fri, 31 Oct 2025 15:01:06 -0400 Subject: [PATCH 1/9] fix: Address permission delegation vulnerability (#5825) This change introduces the `featurePermissionDelegationV1_1` amendment, which is designed to supersede both `featurePermissionDelegation` and `fixDelegateV1_1 amendments, which should be considered deprecated. The `checkPermission` function will now return `terNO_DELEGATE_PERMISSION` when a delegate transaction lacks the necessary permissions. --- include/xrpl/protocol/TER.h | 8 +- include/xrpl/protocol/detail/features.macro | 3 +- .../xrpl/protocol/detail/transactions.macro | 4 +- src/libxrpl/protocol/Permissions.cpp | 25 +- src/libxrpl/protocol/TER.cpp | 2 +- src/test/app/Batch_test.cpp | 3 +- src/test/app/Delegate_test.cpp | 349 ++++++++++-------- src/xrpld/app/misc/DelegateUtils.h | 4 +- src/xrpld/app/misc/detail/DelegateUtils.cpp | 6 +- src/xrpld/app/tx/detail/DelegateSet.cpp | 23 +- .../app/tx/detail/MPTokenIssuanceSet.cpp | 10 +- src/xrpld/app/tx/detail/MPTokenIssuanceSet.h | 2 +- src/xrpld/app/tx/detail/Payment.cpp | 45 +-- src/xrpld/app/tx/detail/Payment.h | 2 +- src/xrpld/app/tx/detail/SetAccount.cpp | 18 +- src/xrpld/app/tx/detail/SetAccount.h | 2 +- src/xrpld/app/tx/detail/SetTrust.cpp | 18 +- src/xrpld/app/tx/detail/SetTrust.h | 2 +- src/xrpld/app/tx/detail/Transactor.cpp | 6 +- src/xrpld/app/tx/detail/Transactor.h | 2 +- src/xrpld/app/tx/detail/applySteps.cpp | 52 +-- 21 files changed, 295 insertions(+), 291 deletions(-) diff --git a/include/xrpl/protocol/TER.h b/include/xrpl/protocol/TER.h index 0a3a3b999e..51d37e2fc7 100644 --- a/include/xrpl/protocol/TER.h +++ b/include/xrpl/protocol/TER.h @@ -225,8 +225,9 @@ enum TERcodes : TERUnderlyingType { terQUEUED, // Transaction is being held in TxQ until fee drops terPRE_TICKET, // Ticket is not yet in ledger but might be on its way terNO_AMM, // AMM doesn't exist for the asset pair - terADDRESS_COLLISION, // Failed to allocate AccountID when trying to - // create a pseudo-account + terADDRESS_COLLISION, // Failed to allocate AccountID when trying to + // create a pseudo-account + terNO_DELEGATE_PERMISSION, // Delegate does not have permission }; //------------------------------------------------------------------------------ @@ -361,6 +362,9 @@ enum TECcodes : TERUnderlyingType { tecLIMIT_EXCEEDED = 195, tecPSEUDO_ACCOUNT = 196, tecPRECISION_LOSS = 197, + // DEPRECATED: This error code tecNO_DELEGATE_PERMISSION is reserved for + // backward compatibility with historical data on non-prod networks, can be + // reclaimed after those networks reset. tecNO_DELEGATE_PERMISSION = 198, }; diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 6100c9c61b..d510edaa43 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -30,11 +30,11 @@ // Add new amendments to the top of this list. // Keep it sorted in reverse chronological order. +XRPL_FEATURE(PermissionDelegationV1_1, Supported::no, VoteBehavior::DefaultNo) XRPL_FIX (DirectoryLimit, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (IncludeKeyletFields, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(DynamicMPT, Supported::no, VoteBehavior::DefaultNo) XRPL_FIX (TokenEscrowV1, Supported::yes, VoteBehavior::DefaultNo) -XRPL_FIX (DelegateV1_1, Supported::no, VoteBehavior::DefaultNo) XRPL_FIX (PriceOracleOrder, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (MPTDeliveredAmount, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (AMMClawbackRounding, Supported::yes, VoteBehavior::DefaultNo) @@ -44,7 +44,6 @@ XRPL_FIX (AMMv1_3, Supported::yes, VoteBehavior::DefaultNo XRPL_FEATURE(PermissionedDEX, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(Batch, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(SingleAssetVault, Supported::no, VoteBehavior::DefaultNo) -XRPL_FEATURE(PermissionDelegation, Supported::no, VoteBehavior::DefaultNo) XRPL_FIX (PayChanCancelAfter, Supported::yes, VoteBehavior::DefaultNo) // Check flags in Credential transactions XRPL_FIX (InvalidTxFlags, Supported::yes, VoteBehavior::DefaultNo) diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index 119c3f8b7b..bfe5182469 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -316,7 +316,7 @@ TRANSACTION(ttTRUST_SET, 20, TrustSet, #endif TRANSACTION(ttACCOUNT_DELETE, 21, AccountDelete, Delegation::notDelegatable, - uint256{}, + featureDeletableAccounts, mustDeleteAcct, ({ {sfDestination, soeREQUIRED}, @@ -837,7 +837,7 @@ TRANSACTION(ttPERMISSIONED_DOMAIN_DELETE, 63, PermissionedDomainDelete, #endif TRANSACTION(ttDELEGATE_SET, 64, DelegateSet, Delegation::notDelegatable, - featurePermissionDelegation, + featurePermissionDelegationV1_1, noPriv, ({ {sfAuthorize, soeREQUIRED}, diff --git a/src/libxrpl/protocol/Permissions.cpp b/src/libxrpl/protocol/Permissions.cpp index c9e32c5056..2d5f00ef77 100644 --- a/src/libxrpl/protocol/Permissions.cpp +++ b/src/libxrpl/protocol/Permissions.cpp @@ -174,21 +174,22 @@ Permission::isDelegatable( auto const txType = permissionToTxType(permissionValue); auto const it = delegatableTx_.find(txType); - if (rules.enabled(fixDelegateV1_1)) - { - if (it == delegatableTx_.end()) - return false; + if (it == delegatableTx_.end()) + return false; - auto const feature = getTxFeature(txType); + auto const txFeaturesIt = txFeatureMap_.find(txType); + XRPL_ASSERT( + txFeaturesIt != txFeatureMap_.end(), + "ripple::Permissions::isDelegatable : tx exists in txFeatureMap_"); - // fixDelegateV1_1: Delegation is only allowed if the required amendment - // for the transaction is enabled. For transactions that do not require - // an amendment, delegation is always allowed. - if (feature && !rules.enabled(*feature)) - return false; - } + // Delegation is only allowed if the required amendment for the transaction + // is enabled. For transactions that do not require an amendment, delegation + // is always allowed. + if (txFeaturesIt->second != uint256{} && + !rules.enabled(txFeaturesIt->second)) + return false; - if (it != delegatableTx_.end() && it->second == Delegation::notDelegatable) + if (it->second == Delegation::notDelegatable) return false; return true; diff --git a/src/libxrpl/protocol/TER.cpp b/src/libxrpl/protocol/TER.cpp index a396949afe..d4716b6f66 100644 --- a/src/libxrpl/protocol/TER.cpp +++ b/src/libxrpl/protocol/TER.cpp @@ -127,7 +127,6 @@ transResults() MAKE_ERROR(tecLIMIT_EXCEEDED, "Limit exceeded."), MAKE_ERROR(tecPSEUDO_ACCOUNT, "This operation is not allowed against a pseudo-account."), MAKE_ERROR(tecPRECISION_LOSS, "The amounts used by the transaction cannot interact."), - MAKE_ERROR(tecNO_DELEGATE_PERMISSION, "Delegated account lacks permission to perform this transaction."), MAKE_ERROR(tefALREADY, "The exact transaction was already in this ledger."), MAKE_ERROR(tefBAD_ADD_AUTH, "Not authorized to add account."), @@ -235,6 +234,7 @@ transResults() MAKE_ERROR(terPRE_TICKET, "Ticket is not yet in ledger."), MAKE_ERROR(terNO_AMM, "AMM doesn't exist for the asset pair."), MAKE_ERROR(terADDRESS_COLLISION, "Failed to allocate an unique account address."), + MAKE_ERROR(terNO_DELEGATE_PERMISSION, "Delegated account lacks permission to perform this transaction."), MAKE_ERROR(tesSUCCESS, "The transaction was applied. Only final in a validated ledger."), }; diff --git a/src/test/app/Batch_test.cpp b/src/test/app/Batch_test.cpp index 92f286ca6a..c87d4b46d2 100644 --- a/src/test/app/Batch_test.cpp +++ b/src/test/app/Batch_test.cpp @@ -3946,14 +3946,13 @@ class Batch_test : public beast::unit_test::suite tesSUCCESS, batch::outer(gw, seq, batchFee, tfIndependent), batch::inner(jv1, seq + 1), - // tecNO_DELEGATE_PERMISSION: not authorized to clear freeze + // terNO_DELEGATE_PERMISSION: not authorized to clear freeze batch::inner(jv2, seq + 2)); env.close(); std::vector testCases = { {0, "Batch", "tesSUCCESS", batchID, std::nullopt}, {1, "TrustSet", "tesSUCCESS", txIDs[0], batchID}, - {2, "TrustSet", "tecNO_DELEGATE_PERMISSION", txIDs[1], batchID}, }; validateClosedLedger(env, testCases); } diff --git a/src/test/app/Delegate_test.cpp b/src/test/app/Delegate_test.cpp index ea5e073a55..fca0a7b249 100644 --- a/src/test/app/Delegate_test.cpp +++ b/src/test/app/Delegate_test.cpp @@ -27,23 +27,27 @@ namespace test { class Delegate_test : public beast::unit_test::suite { void - testFeatureDisabled() + testFeatureDisabled(FeatureBitset features) { - testcase("test featurePermissionDelegation not enabled"); + testcase("test feature not enabled"); using namespace jtx; - Env env{*this, testable_amendments() - featurePermissionDelegation}; + Env env{*this, features}; Account gw{"gateway"}; Account alice{"alice"}; Account bob{"bob"}; env.fund(XRP(1000000), gw, alice, bob); env.close(); + auto res = features[featurePermissionDelegationV1_1] ? ter(tesSUCCESS) + : ter(temDISABLED); + // can not set Delegate when feature disabled - env(delegate::set(gw, alice, {"Payment"}), ter(temDISABLED)); + env(delegate::set(gw, alice, {"Payment"}), res); + env.close(); // can not send delegating transaction when feature disabled - env(pay(alice, bob, XRP(100)), delegate::as(bob), ter(temDISABLED)); + env(pay(gw, bob, XRP(100)), delegate::as(alice), res); } void @@ -217,17 +221,16 @@ class Delegate_test : public beast::unit_test::suite } // non-delegatable transaction - auto const res = features[fixDelegateV1_1] ? ter(temMALFORMED) - : ter(tecNO_PERMISSION); { - env(delegate::set(gw, alice, {"SetRegularKey"}), res); - env(delegate::set(gw, alice, {"AccountSet"}), res); - env(delegate::set(gw, alice, {"SignerListSet"}), res); - env(delegate::set(gw, alice, {"DelegateSet"}), res); - env(delegate::set(gw, alice, {"EnableAmendment"}), res); - env(delegate::set(gw, alice, {"UNLModify"}), res); - env(delegate::set(gw, alice, {"SetFee"}), res); - env(delegate::set(gw, alice, {"Batch"}), res); + env(delegate::set(gw, alice, {"SetRegularKey"}), ter(temMALFORMED)); + env(delegate::set(gw, alice, {"AccountSet"}), ter(temMALFORMED)); + env(delegate::set(gw, alice, {"SignerListSet"}), ter(temMALFORMED)); + env(delegate::set(gw, alice, {"DelegateSet"}), ter(temMALFORMED)); + env(delegate::set(gw, alice, {"EnableAmendment"}), + ter(temMALFORMED)); + env(delegate::set(gw, alice, {"UNLModify"}), ter(temMALFORMED)); + env(delegate::set(gw, alice, {"SetFee"}), ter(temMALFORMED)); + env(delegate::set(gw, alice, {"Batch"}), ter(temMALFORMED)); } } @@ -305,10 +308,6 @@ class Delegate_test : public beast::unit_test::suite env.close(); { - // Fee should be checked before permission check, - // otherwise tecNO_DELEGATE_PERMISSION returned when permission - // check fails could cause context reset to pay fee because it is - // tec error auto aliceBalance = env.balance(alice); auto bobBalance = env.balance(bob); auto carolBalance = env.balance(carol); @@ -316,7 +315,7 @@ class Delegate_test : public beast::unit_test::suite env(pay(alice, carol, XRP(100)), fee(XRP(2000)), delegate::as(bob), - ter(terINSUF_FEE_B)); + ter(terNO_DELEGATE_PERMISSION)); env.close(); BEAST_EXPECT(env.balance(alice) == aliceBalance); BEAST_EXPECT(env.balance(bob) == bobBalance); @@ -523,12 +522,12 @@ class Delegate_test : public beast::unit_test::suite // bob does not have permission to create check env(check::create(alice, bob, XRP(10)), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); // carol does not have permission to create check env(check::create(alice, bob, XRP(10)), delegate::as(carol), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); } void @@ -563,9 +562,8 @@ class Delegate_test : public beast::unit_test::suite // delegate ledger object is not created yet env(pay(gw, alice, USD(50)), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); - env.require(balance(bob, bobBalance - drops(baseFee))); - bobBalance = env.balance(bob, XRP); + ter(terNO_DELEGATE_PERMISSION)); + env.require(balance(bob, bobBalance)); // gw gives bob burn permission env(delegate::set(gw, bob, {"PaymentBurn"})); @@ -576,10 +574,9 @@ class Delegate_test : public beast::unit_test::suite // bob sends a payment transaction on behalf of gw env(pay(gw, alice, USD(50)), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); env.close(); - env.require(balance(bob, bobBalance - drops(baseFee))); - bobBalance = env.balance(bob, XRP); + env.require(balance(bob, bobBalance)); // gw gives bob mint permission, alice gives bob burn permission env(delegate::set(gw, bob, {"PaymentMint"})); @@ -593,10 +590,9 @@ class Delegate_test : public beast::unit_test::suite // can not send XRP env(pay(gw, alice, XRP(50)), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); env.close(); - env.require(balance(bob, bobBalance - drops(baseFee))); - bobBalance = env.balance(bob, XRP); + env.require(balance(bob, bobBalance)); // mint 50 USD env(pay(gw, alice, USD(50)), delegate::as(bob)); @@ -681,10 +677,9 @@ class Delegate_test : public beast::unit_test::suite // permission env(pay(gw, alice, USD(50)), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); env.close(); - env.require(balance(bob, bobBalance - drops(baseFee))); - bobBalance = env.balance(bob, XRP); + env.require(balance(bob, bobBalance)); // gw gives bob Payment permission as well env(delegate::set(gw, bob, {"PaymentBurn", "Payment"})); @@ -723,11 +718,6 @@ class Delegate_test : public beast::unit_test::suite env(pay(gw, carol, USD(10000))); env.close(); - auto const result = features[fixDelegateV1_1] - ? static_cast(tecNO_DELEGATE_PERMISSION) - : static_cast(tesSUCCESS); - auto const offerCount = features[fixDelegateV1_1] ? 1 : 0; - // PaymentMint { env(offer(carol, XRP(100), USD(501))); @@ -735,16 +725,21 @@ class Delegate_test : public beast::unit_test::suite env(delegate::set(gw, bob, {"PaymentMint"})); env.close(); - // post-amendment: fixDelegateV1_1 // bob can not send cross currency payment on behalf of the gw, // even with PaymentMint permission and gw being the issuer. env(pay(gw, alice, USD(5000)), - path(~USD), sendmax(XRP(1001)), txflags(tfPartialPayment), delegate::as(bob), - ter(result)); - BEAST_EXPECT(expectOffers(env, carol, offerCount)); + ter(terNO_DELEGATE_PERMISSION)); + BEAST_EXPECT(expectOffers(env, carol, 1)); + + env(pay(gw, alice, USD(5000)), + path(~XRP), + txflags(tfPartialPayment), + delegate::as(bob), + ter(terNO_DELEGATE_PERMISSION)); + BEAST_EXPECT(expectOffers(env, carol, 1)); // succeed with direct payment env(pay(gw, alice, USD(100)), delegate::as(bob)); @@ -758,16 +753,21 @@ class Delegate_test : public beast::unit_test::suite env(delegate::set(alice, bob, {"PaymentBurn"})); env.close(); - // post-amendment: fixDelegateV1_1 // bob can not send cross currency payment on behalf of alice, // even with PaymentBurn permission and gw being the issuer. env(pay(alice, gw, USD(5000)), - path(~USD), sendmax(XRP(1001)), txflags(tfPartialPayment), delegate::as(bob), - ter(result)); - BEAST_EXPECT(expectOffers(env, bob, offerCount)); + ter(terNO_DELEGATE_PERMISSION)); + BEAST_EXPECT(expectOffers(env, bob, 1)); + + env(pay(alice, gw, USD(5000)), + path(~XRP), + txflags(tfPartialPayment), + delegate::as(bob), + ter(terNO_DELEGATE_PERMISSION)); + BEAST_EXPECT(expectOffers(env, bob, 1)); // succeed with direct payment env(pay(alice, gw, USD(100)), delegate::as(bob)); @@ -802,20 +802,10 @@ class Delegate_test : public beast::unit_test::suite env(delegate::set(gw, bob, {"PaymentMint"})); env.close(); - if (!features[fixDelegateV1_1]) - { - // pre-amendment: PaymentMint is not supported for MPT - env(pay(gw, alice, MPT(50)), - delegate::as(bob), - ter(tefEXCEPTION)); - } - else - { - env(pay(gw, alice, MPT(50)), delegate::as(bob)); - BEAST_EXPECT(env.balance(alice, MPT) == aliceMPT + MPT(50)); - BEAST_EXPECT(env.balance(bob, MPT) == bobMPT); - aliceMPT = env.balance(alice, MPT); - } + env(pay(gw, alice, MPT(50)), delegate::as(bob)); + BEAST_EXPECT(env.balance(alice, MPT) == aliceMPT + MPT(50)); + BEAST_EXPECT(env.balance(bob, MPT) == bobMPT); + aliceMPT = env.balance(alice, MPT); } // PaymentBurn @@ -823,24 +813,13 @@ class Delegate_test : public beast::unit_test::suite env(delegate::set(alice, bob, {"PaymentBurn"})); env.close(); - if (!features[fixDelegateV1_1]) - { - // pre-amendment: PaymentBurn is not supported for MPT - env(pay(alice, gw, MPT(50)), - delegate::as(bob), - ter(tefEXCEPTION)); - } - else - { - env(pay(alice, gw, MPT(50)), delegate::as(bob)); - BEAST_EXPECT(env.balance(alice, MPT) == aliceMPT - MPT(50)); - BEAST_EXPECT(env.balance(bob, MPT) == bobMPT); - aliceMPT = env.balance(alice, MPT); - } + env(pay(alice, gw, MPT(50)), delegate::as(bob)); + BEAST_EXPECT(env.balance(alice, MPT) == aliceMPT - MPT(50)); + BEAST_EXPECT(env.balance(bob, MPT) == bobMPT); + aliceMPT = env.balance(alice, MPT); } - // Payment transaction for MPT is allowed for both pre and post - // amendment + // Grant both granular permissions and tx level permission. { env(delegate::set( alice, bob, {"PaymentBurn", "PaymentMint", "Payment"})); @@ -878,7 +857,7 @@ class Delegate_test : public beast::unit_test::suite // has unfreeze permission env(trust(alice, gw["USD"](50)), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); env.close(); // alice creates trustline by herself @@ -892,38 +871,38 @@ class Delegate_test : public beast::unit_test::suite // unsupported flags env(trust(alice, gw["USD"](50), tfSetNoRipple), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); env(trust(alice, gw["USD"](50), tfClearNoRipple), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); env(trust(gw, gw["USD"](0), alice, tfSetDeepFreeze), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); env(trust(gw, gw["USD"](0), alice, tfClearDeepFreeze), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); env.close(); // supported flags with wrong permission env(trust(gw, gw["USD"](0), alice, tfSetfAuth), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); env(trust(gw, gw["USD"](0), alice, tfSetFreeze), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); env.close(); env(delegate::set(gw, bob, {"TrustlineAuthorize"})); env.close(); env(trust(gw, gw["USD"](0), alice, tfClearFreeze), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); env.close(); // although trustline authorize is granted, bob can not change the // limit number env(trust(gw, gw["USD"](50), alice, tfSetfAuth), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); env.close(); // supported flags with correct permission @@ -944,30 +923,30 @@ class Delegate_test : public beast::unit_test::suite // permission env(trust(gw, gw["USD"](0), alice, tfSetFreeze), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); // cannot update LimitAmount with granular permission, both high and // low account env(trust(alice, gw["USD"](100)), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); env(trust(gw, alice["USD"](100)), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); // can not set QualityIn or QualityOut auto tx = trust(alice, gw["USD"](50)); tx["QualityIn"] = "1000"; - env(tx, delegate::as(bob), ter(tecNO_DELEGATE_PERMISSION)); + env(tx, delegate::as(bob), ter(terNO_DELEGATE_PERMISSION)); auto tx2 = trust(alice, gw["USD"](50)); tx2["QualityOut"] = "1000"; - env(tx2, delegate::as(bob), ter(tecNO_DELEGATE_PERMISSION)); + env(tx2, delegate::as(bob), ter(terNO_DELEGATE_PERMISSION)); auto tx3 = trust(gw, alice["USD"](50)); tx3["QualityIn"] = "1000"; - env(tx3, delegate::as(bob), ter(tecNO_DELEGATE_PERMISSION)); + env(tx3, delegate::as(bob), ter(terNO_DELEGATE_PERMISSION)); auto tx4 = trust(gw, alice["USD"](50)); tx4["QualityOut"] = "1000"; - env(tx4, delegate::as(bob), ter(tecNO_DELEGATE_PERMISSION)); + env(tx4, delegate::as(bob), ter(terNO_DELEGATE_PERMISSION)); // granting TrustSet can make it work env(delegate::set(gw, bob, {"TrustSet"})); @@ -977,7 +956,7 @@ class Delegate_test : public beast::unit_test::suite env(tx5, delegate::as(bob)); auto tx6 = trust(alice, gw["USD"](50)); tx6["QualityOut"] = "1000"; - env(tx6, delegate::as(bob), ter(tecNO_DELEGATE_PERMISSION)); + env(tx6, delegate::as(bob), ter(terNO_DELEGATE_PERMISSION)); env(delegate::set(alice, bob, {"TrustSet"})); env.close(); env(tx6, delegate::as(bob)); @@ -996,14 +975,14 @@ class Delegate_test : public beast::unit_test::suite // bob does not have permission env(trust(alice, gw["USD"](50)), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); env(delegate::set( alice, bob, {"TrustlineUnfreeze", "NFTokenCreateOffer"})); env.close(); // bob still does not have permission env(trust(alice, gw["USD"](50)), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); // add TrustSet permission and some unrelated permission env(delegate::set( @@ -1096,7 +1075,7 @@ class Delegate_test : public beast::unit_test::suite env(delegate::set( alice, bob, {"TrustlineUnfreeze", "AccountEmailHashSet"})); env.close(); - env(jt, ter(tecNO_DELEGATE_PERMISSION)); + env(jt, ter(terNO_DELEGATE_PERMISSION)); // alice give granular permission of AccountDomainSet to bob env(delegate::set(alice, bob, {"AccountDomainSet"})); @@ -1115,7 +1094,7 @@ class Delegate_test : public beast::unit_test::suite std::string const failDomain = "fail_domain_update"; jt[sfFlags] = tfRequireAuth; jt[sfDomain] = strHex(failDomain); - env(jt, ter(tecNO_DELEGATE_PERMISSION)); + env(jt, ter(terNO_DELEGATE_PERMISSION)); // reset flag number jt[sfFlags] = 0; @@ -1124,7 +1103,7 @@ class Delegate_test : public beast::unit_test::suite jt[sfDomain] = strHex(domain); std::string const mh("5F31A79367DC3137FADA860C05742EE6"); jt[sfEmailHash] = mh; - env(jt, ter(tecNO_DELEGATE_PERMISSION)); + env(jt, ter(terNO_DELEGATE_PERMISSION)); // alice give granular permission of AccountEmailHashSet to bob env(delegate::set( @@ -1137,7 +1116,7 @@ class Delegate_test : public beast::unit_test::suite // bob does not have permission to set message key for alice auto const rkp = randomKeyPair(KeyType::ed25519); jt[sfMessageKey] = strHex(rkp.first.slice()); - env(jt, ter(tecNO_DELEGATE_PERMISSION)); + env(jt, ter(terNO_DELEGATE_PERMISSION)); // alice give granular permission of AccountMessageKeySet to bob env(delegate::set( @@ -1160,7 +1139,7 @@ class Delegate_test : public beast::unit_test::suite // bob does not have permission to set transfer rate for alice env(rate(alice, 2.0), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); // alice give granular permission of AccountTransferRateSet to bob env(delegate::set( @@ -1178,7 +1157,7 @@ class Delegate_test : public beast::unit_test::suite // bob does not have permission to set ticksize for alice jt[sfTickSize] = 8; - env(jt, ter(tecNO_DELEGATE_PERMISSION)); + env(jt, ter(terNO_DELEGATE_PERMISSION)); // alice give granular permission of AccountTickSizeSet to bob env(delegate::set( @@ -1196,7 +1175,7 @@ class Delegate_test : public beast::unit_test::suite // can not set asfRequireAuth flag for alice env(fset(alice, asfRequireAuth), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); // reset Delegate will delete the Delegate // object @@ -1205,7 +1184,7 @@ class Delegate_test : public beast::unit_test::suite // alice env(fset(alice, asfRequireAuth), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); // alice can set for herself env(fset(alice, asfRequireAuth)); env.require(flags(alice, asfRequireAuth)); @@ -1213,7 +1192,7 @@ class Delegate_test : public beast::unit_test::suite // can not update tick size because bob no longer has permission jt[sfTickSize] = 7; - env(jt, ter(tecNO_DELEGATE_PERMISSION)); + env(jt, ter(terNO_DELEGATE_PERMISSION)); env(delegate::set( alice, @@ -1231,7 +1210,7 @@ class Delegate_test : public beast::unit_test::suite jv2[sfDomain] = strHex(domain); jv2[sfDelegate] = bob.human(); jv2[sfWalletLocator] = locator; - env(jv2, ter(tecNO_DELEGATE_PERMISSION)); + env(jv2, ter(terNO_DELEGATE_PERMISSION)); } // can not set AccountSet flags on behalf of other account @@ -1246,7 +1225,7 @@ class Delegate_test : public beast::unit_test::suite // bob can not set flag on behalf of alice env(fset(alice, flag), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); // alice set by herself env(fset(alice, flag)); env.close(); @@ -1254,7 +1233,7 @@ class Delegate_test : public beast::unit_test::suite // bob can not clear on behalf of alice env(fclear(alice, flag), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); }; // testSetClearFlag(asfNoFreeze); @@ -1283,19 +1262,19 @@ class Delegate_test : public beast::unit_test::suite // bob can not set asfAccountTxnID on behalf of alice env(fset(alice, asfAccountTxnID), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); env(fset(alice, asfAccountTxnID)); env.close(); BEAST_EXPECT(env.le(alice)->isFieldPresent(sfAccountTxnID)); env(fclear(alice, asfAccountTxnID), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); // bob can not set asfAuthorizedNFTokenMinter on behalf of alice Json::Value jt = fset(alice, asfAuthorizedNFTokenMinter); jt[sfDelegate] = bob.human(); jt[sfNFTokenMinter] = bob.human(); - env(jt, ter(tecNO_DELEGATE_PERMISSION)); + env(jt, ter(terNO_DELEGATE_PERMISSION)); // bob gives alice some permissions env(delegate::set( @@ -1311,14 +1290,14 @@ class Delegate_test : public beast::unit_test::suite // behalf of bob. env(fset(alice, asfNoFreeze), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); env(fset(bob, asfNoFreeze)); env.close(); env.require(flags(bob, asfNoFreeze)); // alice can not clear on behalf of bob env(fclear(alice, asfNoFreeze), delegate::as(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); // bob can not set asfDisableMaster on behalf of alice Account const bobKey{"bobKey", KeyType::secp256k1}; @@ -1327,7 +1306,7 @@ class Delegate_test : public beast::unit_test::suite env(fset(alice, asfDisableMaster), delegate::as(bob), sig(bob), - ter(tecNO_DELEGATE_PERMISSION)); + ter(terNO_DELEGATE_PERMISSION)); } // tfFullyCanonicalSig won't block delegated transaction @@ -1377,7 +1356,7 @@ class Delegate_test : public beast::unit_test::suite {.account = alice, .flags = tfMPTLock, .delegate = bob, - .err = tecNO_DELEGATE_PERMISSION}); + .err = terNO_DELEGATE_PERMISSION}); // alice gives granular permission to bob of MPTokenIssuanceUnlock env(delegate::set(alice, bob, {"MPTokenIssuanceUnlock"})); @@ -1387,7 +1366,7 @@ class Delegate_test : public beast::unit_test::suite {.account = alice, .flags = tfMPTLock, .delegate = bob, - .err = tecNO_DELEGATE_PERMISSION}); + .err = terNO_DELEGATE_PERMISSION}); // bob now has lock permission, but does not have unlock permission env(delegate::set(alice, bob, {"MPTokenIssuanceLock"})); env.close(); @@ -1396,7 +1375,7 @@ class Delegate_test : public beast::unit_test::suite {.account = alice, .flags = tfMPTUnlock, .delegate = bob, - .err = tecNO_DELEGATE_PERMISSION}); + .err = terNO_DELEGATE_PERMISSION}); // now bob can lock and unlock env(delegate::set( @@ -1429,7 +1408,7 @@ class Delegate_test : public beast::unit_test::suite {.account = alice, .flags = tfMPTUnlock, .delegate = bob, - .err = tecNO_DELEGATE_PERMISSION}); + .err = terNO_DELEGATE_PERMISSION}); // alice gives bob some unrelated permission with // MPTokenIssuanceLock @@ -1443,7 +1422,7 @@ class Delegate_test : public beast::unit_test::suite {.account = alice, .flags = tfMPTUnlock, .delegate = bob, - .err = tecNO_DELEGATE_PERMISSION}); + .err = terNO_DELEGATE_PERMISSION}); // alice add MPTokenIssuanceSet to permissions env(delegate::set( @@ -1519,29 +1498,96 @@ class Delegate_test : public beast::unit_test::suite testcase("test single sign with bad secret"); using namespace jtx; - Env env(*this); - Account alice{"alice"}; - Account bob{"bob"}; - Account carol{"carol"}; - env.fund(XRP(100000), alice, bob, carol); - env.close(); + { + Env env(*this); + Account alice{"alice"}; + Account bob{"bob"}; + Account carol{"carol"}; + env.fund(XRP(100000), alice, bob, carol); + env.close(); - env(delegate::set(alice, bob, {"Payment"})); - env.close(); + env(delegate::set(alice, bob, {"Payment"})); + env.close(); - auto aliceBalance = env.balance(alice); - auto bobBalance = env.balance(bob); - auto carolBalance = env.balance(carol); + auto aliceBalance = env.balance(alice); + auto bobBalance = env.balance(bob); + auto carolBalance = env.balance(carol); - env(pay(alice, carol, XRP(100)), - fee(XRP(10)), - delegate::as(bob), - sig(alice), - ter(tefBAD_AUTH)); - env.close(); - BEAST_EXPECT(env.balance(alice) == aliceBalance); - BEAST_EXPECT(env.balance(bob) == bobBalance); - BEAST_EXPECT(env.balance(carol) == carolBalance); + env(pay(alice, carol, XRP(100)), + fee(XRP(10)), + delegate::as(bob), + sig(alice), + ter(tefBAD_AUTH)); + env.close(); + BEAST_EXPECT(env.balance(alice) == aliceBalance); + BEAST_EXPECT(env.balance(bob) == bobBalance); + BEAST_EXPECT(env.balance(carol) == carolBalance); + } + + { + Env env(*this); + Account alice{"alice"}, bob{"bob"}, carol{"carol"}; + env.fund(XRP(100000), alice, bob, carol); + env.close(); + + env(delegate::set(alice, bob, {"TrustSet"})); + env.close(); + + auto aliceBalance = env.balance(alice); + auto bobBalance = env.balance(bob); + auto carolBalance = env.balance(carol); + + env(pay(alice, carol, XRP(100)), + fee(XRP(10)), + delegate::as(bob), + sig(carol), + ter(terNO_DELEGATE_PERMISSION)); + env.close(); + BEAST_EXPECT(env.balance(alice) == aliceBalance); + BEAST_EXPECT(env.balance(bob) == bobBalance); + BEAST_EXPECT(env.balance(carol) == carolBalance); + + env(pay(alice, carol, XRP(100)), + fee(XRP(10)), + delegate::as(bob), + sig(alice), + ter(terNO_DELEGATE_PERMISSION)); + env.close(); + BEAST_EXPECT(env.balance(alice) == aliceBalance); + BEAST_EXPECT(env.balance(bob) == bobBalance); + BEAST_EXPECT(env.balance(carol) == carolBalance); + } + + { + Env env(*this); + Account alice{"alice"}, bob{"bob"}, carol{"carol"}; + env.fund(XRP(100000), alice, bob, carol); + env.close(); + + auto aliceBalance = env.balance(alice); + auto bobBalance = env.balance(bob); + auto carolBalance = env.balance(carol); + + env(pay(alice, carol, XRP(100)), + fee(XRP(10)), + delegate::as(bob), + sig(alice), + ter(terNO_DELEGATE_PERMISSION)); + env.close(); + BEAST_EXPECT(env.balance(alice) == aliceBalance); + BEAST_EXPECT(env.balance(bob) == bobBalance); + BEAST_EXPECT(env.balance(carol) == carolBalance); + + env(pay(alice, carol, XRP(100)), + fee(XRP(10)), + delegate::as(bob), + sig(carol), + ter(terNO_DELEGATE_PERMISSION)); + env.close(); + BEAST_EXPECT(env.balance(alice) == aliceBalance); + BEAST_EXPECT(env.balance(bob) == bobBalance); + BEAST_EXPECT(env.balance(carol) == carolBalance); + } } void @@ -1659,10 +1705,7 @@ class Delegate_test : public beast::unit_test::suite for (auto value : {0, 100000, 54321}) { auto jv = buildRequest(value); - if (!features[fixDelegateV1_1]) - env(jv); - else - env(jv, ter(temMALFORMED)); + env(jv, ter(temMALFORMED)); } } @@ -1720,8 +1763,7 @@ class Delegate_test : public beast::unit_test::suite {"VaultWithdraw", featureSingleAssetVault}, {"VaultClawback", featureSingleAssetVault}}; - // fixDelegateV1_1 post-amendment: can not delegate tx if any - // required feature disabled. + // Can not delegate tx if any required feature disabled. { auto txAmendmentDisabled = [&](FeatureBitset features, std::string const& tx) { @@ -1734,10 +1776,7 @@ class Delegate_test : public beast::unit_test::suite env.fund(XRP(100000), alice, bob); env.close(); - if (!features[fixDelegateV1_1]) - env(delegate::set(alice, bob, {tx})); - else - env(delegate::set(alice, bob, {tx}), ter(temMALFORMED)); + env(delegate::set(alice, bob, {tx}), ter(temMALFORMED)); }; for (auto const& tx : txRequiredFeatures) @@ -1786,10 +1825,7 @@ class Delegate_test : public beast::unit_test::suite "NFTokenCancelOffer", "NFTokenAcceptOffer"}) { - if (!features[fixDelegateV1_1]) - env(delegate::set(alice, bob, {tx})); - else - env(delegate::set(alice, bob, {tx}), ter(temMALFORMED)); + env(delegate::set(alice, bob, {tx}), ter(temMALFORMED)); } } @@ -1823,17 +1859,16 @@ class Delegate_test : public beast::unit_test::suite { FeatureBitset const all = jtx::testable_amendments(); - testFeatureDisabled(); + testFeatureDisabled(all - featurePermissionDelegationV1_1); + testFeatureDisabled(all); testDelegateSet(); testInvalidRequest(all); - testInvalidRequest(all - fixDelegateV1_1); testReserve(); testFee(); testSequence(); testAccountDelete(); testDelegateTransaction(); testPaymentGranular(all); - testPaymentGranular(all - fixDelegateV1_1); testTrustSetGranular(); testAccountSetGranular(); testMPTokenIssuanceSetGranular(); @@ -1842,9 +1877,7 @@ class Delegate_test : public beast::unit_test::suite testMultiSign(); testMultiSignQuorumNotMet(); testPermissionValue(all); - testPermissionValue(all - fixDelegateV1_1); testTxReqireFeatures(all); - testTxReqireFeatures(all - fixDelegateV1_1); } }; BEAST_DEFINE_TESTSUITE(Delegate, app, ripple); diff --git a/src/xrpld/app/misc/DelegateUtils.h b/src/xrpld/app/misc/DelegateUtils.h index 8d657e6a09..b51d808d66 100644 --- a/src/xrpld/app/misc/DelegateUtils.h +++ b/src/xrpld/app/misc/DelegateUtils.h @@ -31,10 +31,10 @@ namespace ripple { * Check if the delegate account has permission to execute the transaction. * @param delegate The delegate account. * @param tx The transaction that the delegate account intends to execute. - * @return tesSUCCESS if the transaction is allowed, tecNO_DELEGATE_PERMISSION + * @return tesSUCCESS if the transaction is allowed, terNO_DELEGATE_PERMISSION * if not. */ -TER +NotTEC checkTxPermission(std::shared_ptr const& delegate, STTx const& tx); /** diff --git a/src/xrpld/app/misc/detail/DelegateUtils.cpp b/src/xrpld/app/misc/detail/DelegateUtils.cpp index 229af555ff..19aed98776 100644 --- a/src/xrpld/app/misc/detail/DelegateUtils.cpp +++ b/src/xrpld/app/misc/detail/DelegateUtils.cpp @@ -22,11 +22,11 @@ #include namespace ripple { -TER +NotTEC checkTxPermission(std::shared_ptr const& delegate, STTx const& tx) { if (!delegate) - return tecNO_DELEGATE_PERMISSION; // LCOV_EXCL_LINE + return terNO_DELEGATE_PERMISSION; // LCOV_EXCL_LINE auto const permissionArray = delegate->getFieldArray(sfPermissions); auto const txPermission = tx.getTxnType() + 1; @@ -38,7 +38,7 @@ checkTxPermission(std::shared_ptr const& delegate, STTx const& tx) return tesSUCCESS; } - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; } void diff --git a/src/xrpld/app/tx/detail/DelegateSet.cpp b/src/xrpld/app/tx/detail/DelegateSet.cpp index e769f75d8a..674ab3b745 100644 --- a/src/xrpld/app/tx/detail/DelegateSet.cpp +++ b/src/xrpld/app/tx/detail/DelegateSet.cpp @@ -45,8 +45,7 @@ DelegateSet::preflight(PreflightContext const& ctx) if (!permissionSet.insert(permission[sfPermissionValue]).second) return temMALFORMED; - if (ctx.rules.enabled(fixDelegateV1_1) && - !Permission::getInstance().isDelegatable( + if (!Permission::getInstance().isDelegatable( permission[sfPermissionValue], ctx.rules)) return temMALFORMED; } @@ -63,26 +62,6 @@ DelegateSet::preclaim(PreclaimContext const& ctx) if (!ctx.view.exists(keylet::account(ctx.tx[sfAuthorize]))) return tecNO_TARGET; - auto const& permissions = ctx.tx.getFieldArray(sfPermissions); - for (auto const& permission : permissions) - { - if (!ctx.view.rules().enabled(fixDelegateV1_1) && - !Permission::getInstance().isDelegatable( - permission[sfPermissionValue], ctx.view.rules())) - { - // Before fixDelegateV1_1: - // - The check was performed during preclaim. - // - Transactions from amendments not yet enabled could still be - // delegated. - // - // After fixDelegateV1_1: - // - The check is performed during preflight. - // - Transactions from amendments not yet enabled can no longer be - // delegated. - return tecNO_PERMISSION; - } - } - return tesSUCCESS; } diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp index f2c9bd8a96..1392472569 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp @@ -140,7 +140,7 @@ MPTokenIssuanceSet::preflight(PreflightContext const& ctx) return tesSUCCESS; } -TER +NotTEC MPTokenIssuanceSet::checkPermission(ReadView const& view, STTx const& tx) { auto const delegate = tx[~sfDelegate]; @@ -151,7 +151,7 @@ MPTokenIssuanceSet::checkPermission(ReadView const& view, STTx const& tx) auto const sle = view.read(delegateKey); if (!sle) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; if (checkTxPermission(sle, tx) == tesSUCCESS) return tesSUCCESS; @@ -161,18 +161,18 @@ MPTokenIssuanceSet::checkPermission(ReadView const& view, STTx const& tx) // this is added in case more flags will be added for MPTokenIssuanceSet // in the future. Currently unreachable. if (txFlags & tfMPTokenIssuanceSetPermissionMask) - return tecNO_DELEGATE_PERMISSION; // LCOV_EXCL_LINE + return terNO_DELEGATE_PERMISSION; // LCOV_EXCL_LINE std::unordered_set granularPermissions; loadGranularPermission(sle, ttMPTOKEN_ISSUANCE_SET, granularPermissions); if (txFlags & tfMPTLock && !granularPermissions.contains(MPTokenIssuanceLock)) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; if (txFlags & tfMPTUnlock && !granularPermissions.contains(MPTokenIssuanceUnlock)) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; return tesSUCCESS; } diff --git a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.h b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.h index f63812097e..c801d2d00f 100644 --- a/src/xrpld/app/tx/detail/MPTokenIssuanceSet.h +++ b/src/xrpld/app/tx/detail/MPTokenIssuanceSet.h @@ -42,7 +42,7 @@ public: static NotTEC preflight(PreflightContext const& ctx); - static TER + static NotTEC checkPermission(ReadView const& view, STTx const& tx); static TER diff --git a/src/xrpld/app/tx/detail/Payment.cpp b/src/xrpld/app/tx/detail/Payment.cpp index b9bdbd0a34..5619100a12 100644 --- a/src/xrpld/app/tx/detail/Payment.cpp +++ b/src/xrpld/app/tx/detail/Payment.cpp @@ -250,7 +250,7 @@ Payment::preflight(PreflightContext const& ctx) return tesSUCCESS; } -TER +NotTEC Payment::checkPermission(ReadView const& view, STTx const& tx) { auto const delegate = tx[~sfDelegate]; @@ -261,7 +261,7 @@ Payment::checkPermission(ReadView const& view, STTx const& tx) auto const sle = view.read(delegateKey); if (!sle) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; if (checkTxPermission(sle, tx) == tesSUCCESS) return tesSUCCESS; @@ -270,42 +270,23 @@ Payment::checkPermission(ReadView const& view, STTx const& tx) loadGranularPermission(sle, ttPAYMENT, granularPermissions); auto const& dstAmount = tx.getFieldAmount(sfAmount); - // post-amendment: disallow cross currency payments for PaymentMint and - // PaymentBurn - if (view.rules().enabled(fixDelegateV1_1)) - { - auto const& amountAsset = dstAmount.asset(); - if (tx.isFieldPresent(sfSendMax) && - tx[sfSendMax].asset() != amountAsset) - return tecNO_DELEGATE_PERMISSION; + auto const& amountAsset = dstAmount.asset(); - if (granularPermissions.contains(PaymentMint) && !isXRP(amountAsset) && - amountAsset.getIssuer() == tx[sfAccount]) - return tesSUCCESS; + // Granular permissions are only valid for direct payments. + if ((tx.isFieldPresent(sfSendMax) && + tx[sfSendMax].asset() != amountAsset) || + tx.isFieldPresent(sfPaths)) + return terNO_DELEGATE_PERMISSION; - if (granularPermissions.contains(PaymentBurn) && !isXRP(amountAsset) && - amountAsset.getIssuer() == tx[sfDestination]) - return tesSUCCESS; - - return tecNO_DELEGATE_PERMISSION; - } - - // Calling dstAmount.issue() in the next line would throw if it holds MPT. - // That exception would be caught in preclaim and returned as tefEXCEPTION. - // This check is just a cleaner, more explicit way to get the same result. - if (dstAmount.holds()) - return tefEXCEPTION; - - auto const& amountIssue = dstAmount.issue(); - if (granularPermissions.contains(PaymentMint) && !isXRP(amountIssue) && - amountIssue.account == tx[sfAccount]) + if (granularPermissions.contains(PaymentMint) && !isXRP(amountAsset) && + amountAsset.getIssuer() == tx[sfAccount]) return tesSUCCESS; - if (granularPermissions.contains(PaymentBurn) && !isXRP(amountIssue) && - amountIssue.account == tx[sfDestination]) + if (granularPermissions.contains(PaymentBurn) && !isXRP(amountAsset) && + amountAsset.getIssuer() == tx[sfDestination]) return tesSUCCESS; - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; } TER diff --git a/src/xrpld/app/tx/detail/Payment.h b/src/xrpld/app/tx/detail/Payment.h index 04bba390e2..f74df4d099 100644 --- a/src/xrpld/app/tx/detail/Payment.h +++ b/src/xrpld/app/tx/detail/Payment.h @@ -51,7 +51,7 @@ public: static NotTEC preflight(PreflightContext const& ctx); - static TER + static NotTEC checkPermission(ReadView const& view, STTx const& tx); static TER diff --git a/src/xrpld/app/tx/detail/SetAccount.cpp b/src/xrpld/app/tx/detail/SetAccount.cpp index 7c60ec646a..73ddcef974 100644 --- a/src/xrpld/app/tx/detail/SetAccount.cpp +++ b/src/xrpld/app/tx/detail/SetAccount.cpp @@ -186,7 +186,7 @@ SetAccount::preflight(PreflightContext const& ctx) return tesSUCCESS; } -TER +NotTEC SetAccount::checkPermission(ReadView const& view, STTx const& tx) { // SetAccount is prohibited to be granted on a transaction level, @@ -199,7 +199,7 @@ SetAccount::checkPermission(ReadView const& view, STTx const& tx) auto const sle = view.read(delegateKey); if (!sle) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; std::unordered_set granularPermissions; loadGranularPermission(sle, ttACCOUNT_SET, granularPermissions); @@ -212,31 +212,31 @@ SetAccount::checkPermission(ReadView const& view, STTx const& tx) // update the flag on behalf of another account, it is not // authorized. if (uSetFlag != 0 || uClearFlag != 0 || uTxFlags & tfUniversalMask) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; if (tx.isFieldPresent(sfEmailHash) && !granularPermissions.contains(AccountEmailHashSet)) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; if (tx.isFieldPresent(sfWalletLocator) || tx.isFieldPresent(sfNFTokenMinter)) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; if (tx.isFieldPresent(sfMessageKey) && !granularPermissions.contains(AccountMessageKeySet)) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; if (tx.isFieldPresent(sfDomain) && !granularPermissions.contains(AccountDomainSet)) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; if (tx.isFieldPresent(sfTransferRate) && !granularPermissions.contains(AccountTransferRateSet)) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; if (tx.isFieldPresent(sfTickSize) && !granularPermissions.contains(AccountTickSizeSet)) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; return tesSUCCESS; } diff --git a/src/xrpld/app/tx/detail/SetAccount.h b/src/xrpld/app/tx/detail/SetAccount.h index bcc0a61b1b..797d8f94fc 100644 --- a/src/xrpld/app/tx/detail/SetAccount.h +++ b/src/xrpld/app/tx/detail/SetAccount.h @@ -44,7 +44,7 @@ public: static NotTEC preflight(PreflightContext const& ctx); - static TER + static NotTEC checkPermission(ReadView const& view, STTx const& tx); static TER diff --git a/src/xrpld/app/tx/detail/SetTrust.cpp b/src/xrpld/app/tx/detail/SetTrust.cpp index 59333f3077..2820f13283 100644 --- a/src/xrpld/app/tx/detail/SetTrust.cpp +++ b/src/xrpld/app/tx/detail/SetTrust.cpp @@ -127,7 +127,7 @@ SetTrust::preflight(PreflightContext const& ctx) return tesSUCCESS; } -TER +NotTEC SetTrust::checkPermission(ReadView const& view, STTx const& tx) { auto const delegate = tx[~sfDelegate]; @@ -138,7 +138,7 @@ SetTrust::checkPermission(ReadView const& view, STTx const& tx) auto const sle = view.read(delegateKey); if (!sle) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; if (checkTxPermission(sle, tx) == tesSUCCESS) return tesSUCCESS; @@ -149,10 +149,10 @@ SetTrust::checkPermission(ReadView const& view, STTx const& tx) // TrustlineUnfreeze granular permission. Setting other flags returns // error. if (txFlags & tfTrustSetPermissionMask) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; if (tx.isFieldPresent(sfQualityIn) || tx.isFieldPresent(sfQualityOut)) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; auto const saLimitAmount = tx.getFieldAmount(sfLimitAmount); auto const sleRippleState = view.read(keylet::line( @@ -161,19 +161,19 @@ SetTrust::checkPermission(ReadView const& view, STTx const& tx) // if the trustline does not exist, granular permissions are // not allowed to create trustline if (!sleRippleState) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; std::unordered_set granularPermissions; loadGranularPermission(sle, ttTRUST_SET, granularPermissions); if (txFlags & tfSetfAuth && !granularPermissions.contains(TrustlineAuthorize)) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; if (txFlags & tfSetFreeze && !granularPermissions.contains(TrustlineFreeze)) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; if (txFlags & tfClearFreeze && !granularPermissions.contains(TrustlineUnfreeze)) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; // updating LimitAmount is not allowed only with granular permissions, // unless there's a new granular permission for this in the future. @@ -185,7 +185,7 @@ SetTrust::checkPermission(ReadView const& view, STTx const& tx) saLimitAllow.setIssuer(tx[sfAccount]); if (curLimit != saLimitAllow) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; return tesSUCCESS; } diff --git a/src/xrpld/app/tx/detail/SetTrust.h b/src/xrpld/app/tx/detail/SetTrust.h index 443080bf74..7219c16bb9 100644 --- a/src/xrpld/app/tx/detail/SetTrust.h +++ b/src/xrpld/app/tx/detail/SetTrust.h @@ -41,7 +41,7 @@ public: static NotTEC preflight(PreflightContext const& ctx); - static TER + static NotTEC checkPermission(ReadView const& view, STTx const& tx); static TER diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index 6e3b56fe99..8af5ccf232 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -172,7 +172,7 @@ Transactor::preflight1(PreflightContext const& ctx, std::uint32_t flagMask) if (ctx.tx.isFieldPresent(sfDelegate)) { - if (!ctx.rules.enabled(featurePermissionDelegation)) + if (!ctx.rules.enabled(featurePermissionDelegationV1_1)) return temDISABLED; if (ctx.tx[sfDelegate] == ctx.tx[sfAccount]) @@ -273,7 +273,7 @@ Transactor::preflightSigValidated(PreflightContext const& ctx) return tesSUCCESS; } -TER +NotTEC Transactor::checkPermission(ReadView const& view, STTx const& tx) { auto const delegate = tx[~sfDelegate]; @@ -284,7 +284,7 @@ Transactor::checkPermission(ReadView const& view, STTx const& tx) auto const sle = view.read(delegateKey); if (!sle) - return tecNO_DELEGATE_PERMISSION; + return terNO_DELEGATE_PERMISSION; return checkTxPermission(sle, tx); } diff --git a/src/xrpld/app/tx/detail/Transactor.h b/src/xrpld/app/tx/detail/Transactor.h index 17ef62e607..af8a6813e9 100644 --- a/src/xrpld/app/tx/detail/Transactor.h +++ b/src/xrpld/app/tx/detail/Transactor.h @@ -237,7 +237,7 @@ public: return tesSUCCESS; } - static TER + static NotTEC checkPermission(ReadView const& view, STTx const& tx); ///////////////////////////////////////////////////// diff --git a/src/xrpld/app/tx/detail/applySteps.cpp b/src/xrpld/app/tx/detail/applySteps.cpp index 1cad93eedc..f34c23af80 100644 --- a/src/xrpld/app/tx/detail/applySteps.cpp +++ b/src/xrpld/app/tx/detail/applySteps.cpp @@ -145,37 +145,45 @@ invoke_preclaim(PreclaimContext const& ctx) { // use name hiding to accomplish compile-time polymorphism of static // class functions for Transactor and derived classes. - return with_txn_type(ctx.tx.getTxnType(), [&]() { - // If the transactor requires a valid account and the transaction - // doesn't list one, preflight will have already a flagged a - // failure. + return with_txn_type(ctx.tx.getTxnType(), [&]() -> TER { + // preclaim functionality is divided into two sections: + // 1. Up to and including the signature check: returns NotTEC. + // All transaction checks before and including checkSign + // MUST return NotTEC, or something more restrictive. + // Allowing tec results in these steps risks theft or + // destruction of funds, as a fee will be charged before the + // signature is checked. + // 2. After the signature check: returns TER. + + // If the transactor requires a valid account and the + // transaction doesn't list one, preflight will have already + // a flagged a failure. auto const id = ctx.tx.getAccountID(sfAccount); if (id != beast::zero) { - TER result = T::checkSeqProxy(ctx.view, ctx.tx, ctx.j); + if (NotTEC const preSigResult = [&]() -> NotTEC { + if (NotTEC const result = + T::checkSeqProxy(ctx.view, ctx.tx, ctx.j)) + return result; - if (result != tesSUCCESS) - return result; + if (NotTEC const result = + T::checkPriorTxAndLastLedger(ctx)) + return result; - result = T::checkPriorTxAndLastLedger(ctx); + if (NotTEC const result = + T::checkPermission(ctx.view, ctx.tx)) + return result; - if (result != tesSUCCESS) - return result; + if (NotTEC const result = T::checkSign(ctx)) + return result; - result = T::checkFee(ctx, calculateBaseFee(ctx.view, ctx.tx)); + return tesSUCCESS; + }()) + return preSigResult; - if (result != tesSUCCESS) - return result; - - result = T::checkPermission(ctx.view, ctx.tx); - - if (result != tesSUCCESS) - return result; - - result = T::checkSign(ctx); - - if (result != tesSUCCESS) + if (TER const result = + T::checkFee(ctx, calculateBaseFee(ctx.view, ctx.tx))) return result; } From 4e32d2ed98784dbcc8873a019eafb6d3385d654c Mon Sep 17 00:00:00 2001 From: Mayukha Vadari Date: Fri, 31 Oct 2025 15:29:30 -0400 Subject: [PATCH 2/9] refactor: Clean up `TxMeta` (#5845) This change: * Simplifies the `TxMeta` constructors - both were setting the same set of fields, and to make it harder for future bugs to arise and keep the code DRY, we can combine those into one helper function. * Removes an unused constructor. * Renames the variables to avoid Hungarian naming. * Removes a bunch of now-unnecessary helper functions. --- include/xrpl/protocol/TxMeta.h | 86 +++++----------- src/libxrpl/ledger/ApplyStateTable.cpp | 6 +- src/libxrpl/protocol/TxMeta.cpp | 125 +++++++++-------------- src/xrpld/rpc/detail/DeliveredAmount.cpp | 5 +- 4 files changed, 82 insertions(+), 140 deletions(-) diff --git a/include/xrpl/protocol/TxMeta.h b/include/xrpl/protocol/TxMeta.h index 02fde2ffe5..975ade355f 100644 --- a/include/xrpl/protocol/TxMeta.h +++ b/include/xrpl/protocol/TxMeta.h @@ -33,51 +33,35 @@ namespace ripple { class TxMeta { -private: - struct CtorHelper - { - explicit CtorHelper() = default; - }; - template - TxMeta( - uint256 const& txID, - std::uint32_t ledger, - T const& data, - CtorHelper); - public: - TxMeta( - uint256 const& transactionID, - std::uint32_t ledger, - std::optional parentBatchId = std::nullopt); + TxMeta(uint256 const& transactionID, std::uint32_t ledger); TxMeta(uint256 const& txID, std::uint32_t ledger, Blob const&); - TxMeta(uint256 const& txID, std::uint32_t ledger, std::string const&); TxMeta(uint256 const& txID, std::uint32_t ledger, STObject const&); uint256 const& getTxID() const { - return mTransactionID; + return transactionID_; } std::uint32_t getLgrSeq() const { - return mLedger; + return ledgerSeq_; } int getResult() const { - return mResult; + return result_; } TER getResultTER() const { - return TER::fromInt(mResult); + return TER::fromInt(result_); } std::uint32_t getIndex() const { - return mIndex; + return index_; } void @@ -104,66 +88,52 @@ public: STArray& getNodes() { - return (mNodes); + return nodes_; } STArray const& getNodes() const { - return (mNodes); + return nodes_; } void - setDeliveredAmount(STAmount const& delivered) + setAdditionalFields(STObject const& obj) { - mDelivered = delivered; + if (obj.isFieldPresent(sfDeliveredAmount)) + deliveredAmount_ = obj.getFieldAmount(sfDeliveredAmount); + + if (obj.isFieldPresent(sfParentBatchID)) + parentBatchID_ = obj.getFieldH256(sfParentBatchID); } - STAmount + std::optional const& getDeliveredAmount() const { - XRPL_ASSERT( - hasDeliveredAmount(), - "ripple::TxMeta::getDeliveredAmount : non-null delivered amount"); - return *mDelivered; - } - - bool - hasDeliveredAmount() const - { - return static_cast(mDelivered); + return deliveredAmount_; } void - setParentBatchId(uint256 const& parentBatchId) + setDeliveredAmount(std::optional const& amount) { - mParentBatchId = parentBatchId; + deliveredAmount_ = amount; } - uint256 - getParentBatchId() const + void + setParentBatchID(std::optional const& id) { - XRPL_ASSERT( - hasParentBatchId(), - "ripple::TxMeta::getParentBatchId : non-null batch id"); - return *mParentBatchId; - } - - bool - hasParentBatchId() const - { - return static_cast(mParentBatchId); + parentBatchID_ = id; } private: - uint256 mTransactionID; - std::uint32_t mLedger; - std::uint32_t mIndex; - int mResult; + uint256 transactionID_; + std::uint32_t ledgerSeq_; + std::uint32_t index_; + int result_; - std::optional mDelivered; - std::optional mParentBatchId; + std::optional deliveredAmount_; + std::optional parentBatchID_; - STArray mNodes; + STArray nodes_; }; } // namespace ripple diff --git a/src/libxrpl/ledger/ApplyStateTable.cpp b/src/libxrpl/ledger/ApplyStateTable.cpp index 99b76e2d11..11126d5242 100644 --- a/src/libxrpl/ledger/ApplyStateTable.cpp +++ b/src/libxrpl/ledger/ApplyStateTable.cpp @@ -126,10 +126,10 @@ ApplyStateTable::apply( std::optional metadata; if (!to.open() || isDryRun) { - TxMeta meta(tx.getTransactionID(), to.seq(), parentBatchId); + TxMeta meta(tx.getTransactionID(), to.seq()); - if (deliver) - meta.setDeliveredAmount(*deliver); + meta.setDeliveredAmount(deliver); + meta.setParentBatchID(parentBatchId); Mods newMod; for (auto& item : items_) diff --git a/src/libxrpl/protocol/TxMeta.cpp b/src/libxrpl/protocol/TxMeta.cpp index 833a0677b9..965a955e44 100644 --- a/src/libxrpl/protocol/TxMeta.cpp +++ b/src/libxrpl/protocol/TxMeta.cpp @@ -39,35 +39,13 @@ namespace ripple { -template -TxMeta::TxMeta( - uint256 const& txid, - std::uint32_t ledger, - T const& data, - CtorHelper) - : mTransactionID(txid), mLedger(ledger), mNodes(sfAffectedNodes, 32) -{ - SerialIter sit(makeSlice(data)); - - STObject obj(sit, sfMetadata); - mResult = obj.getFieldU8(sfTransactionResult); - mIndex = obj.getFieldU32(sfTransactionIndex); - mNodes = *dynamic_cast(&obj.getField(sfAffectedNodes)); - - if (obj.isFieldPresent(sfDeliveredAmount)) - setDeliveredAmount(obj.getFieldAmount(sfDeliveredAmount)); - - if (obj.isFieldPresent(sfParentBatchID)) - setParentBatchId(obj.getFieldH256(sfParentBatchID)); -} - TxMeta::TxMeta(uint256 const& txid, std::uint32_t ledger, STObject const& obj) - : mTransactionID(txid) - , mLedger(ledger) - , mNodes(obj.getFieldArray(sfAffectedNodes)) + : transactionID_(txid) + , ledgerSeq_(ledger) + , nodes_(obj.getFieldArray(sfAffectedNodes)) { - mResult = obj.getFieldU8(sfTransactionResult); - mIndex = obj.getFieldU32(sfTransactionIndex); + result_ = obj.getFieldU8(sfTransactionResult); + index_ = obj.getFieldU32(sfTransactionIndex); auto affectedNodes = dynamic_cast(obj.peekAtPField(sfAffectedNodes)); @@ -75,40 +53,32 @@ TxMeta::TxMeta(uint256 const& txid, std::uint32_t ledger, STObject const& obj) affectedNodes, "ripple::TxMeta::TxMeta(STObject) : type cast succeeded"); if (affectedNodes) - mNodes = *affectedNodes; + nodes_ = *affectedNodes; - if (obj.isFieldPresent(sfDeliveredAmount)) - setDeliveredAmount(obj.getFieldAmount(sfDeliveredAmount)); - - if (obj.isFieldPresent(sfParentBatchID)) - setParentBatchId(obj.getFieldH256(sfParentBatchID)); + setAdditionalFields(obj); } TxMeta::TxMeta(uint256 const& txid, std::uint32_t ledger, Blob const& vec) - : TxMeta(txid, ledger, vec, CtorHelper()) + : transactionID_(txid), ledgerSeq_(ledger), nodes_(sfAffectedNodes, 32) { + SerialIter sit(makeSlice(vec)); + + STObject obj(sit, sfMetadata); + result_ = obj.getFieldU8(sfTransactionResult); + index_ = obj.getFieldU32(sfTransactionIndex); + nodes_ = obj.getFieldArray(sfAffectedNodes); + + setAdditionalFields(obj); } -TxMeta::TxMeta( - uint256 const& txid, - std::uint32_t ledger, - std::string const& data) - : TxMeta(txid, ledger, data, CtorHelper()) +TxMeta::TxMeta(uint256 const& transactionID, std::uint32_t ledger) + : transactionID_(transactionID) + , ledgerSeq_(ledger) + , index_(std::numeric_limits::max()) + , result_(255) + , nodes_(sfAffectedNodes) { -} - -TxMeta::TxMeta( - uint256 const& transactionID, - std::uint32_t ledger, - std::optional parentBatchId) - : mTransactionID(transactionID) - , mLedger(ledger) - , mIndex(static_cast(-1)) - , mResult(255) - , mParentBatchId(parentBatchId) - , mNodes(sfAffectedNodes) -{ - mNodes.reserve(32); + nodes_.reserve(32); } void @@ -118,7 +88,7 @@ TxMeta::setAffectedNode( std::uint16_t nodeType) { // make sure the node exists and force its type - for (auto& n : mNodes) + for (auto& n : nodes_) { if (n.getFieldH256(sfLedgerIndex) == node) { @@ -128,8 +98,8 @@ TxMeta::setAffectedNode( } } - mNodes.push_back(STObject(type)); - STObject& obj = mNodes.back(); + nodes_.push_back(STObject(type)); + STObject& obj = nodes_.back(); XRPL_ASSERT( obj.getFName() == type, @@ -146,14 +116,15 @@ TxMeta::getAffectedAccounts() const // This code should match the behavior of the JS method: // Meta#getAffectedAccounts - for (auto const& it : mNodes) + for (auto const& node : nodes_) { - int index = it.getFieldIndex( - (it.getFName() == sfCreatedNode) ? sfNewFields : sfFinalFields); + int index = node.getFieldIndex( + (node.getFName() == sfCreatedNode) ? sfNewFields : sfFinalFields); if (index != -1) { - auto inner = dynamic_cast(&it.peekAtIndex(index)); + auto const* inner = + dynamic_cast(&node.peekAtIndex(index)); XRPL_ASSERT( inner, "ripple::getAffectedAccounts : STObject type cast succeeded"); @@ -213,13 +184,13 @@ STObject& TxMeta::getAffectedNode(SLE::ref node, SField const& type) { uint256 index = node->key(); - for (auto& n : mNodes) + for (auto& n : nodes_) { if (n.getFieldH256(sfLedgerIndex) == index) return n; } - mNodes.push_back(STObject(type)); - STObject& obj = mNodes.back(); + nodes_.push_back(STObject(type)); + STObject& obj = nodes_.back(); XRPL_ASSERT( obj.getFName() == type, @@ -233,7 +204,7 @@ TxMeta::getAffectedNode(SLE::ref node, SField const& type) STObject& TxMeta::getAffectedNode(uint256 const& node) { - for (auto& n : mNodes) + for (auto& n : nodes_) { if (n.getFieldH256(sfLedgerIndex) == node) return n; @@ -241,7 +212,7 @@ TxMeta::getAffectedNode(uint256 const& node) // LCOV_EXCL_START UNREACHABLE("ripple::TxMeta::getAffectedNode(uint256) : node not found"); Throw("Affected node not found"); - return *(mNodes.begin()); // Silence compiler warning. + return *(nodes_.begin()); // Silence compiler warning. // LCOV_EXCL_STOP } @@ -249,15 +220,15 @@ STObject TxMeta::getAsObject() const { STObject metaData(sfTransactionMetaData); - XRPL_ASSERT(mResult != 255, "ripple::TxMeta::getAsObject : result is set"); - metaData.setFieldU8(sfTransactionResult, mResult); - metaData.setFieldU32(sfTransactionIndex, mIndex); - metaData.emplace_back(mNodes); - if (hasDeliveredAmount()) - metaData.setFieldAmount(sfDeliveredAmount, getDeliveredAmount()); + XRPL_ASSERT(result_ != 255, "ripple::TxMeta::getAsObject : result_ is set"); + metaData.setFieldU8(sfTransactionResult, result_); + metaData.setFieldU32(sfTransactionIndex, index_); + metaData.emplace_back(nodes_); + if (deliveredAmount_.has_value()) + metaData.setFieldAmount(sfDeliveredAmount, *deliveredAmount_); - if (hasParentBatchId()) - metaData.setFieldH256(sfParentBatchID, getParentBatchId()); + if (parentBatchID_.has_value()) + metaData.setFieldH256(sfParentBatchID, *parentBatchID_); return metaData; } @@ -265,13 +236,13 @@ TxMeta::getAsObject() const void TxMeta::addRaw(Serializer& s, TER result, std::uint32_t index) { - mResult = TERtoInt(result); - mIndex = index; + result_ = TERtoInt(result); + index_ = index; XRPL_ASSERT( - (mResult == 0) || ((mResult > 100) && (mResult <= 255)), + (result_ == 0) || ((result_ > 100) && (result_ <= 255)), "ripple::TxMeta::addRaw : valid TER input"); - mNodes.sort([](STObject const& o1, STObject const& o2) { + nodes_.sort([](STObject const& o1, STObject const& o2) { return o1.getFieldH256(sfLedgerIndex) < o2.getFieldH256(sfLedgerIndex); }); diff --git a/src/xrpld/rpc/detail/DeliveredAmount.cpp b/src/xrpld/rpc/detail/DeliveredAmount.cpp index b38210d19e..830b456ad6 100644 --- a/src/xrpld/rpc/detail/DeliveredAmount.cpp +++ b/src/xrpld/rpc/detail/DeliveredAmount.cpp @@ -50,9 +50,10 @@ getDeliveredAmount( if (!serializedTx) return {}; - if (transactionMeta.hasDeliveredAmount()) + if (auto const& deliveredAmount = transactionMeta.getDeliveredAmount(); + deliveredAmount.has_value()) { - return transactionMeta.getDeliveredAmount(); + return *deliveredAmount; } if (serializedTx->isFieldPresent(sfAmount)) From dfafb141cc76008eb9ec6067f42c967ba5649de6 Mon Sep 17 00:00:00 2001 From: Jingchen Date: Fri, 31 Oct 2025 20:01:12 +0000 Subject: [PATCH 3/9] refactor: Retire fixAmendmentMajorityCalc amendment (#5961) Amendments activated for more than 2 years can be retired. This change retires the fixAmendmentMajorityCalc amendment. --- include/xrpl/protocol/SystemParameters.h | 10 ++---- include/xrpl/protocol/detail/features.macro | 2 +- src/test/app/AmendmentTable_test.cpp | 13 +------- src/xrpld/app/misc/detail/AmendmentTable.cpp | 32 ++++---------------- 4 files changed, 10 insertions(+), 47 deletions(-) diff --git a/include/xrpl/protocol/SystemParameters.h b/include/xrpl/protocol/SystemParameters.h index 121435dd92..5aa21827fc 100644 --- a/include/xrpl/protocol/SystemParameters.h +++ b/include/xrpl/protocol/SystemParameters.h @@ -73,14 +73,8 @@ static constexpr std::uint32_t XRP_LEDGER_EARLIEST_SEQ{32570u}; * used in asserts and tests. */ static constexpr std::uint32_t XRP_LEDGER_EARLIEST_FEES{562177u}; -/** The minimum amount of support an amendment should have. - - @note This value is used by legacy code and will become obsolete - once the fixAmendmentMajorityCalc amendment activates. -*/ -constexpr std::ratio<204, 256> preFixAmendmentMajorityCalcThreshold; - -constexpr std::ratio<80, 100> postFixAmendmentMajorityCalcThreshold; +/** The minimum amount of support an amendment should have. */ +constexpr std::ratio<80, 100> amendmentMajorityCalcThreshold; /** The minimum amount of time an amendment must hold a majority */ constexpr std::chrono::seconds const defaultAmendmentMajorityTime = weeks{2}; diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index d510edaa43..b6bdcd5c06 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -92,7 +92,6 @@ XRPL_FEATURE(CheckCashMakesTrustLine, Supported::yes, VoteBehavior::DefaultNo XRPL_FEATURE(FlowSortStrands, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(TicketBatch, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(NegativeUNL, Supported::yes, VoteBehavior::DefaultYes) -XRPL_FIX (AmendmentMajorityCalc, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(HardenedValidations, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(RequireFullyCanonicalSig, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(DeletableAccounts, Supported::yes, VoteBehavior::DefaultYes) @@ -139,6 +138,7 @@ XRPL_RETIRE(fix1571) XRPL_RETIRE(fix1578) XRPL_RETIRE(fix1623) XRPL_RETIRE(fix1781) +XRPL_RETIRE(fixAmendmentMajorityCalc) XRPL_RETIRE(fixCheckThreading) XRPL_RETIRE(fixQualityUpperBound) XRPL_RETIRE(fixRmSmallIncreasedQOffers) diff --git a/src/test/app/AmendmentTable_test.cpp b/src/test/app/AmendmentTable_test.cpp index 407b2fafe1..35f59ea2d8 100644 --- a/src/test/app/AmendmentTable_test.cpp +++ b/src/test/app/AmendmentTable_test.cpp @@ -545,8 +545,7 @@ public: for (auto const& [hash, nVotes] : votes) { - if (rules.enabled(fixAmendmentMajorityCalc) ? nVotes >= i - : nVotes > i) + if (nVotes >= i) { // We vote yes on this amendment field.push_back(hash); @@ -982,10 +981,6 @@ public: void testChangedUNL(FeatureBitset const& feat) { - // This test doesn't work without fixAmendmentMajorityCalc enabled. - if (!feat[fixAmendmentMajorityCalc]) - return; - testcase("changedUNL"); auto const testAmendment = amendmentId("changedUNL"); @@ -1143,10 +1138,6 @@ public: void testValidatorFlapping(FeatureBitset const& feat) { - // This test doesn't work without fixAmendmentMajorityCalc enabled. - if (!feat[fixAmendmentMajorityCalc]) - return; - testcase("validatorFlapping"); // We run a test where a validator flaps on and off every 23 hours @@ -1289,14 +1280,12 @@ public: run() override { FeatureBitset const all{test::jtx::testable_amendments()}; - FeatureBitset const fixMajorityCalc{fixAmendmentMajorityCalc}; testConstruct(); testGet(); testBadConfig(); testEnableVeto(); testHasUnsupported(); - testFeature(all - fixMajorityCalc); testFeature(all); } }; diff --git a/src/xrpld/app/misc/detail/AmendmentTable.cpp b/src/xrpld/app/misc/detail/AmendmentTable.cpp index b13e40c3ae..9f73879700 100644 --- a/src/xrpld/app/misc/detail/AmendmentTable.cpp +++ b/src/xrpld/app/misc/detail/AmendmentTable.cpp @@ -316,36 +316,16 @@ class AmendmentSet private: // How many yes votes each amendment received hash_map votes_; - Rules const& rules_; // number of trusted validations int trustedValidations_ = 0; // number of votes needed int threshold_ = 0; - void - computeThreshold(int trustedValidations, Rules const& rules) - { - threshold_ = !rules_.enabled(fixAmendmentMajorityCalc) - ? std::max( - 1L, - static_cast( - (trustedValidations_ * - preFixAmendmentMajorityCalcThreshold.num) / - preFixAmendmentMajorityCalcThreshold.den)) - : std::max( - 1L, - static_cast( - (trustedValidations_ * - postFixAmendmentMajorityCalcThreshold.num) / - postFixAmendmentMajorityCalcThreshold.den)); - } - public: AmendmentSet( Rules const& rules, TrustedVotes const& trustedVotes, std::lock_guard const& lock) - : rules_(rules) { // process validations for ledger before flag ledger. auto [trustedCount, newVotes] = trustedVotes.getVotes(rules, lock); @@ -353,7 +333,11 @@ public: trustedValidations_ = trustedCount; votes_.swap(newVotes); - computeThreshold(trustedValidations_, rules); + threshold_ = std::max( + 1L, + static_cast( + (trustedValidations_ * amendmentMajorityCalcThreshold.num) / + amendmentMajorityCalcThreshold.den)); } bool @@ -364,13 +348,9 @@ public: if (it == votes_.end()) return false; - // Before this fix, it was possible for an amendment to activate with a - // percentage slightly less than 80% because we compared for "greater - // than or equal to" instead of strictly "greater than". // One validator is an exception, otherwise it is not possible // to gain majority. - if (!rules_.enabled(fixAmendmentMajorityCalc) || - trustedValidations_ == 1) + if (trustedValidations_ == 1) return it->second >= threshold_; return it->second > threshold_; From ab45a8a737680f1fb39eb336732a7fbedd431254 Mon Sep 17 00:00:00 2001 From: Jingchen Date: Fri, 31 Oct 2025 20:25:05 +0000 Subject: [PATCH 4/9] refactor: Retire fixReducedOffersV1 amendment (#5972) Amendments activated for more than 2 years can be retired. This change retires the fixReducedOffersV1 amendment. --- include/xrpl/protocol/detail/features.macro | 2 +- src/test/app/ReducedOffer_test.cpp | 94 ++++----------------- src/xrpld/app/paths/detail/AMMOffer.cpp | 11 +-- src/xrpld/app/tx/detail/CreateOffer.cpp | 18 +--- src/xrpld/app/tx/detail/Offer.h | 11 +-- src/xrpld/app/tx/detail/OfferStream.cpp | 16 +--- 6 files changed, 29 insertions(+), 123 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index b6bdcd5c06..4594a4f856 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -77,7 +77,6 @@ XRPL_FIX (DisallowIncomingV1, Supported::yes, VoteBehavior::DefaultNo XRPL_FEATURE(XChainBridge, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(AMM, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(Clawback, Supported::yes, VoteBehavior::DefaultNo) -XRPL_FIX (ReducedOffersV1, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (NFTokenRemint, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (NonFungibleTokensV1_2, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (UniversalNumber, Supported::yes, VoteBehavior::DefaultNo) @@ -141,6 +140,7 @@ XRPL_RETIRE(fix1781) XRPL_RETIRE(fixAmendmentMajorityCalc) XRPL_RETIRE(fixCheckThreading) XRPL_RETIRE(fixQualityUpperBound) +XRPL_RETIRE(fixReducedOffersV1) XRPL_RETIRE(fixRmSmallIncreasedQOffers) XRPL_RETIRE(fixSTAmountCanonicalize) XRPL_RETIRE(fixTakerDryOfferRemoval) diff --git a/src/test/app/ReducedOffer_test.cpp b/src/test/app/ReducedOffer_test.cpp index fa2be451fa..c991daef85 100644 --- a/src/test/app/ReducedOffer_test.cpp +++ b/src/test/app/ReducedOffer_test.cpp @@ -80,12 +80,8 @@ public: auto const bob = Account{"bob"}; auto const USD = gw["USD"]; - // Make one test run without fixReducedOffersV1 and one with. - for (FeatureBitset features : - {testable_amendments() - fixReducedOffersV1, - testable_amendments() | fixReducedOffersV1}) { - Env env{*this, features}; + Env env{*this, testable_amendments()}; // Make sure none of the offers we generate are under funded. env.fund(XRP(10'000'000), gw, alice, bob); @@ -208,19 +204,9 @@ public: blockedCount += exerciseOfferPair(alicesOffer, bobsOffer); } - // If fixReducedOffersV1 is enabled, then none of the test cases - // should produce a potentially blocking rate. - // - // Also verify that if fixReducedOffersV1 is not enabled then - // some of the test cases produced a potentially blocking rate. - if (features[fixReducedOffersV1]) - { - BEAST_EXPECT(blockedCount == 0); - } - else - { - BEAST_EXPECT(blockedCount >= 170); - } + // None of the test cases should produce a potentially blocking + // rate. + BEAST_EXPECT(blockedCount == 0); } } @@ -236,13 +222,9 @@ public: auto const bob = Account{"bob"}; auto const USD = gw["USD"]; - // Make one test run without fixReducedOffersV1 and one with. - for (FeatureBitset features : - {testable_amendments() - fixReducedOffersV1, - testable_amendments() | fixReducedOffersV1}) { // Make sure none of the offers we generate are under funded. - Env env{*this, features}; + Env env{*this, testable_amendments()}; env.fund(XRP(10'000'000), gw, alice, bob); env.close(); @@ -367,19 +349,9 @@ public: blockedCount += exerciseOfferPair(aliceOffer, bobsOffer); } - // If fixReducedOffersV1 is enabled, then none of the test cases - // should produce a potentially blocking rate. - // - // Also verify that if fixReducedOffersV1 is not enabled then - // some of the test cases produced a potentially blocking rate. - if (features[fixReducedOffersV1]) - { - BEAST_EXPECT(blockedCount == 0); - } - else - { - BEAST_EXPECT(blockedCount > 10); - } + // None of the test cases should produce a potentially blocking + // rate. + BEAST_EXPECT(blockedCount == 0); } } @@ -389,9 +361,6 @@ public: testcase("exercise underfunded XRP/IOU offer Q change"); // Bob places an offer that is not fully funded. - // - // This unit test compares the behavior of this situation before and - // after applying the fixReducedOffersV1 amendment. using namespace jtx; auto const alice = Account{"alice"}; @@ -399,12 +368,8 @@ public: auto const gw = Account{"gw"}; auto const USD = gw["USD"]; - // Make one test run without fixReducedOffersV1 and one with. - for (FeatureBitset features : - {testable_amendments() - fixReducedOffersV1, - testable_amendments() | fixReducedOffersV1}) { - Env env{*this, features}; + Env env{*this, testable_amendments()}; env.fund(XRP(10000), alice, bob, gw); env.close(); @@ -470,19 +435,9 @@ public: } } - // If fixReducedOffersV1 is enabled, then none of the test cases - // should produce a potentially blocking rate. - // - // Also verify that if fixReducedOffersV1 is not enabled then - // some of the test cases produced a potentially blocking rate. - if (features[fixReducedOffersV1]) - { - BEAST_EXPECT(blockedOrderBookCount == 0); - } - else - { - BEAST_EXPECT(blockedOrderBookCount > 15); - } + // None of the test cases should produce a potentially blocking + // rate. + BEAST_EXPECT(blockedOrderBookCount == 0); } } @@ -492,9 +447,6 @@ public: testcase("exercise underfunded IOU/IOU offer Q change"); // Bob places an IOU/IOU offer that is not fully funded. - // - // This unit test compares the behavior of this situation before and - // after applying the fixReducedOffersV1 amendment. using namespace jtx; using namespace std::chrono_literals; @@ -507,12 +459,8 @@ public: STAmount const tinyUSD(USD.issue(), /*mantissa*/ 1, /*exponent*/ -81); - // Make one test run without fixReducedOffersV1 and one with. - for (FeatureBitset features : - {testable_amendments() - fixReducedOffersV1, - testable_amendments() | fixReducedOffersV1}) { - Env env{*this, features}; + Env env{*this, testable_amendments()}; env.fund(XRP(10000), alice, bob, gw); env.close(); @@ -594,19 +542,9 @@ public: env.close(); } - // If fixReducedOffersV1 is enabled, then none of the test cases - // should produce a potentially blocking rate. - // - // Also verify that if fixReducedOffersV1 is not enabled then - // some of the test cases produced a potentially blocking rate. - if (features[fixReducedOffersV1]) - { - BEAST_EXPECT(blockedOrderBookCount == 0); - } - else - { - BEAST_EXPECT(blockedOrderBookCount > 20); - } + // None of the test cases should produce a potentially blocking + // rate. + BEAST_EXPECT(blockedOrderBookCount == 0); } } diff --git a/src/xrpld/app/paths/detail/AMMOffer.cpp b/src/xrpld/app/paths/detail/AMMOffer.cpp index c719c0a92d..aa55a9759e 100644 --- a/src/xrpld/app/paths/detail/AMMOffer.cpp +++ b/src/xrpld/app/paths/detail/AMMOffer.cpp @@ -92,14 +92,9 @@ AMMOffer::limitOut( // poolPays * poolGets < (poolPays - assetOut) * (poolGets + assetIn) if (ammLiquidity_.multiPath()) { - if (auto const& rules = getCurrentTransactionRules(); - rules && rules->enabled(fixReducedOffersV1)) - // It turns out that the ceil_out implementation has some slop in - // it. ceil_out_strict removes that slop. But removing that slop - // affects transaction outcomes, so the change must be made using - // an amendment. - return quality().ceil_out_strict(offrAmt, limit, roundUp); - return quality().ceil_out(offrAmt, limit); + // It turns out that the ceil_out implementation has some slop in + // it, which ceil_out_strict removes. + return quality().ceil_out_strict(offrAmt, limit, roundUp); } // Change the offer size according to the conservation function. The offer // quality is increased in this case, but it doesn't matter since there is diff --git a/src/xrpld/app/tx/detail/CreateOffer.cpp b/src/xrpld/app/tx/detail/CreateOffer.cpp index 6303918ef2..a7884a825c 100644 --- a/src/xrpld/app/tx/detail/CreateOffer.cpp +++ b/src/xrpld/app/tx/detail/CreateOffer.cpp @@ -477,22 +477,8 @@ CreateOffer::flowCross( // what is a good threshold to check? afterCross.in.clear(); - afterCross.out = [&]() { - // Careful analysis showed that rounding up this - // divRound result could lead to placing a reduced - // offer in the ledger that blocks order books. So - // the fixReducedOffersV1 amendment changes the - // behavior to round down instead. - if (psb.rules().enabled(fixReducedOffersV1)) - return divRoundStrict( - afterCross.in, - rate, - takerAmount.out.issue(), - false); - - return divRound( - afterCross.in, rate, takerAmount.out.issue(), true); - }(); + afterCross.out = divRoundStrict( + afterCross.in, rate, takerAmount.out.issue(), false); } else { diff --git a/src/xrpld/app/tx/detail/Offer.h b/src/xrpld/app/tx/detail/Offer.h index 58bd65ce46..8c2b14af6c 100644 --- a/src/xrpld/app/tx/detail/Offer.h +++ b/src/xrpld/app/tx/detail/Offer.h @@ -241,14 +241,9 @@ TOffer::limitOut( TOut const& limit, bool roundUp) const { - if (auto const& rules = getCurrentTransactionRules(); - rules && rules->enabled(fixReducedOffersV1)) - // It turns out that the ceil_out implementation has some slop in - // it. ceil_out_strict removes that slop. But removing that slop - // affects transaction outcomes, so the change must be made using - // an amendment. - return quality().ceil_out_strict(offrAmt, limit, roundUp); - return m_quality.ceil_out(offrAmt, limit); + // It turns out that the ceil_out implementation has some slop in + // it, which ceil_out_strict removes. + return quality().ceil_out_strict(offrAmt, limit, roundUp); } template diff --git a/src/xrpld/app/tx/detail/OfferStream.cpp b/src/xrpld/app/tx/detail/OfferStream.cpp index 8f19c20cf5..9b638d322c 100644 --- a/src/xrpld/app/tx/detail/OfferStream.cpp +++ b/src/xrpld/app/tx/detail/OfferStream.cpp @@ -184,7 +184,6 @@ TOfferStreamBase::shouldRmSmallIncreasedQOffer() const } TTakerGets const ownerFunds = toAmount(*ownerFunds_); - bool const fixReduced = view_.rules().enabled(fixReducedOffersV1); auto const effectiveAmounts = [&] { if (offer_.owner() != offer_.issueOut().account && @@ -193,22 +192,15 @@ TOfferStreamBase::shouldRmSmallIncreasedQOffer() const // adjust the amounts by owner funds. // // It turns out we can prevent order book blocking by rounding down - // the ceil_out() result. This adjustment changes transaction - // results, so it must be made under an amendment. - if (fixReduced) - return offer_.quality().ceil_out_strict( - ofrAmts, ownerFunds, /* roundUp */ false); - - return offer_.quality().ceil_out(ofrAmts, ownerFunds); + // the ceil_out() result. + return offer_.quality().ceil_out_strict( + ofrAmts, ownerFunds, /* roundUp */ false); } return ofrAmts; }(); // If either the effective in or out are zero then remove the offer. - // This can happen with fixReducedOffersV1 since it rounds down. - if (fixReduced && - (effectiveAmounts.in.signum() <= 0 || - effectiveAmounts.out.signum() <= 0)) + if (effectiveAmounts.in.signum() <= 0 || effectiveAmounts.out.signum() <= 0) return true; if (effectiveAmounts.in > TTakerPays::minPositiveAmount()) From 50fc93f742ce199ac6017dd7665eecb62c3b9102 Mon Sep 17 00:00:00 2001 From: Jingchen Date: Fri, 31 Oct 2025 21:01:44 +0000 Subject: [PATCH 5/9] refactor: Retire fixMasterKeyAsRegularKey amendment (#5959) Amendments activated for more than 2 years can be retired. This change retires the fixMasterKeyAsRegularKey amendment. --- include/xrpl/protocol/detail/features.macro | 2 +- src/test/app/SetRegularKey_test.cpp | 85 ++------------------- src/test/jtx/Env_test.cpp | 2 +- src/test/rpc/Feature_test.cpp | 17 ----- src/xrpld/app/tx/detail/SetRegularKey.cpp | 3 +- src/xrpld/app/tx/detail/Transactor.cpp | 57 ++++---------- 6 files changed, 22 insertions(+), 144 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 4594a4f856..b8398f6da5 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -95,7 +95,6 @@ XRPL_FEATURE(HardenedValidations, Supported::yes, VoteBehavior::DefaultYe XRPL_FEATURE(RequireFullyCanonicalSig, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(DeletableAccounts, Supported::yes, VoteBehavior::DefaultYes) XRPL_FIX (PayChanRecipientOwnerDir, Supported::yes, VoteBehavior::DefaultYes) -XRPL_FIX (MasterKeyAsRegularKey, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(MultiSignReserve, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(DepositPreauth, Supported::yes, VoteBehavior::DefaultYes) XRPL_FEATURE(Checks, Supported::yes, VoteBehavior::DefaultYes) @@ -139,6 +138,7 @@ XRPL_RETIRE(fix1623) XRPL_RETIRE(fix1781) XRPL_RETIRE(fixAmendmentMajorityCalc) XRPL_RETIRE(fixCheckThreading) +XRPL_RETIRE(fixMasterKeyAsRegularKey) XRPL_RETIRE(fixQualityUpperBound) XRPL_RETIRE(fixReducedOffersV1) XRPL_RETIRE(fixRmSmallIncreasedQOffers) diff --git a/src/test/app/SetRegularKey_test.cpp b/src/test/app/SetRegularKey_test.cpp index 78b75fc458..3bceec335c 100644 --- a/src/test/app/SetRegularKey_test.cpp +++ b/src/test/app/SetRegularKey_test.cpp @@ -27,52 +27,12 @@ class SetRegularKey_test : public beast::unit_test::suite { public: void - testDisableMasterKey() + testDisabledMasterKey() { using namespace test::jtx; testcase("Set regular key"); - Env env{*this, testable_amendments() - fixMasterKeyAsRegularKey}; - Account const alice("alice"); - Account const bob("bob"); - env.fund(XRP(10000), alice, bob); - - env(regkey(alice, bob)); - auto const ar = env.le(alice); - BEAST_EXPECT( - ar->isFieldPresent(sfRegularKey) && - (ar->getAccountID(sfRegularKey) == bob.id())); - - env(noop(alice), sig(bob)); - env(noop(alice), sig(alice)); - - testcase("Disable master key"); - env(fset(alice, asfDisableMaster), sig(alice)); - env(noop(alice), sig(bob)); - env(noop(alice), sig(alice), ter(tefMASTER_DISABLED)); - - testcase("Re-enable master key"); - env(fclear(alice, asfDisableMaster), - sig(alice), - ter(tefMASTER_DISABLED)); - - env(fclear(alice, asfDisableMaster), sig(bob)); - env(noop(alice), sig(bob)); - env(noop(alice), sig(alice)); - - testcase("Revoke regular key"); - env(regkey(alice, disabled)); - env(noop(alice), sig(bob), ter(tefBAD_AUTH_MASTER)); - env(noop(alice), sig(alice)); - } - - void - testDisableMasterKeyAfterFix() - { - using namespace test::jtx; - - testcase("Set regular key"); - Env env{*this, testable_amendments() | fixMasterKeyAsRegularKey}; + Env env{*this, testable_amendments()}; Account const alice("alice"); Account const bob("bob"); env.fund(XRP(10000), alice, bob); @@ -106,44 +66,11 @@ public: { using namespace test::jtx; - // See https://ripplelabs.atlassian.net/browse/RIPD-1721. - testcase( - "Set regular key to master key (before fixMasterKeyAsRegularKey)"); - Env env{*this, testable_amendments() - fixMasterKeyAsRegularKey}; + testcase("Set regular key to master key"); + Env env{*this, testable_amendments()}; Account const alice("alice"); env.fund(XRP(10000), alice); - // Must be possible unless amendment `fixMasterKeyAsRegularKey` enabled. - env(regkey(alice, alice), sig(alice)); - env(fset(alice, asfDisableMaster), sig(alice)); - - // No way to sign... - env(noop(alice), ter(tefMASTER_DISABLED)); - env(noop(alice), sig(alice), ter(tefMASTER_DISABLED)); - - // ... until now. - env.enableFeature(fixMasterKeyAsRegularKey); - env(noop(alice)); - env(noop(alice), sig(alice)); - - env(regkey(alice, disabled), ter(tecNO_ALTERNATIVE_KEY)); - env(fclear(alice, asfDisableMaster)); - env(regkey(alice, disabled)); - env(fset(alice, asfDisableMaster), ter(tecNO_ALTERNATIVE_KEY)); - } - - void - testDisableRegularKeyAfterFix() - { - using namespace test::jtx; - - testcase( - "Set regular key to master key (after fixMasterKeyAsRegularKey)"); - Env env{*this, testable_amendments() | fixMasterKeyAsRegularKey}; - Account const alice("alice"); - env.fund(XRP(10000), alice); - - // Must be possible unless amendment `fixMasterKeyAsRegularKey` enabled. env(regkey(alice, alice), ter(temBAD_REGKEY)); } @@ -253,10 +180,8 @@ public: void run() override { - testDisableMasterKey(); - testDisableMasterKeyAfterFix(); + testDisabledMasterKey(); testDisabledRegularKey(); - testDisableRegularKeyAfterFix(); testPasswordSpent(); testUniversalMask(); testTicketRegularKey(); diff --git a/src/test/jtx/Env_test.cpp b/src/test/jtx/Env_test.cpp index 34d9f6c0e8..8f1f30c241 100644 --- a/src/test/jtx/Env_test.cpp +++ b/src/test/jtx/Env_test.cpp @@ -265,7 +265,7 @@ public: { using namespace jtx; - Env env{*this, testable_amendments() | fixMasterKeyAsRegularKey}; + Env env{*this, testable_amendments()}; Account const alice("alice", KeyType::ed25519); Account const bob("bob", KeyType::secp256k1); Account const carol("carol"); diff --git a/src/test/rpc/Feature_test.cpp b/src/test/rpc/Feature_test.cpp index f01ee5b116..ea847a5a83 100644 --- a/src/test/rpc/Feature_test.cpp +++ b/src/test/rpc/Feature_test.cpp @@ -332,23 +332,6 @@ class Feature_test : public beast::unit_test::suite result[jss::error_message] == "You don't have permission for this command."); } - - { - std::string const feature = - "C4483A1896170C66C098DEA5B0E024309C60DC960DE5F01CD7AF986AA3D9AD" - "37"; - Json::Value params; - params[jss::feature] = feature; - auto const result = - env.rpc("json", "feature", to_string(params))[jss::result]; - BEAST_EXPECT(result.isMember(feature)); - auto const amendmentResult = result[feature]; - BEAST_EXPECT(amendmentResult[jss::enabled].asBool() == false); - BEAST_EXPECT(amendmentResult[jss::supported].asBool() == true); - BEAST_EXPECT( - amendmentResult[jss::name].asString() == - "fixMasterKeyAsRegularKey"); - } } void diff --git a/src/xrpld/app/tx/detail/SetRegularKey.cpp b/src/xrpld/app/tx/detail/SetRegularKey.cpp index 93d5899861..96d97c8302 100644 --- a/src/xrpld/app/tx/detail/SetRegularKey.cpp +++ b/src/xrpld/app/tx/detail/SetRegularKey.cpp @@ -51,8 +51,7 @@ SetRegularKey::calculateBaseFee(ReadView const& view, STTx const& tx) NotTEC SetRegularKey::preflight(PreflightContext const& ctx) { - if (ctx.rules.enabled(fixMasterKeyAsRegularKey) && - ctx.tx.isFieldPresent(sfRegularKey) && + if (ctx.tx.isFieldPresent(sfRegularKey) && (ctx.tx.getAccountID(sfRegularKey) == ctx.tx.getAccountID(sfAccount))) { return temBAD_REGKEY; diff --git a/src/xrpld/app/tx/detail/Transactor.cpp b/src/xrpld/app/tx/detail/Transactor.cpp index 8af5ccf232..656f0800c2 100644 --- a/src/xrpld/app/tx/detail/Transactor.cpp +++ b/src/xrpld/app/tx/detail/Transactor.cpp @@ -788,55 +788,26 @@ Transactor::checkSingleSign( { bool const isMasterDisabled = sleAccount->isFlag(lsfDisableMaster); - if (view.rules().enabled(fixMasterKeyAsRegularKey)) + // Signed with regular key. + if ((*sleAccount)[~sfRegularKey] == idSigner) { - // Signed with regular key. - if ((*sleAccount)[~sfRegularKey] == idSigner) - { - return tesSUCCESS; - } - - // Signed with enabled mater key. - if (!isMasterDisabled && idAccount == idSigner) - { - return tesSUCCESS; - } - - // Signed with disabled master key. - if (isMasterDisabled && idAccount == idSigner) - { - return tefMASTER_DISABLED; - } - - // Signed with any other key. - return tefBAD_AUTH; + return tesSUCCESS; } - if (idSigner == idAccount) + // Signed with enabled master key. + if (!isMasterDisabled && idAccount == idSigner) { - // Signing with the master key. Continue if it is not disabled. - if (isMasterDisabled) - return tefMASTER_DISABLED; - } - else if ((*sleAccount)[~sfRegularKey] == idSigner) - { - // Signing with the regular key. Continue. - } - else if (sleAccount->isFieldPresent(sfRegularKey)) - { - // Signing key does not match master or regular key. - JLOG(j.trace()) << "checkSingleSign: Not authorized to use account."; - return tefBAD_AUTH; - } - else - { - // No regular key on account and signing key does not match master key. - // FIXME: Why differentiate this case from tefBAD_AUTH? - JLOG(j.trace()) << "checkSingleSign: Not authorized to use account."; - return tefBAD_AUTH_MASTER; + return tesSUCCESS; } - return tesSUCCESS; + // Signed with disabled master key. + if (isMasterDisabled && idAccount == idSigner) + { + return tefMASTER_DISABLED; + } + + // Signed with any other key. + return tefBAD_AUTH; } NotTEC From 8eb233c2ea8ad5a159be73b77f0f5e1496d547ac Mon Sep 17 00:00:00 2001 From: Jingchen Date: Fri, 31 Oct 2025 22:25:16 +0000 Subject: [PATCH 6/9] refactor: Modularize shamap and nodestore (#5668) This change moves the shamap and nodestore from `xrpld` to `libxrpl`. --- .../scripts/levelization/results/loops.txt | 2 +- .../scripts/levelization/results/ordering.txt | 37 +++++++++++-------- cmake/RippledCore.cmake | 28 +++++++++++--- cmake/RippledInstall.cmake | 11 ++++-- .../unity => include/xrpl/basics}/rocksdb.h | 0 .../xrpl}/nodestore/Backend.h | 2 +- .../xrpl}/nodestore/Database.h | 7 ++-- .../xrpl}/nodestore/DatabaseRotating.h | 2 +- .../xrpl}/nodestore/DummyScheduler.h | 2 +- .../xrpl}/nodestore/Factory.h | 5 +-- .../xrpl}/nodestore/Manager.h | 4 +- .../xrpl}/nodestore/NodeObject.h | 0 .../xrpl}/nodestore/README.md | 0 .../xrpl}/nodestore/Scheduler.h | 2 +- {src/xrpld => include/xrpl}/nodestore/Task.h | 0 {src/xrpld => include/xrpl}/nodestore/Types.h | 2 +- .../xrpl}/nodestore/detail/BatchWriter.h | 6 +-- .../xrpl}/nodestore/detail/DatabaseNodeImp.h | 3 +- .../nodestore/detail/DatabaseRotatingImp.h | 2 +- .../xrpl}/nodestore/detail/DecodedBlob.h | 2 +- .../xrpl}/nodestore/detail/EncodedBlob.h | 3 +- .../xrpl}/nodestore/detail/ManagerImp.h | 4 +- .../xrpl}/nodestore/detail/codec.h | 5 +-- .../xrpl}/nodestore/detail/varint.h | 0 {src/xrpld => include/xrpl}/shamap/Family.h | 7 ++-- .../xrpl}/shamap/FullBelowCache.h | 0 {src/xrpld => include/xrpl}/shamap/README.md | 0 {src/xrpld => include/xrpl}/shamap/SHAMap.h | 20 +++++----- .../xrpl}/shamap/SHAMapAccountStateLeafNode.h | 5 +-- .../xrpl}/shamap/SHAMapAddNode.h | 0 .../xrpl}/shamap/SHAMapInnerNode.h | 5 +-- .../xrpl}/shamap/SHAMapItem.h | 0 .../xrpl}/shamap/SHAMapLeafNode.h | 4 +- .../xrpl}/shamap/SHAMapMissingNode.h | 3 +- .../xrpl}/shamap/SHAMapNodeID.h | 0 .../xrpl}/shamap/SHAMapSyncFilter.h | 2 +- .../xrpl}/shamap/SHAMapTreeNode.h | 5 +-- .../xrpl}/shamap/SHAMapTxLeafNode.h | 5 +-- .../xrpl}/shamap/SHAMapTxPlusMetaLeafNode.h | 5 +-- .../xrpl}/shamap/TreeNodeCache.h | 3 +- .../xrpl}/shamap/detail/TaggedPointer.h | 3 +- .../xrpl}/shamap/detail/TaggedPointer.ipp | 5 +-- .../nodestore}/BatchWriter.cpp | 2 +- .../detail => libxrpl/nodestore}/Database.cpp | 3 +- .../nodestore}/DatabaseNodeImp.cpp | 2 +- .../nodestore}/DatabaseRotatingImp.cpp | 2 +- .../nodestore}/DecodedBlob.cpp | 3 +- .../nodestore}/DummyScheduler.cpp | 2 +- .../nodestore}/ManagerImp.cpp | 25 ++++++++++++- .../nodestore}/NodeObject.cpp | 2 +- .../nodestore/backend/MemoryFactory.cpp | 31 ++++++++-------- .../nodestore/backend/NuDBFactory.cpp | 29 ++++++++------- .../nodestore/backend/NullFactory.cpp | 22 ++++++----- .../nodestore/backend/RocksDBFactory.cpp | 32 ++++++++-------- .../detail => libxrpl/shamap}/SHAMap.cpp | 13 +++---- .../detail => libxrpl/shamap}/SHAMapDelta.cpp | 3 +- .../shamap}/SHAMapInnerNode.cpp | 7 ++-- .../shamap}/SHAMapLeafNode.cpp | 2 +- .../shamap}/SHAMapNodeID.cpp | 5 +-- .../detail => libxrpl/shamap}/SHAMapSync.cpp | 7 ++-- .../shamap}/SHAMapTreeNode.cpp | 11 +++--- src/test/app/SHAMapStore_test.cpp | 2 +- src/test/nodestore/Backend_test.cpp | 7 ++-- src/test/nodestore/Basics_test.cpp | 4 +- src/test/nodestore/Database_test.cpp | 4 +- src/test/nodestore/NuDBFactory_test.cpp | 5 +-- src/test/nodestore/TestBase.h | 7 ++-- src/test/nodestore/Timing_test.cpp | 5 +-- src/test/nodestore/import_test.cpp | 5 +-- src/test/nodestore/varint_test.cpp | 3 +- src/test/overlay/compression_test.cpp | 2 +- src/test/shamap/FetchPack_test.cpp | 5 +-- src/test/shamap/SHAMapSync_test.cpp | 5 +-- src/test/shamap/SHAMap_test.cpp | 3 +- src/test/shamap/common.h | 7 ++-- .../app/consensus/RCLCensorshipDetector.h | 3 +- src/xrpld/app/consensus/RCLConsensus.h | 2 +- src/xrpld/app/consensus/RCLCxTx.h | 2 +- src/xrpld/app/ledger/AccountStateSF.h | 5 ++- src/xrpld/app/ledger/ConsensusTransSetSF.cpp | 2 +- src/xrpld/app/ledger/ConsensusTransSetSF.h | 2 +- src/xrpld/app/ledger/InboundTransactions.h | 2 +- src/xrpld/app/ledger/Ledger.cpp | 4 +- src/xrpld/app/ledger/Ledger.h | 2 +- src/xrpld/app/ledger/TransactionMaster.h | 4 +- src/xrpld/app/ledger/TransactionStateSF.h | 5 ++- src/xrpld/app/ledger/detail/InboundLedger.cpp | 2 +- src/xrpld/app/ledger/detail/SkipListAcquire.h | 3 +- .../app/ledger/detail/TransactionAcquire.h | 3 +- src/xrpld/app/main/Application.cpp | 2 +- src/xrpld/app/main/Application.h | 2 +- src/xrpld/app/main/NodeStoreScheduler.h | 3 +- src/xrpld/app/misc/FeeVote.h | 3 +- src/xrpld/app/misc/NegativeUNLVote.cpp | 3 +- src/xrpld/app/misc/SHAMapStore.h | 3 +- src/xrpld/app/misc/SHAMapStoreImp.cpp | 6 +-- src/xrpld/app/misc/SHAMapStoreImp.h | 5 ++- src/xrpld/rpc/handlers/GetCounts.cpp | 2 +- src/xrpld/shamap/{detail => }/NodeFamily.cpp | 3 +- src/xrpld/shamap/NodeFamily.h | 3 +- 100 files changed, 282 insertions(+), 262 deletions(-) rename {src/xrpld/unity => include/xrpl/basics}/rocksdb.h (100%) rename {src/xrpld => include/xrpl}/nodestore/Backend.h (99%) rename {src/xrpld => include/xrpl}/nodestore/Database.h (98%) rename {src/xrpld => include/xrpl}/nodestore/DatabaseRotating.h (98%) rename {src/xrpld => include/xrpl}/nodestore/DummyScheduler.h (97%) rename {src/xrpld => include/xrpl}/nodestore/Factory.h (97%) rename {src/xrpld => include/xrpl}/nodestore/Manager.h (97%) rename {src/xrpld => include/xrpl}/nodestore/NodeObject.h (100%) rename {src/xrpld => include/xrpl}/nodestore/README.md (100%) rename {src/xrpld => include/xrpl}/nodestore/Scheduler.h (98%) rename {src/xrpld => include/xrpl}/nodestore/Task.h (100%) rename {src/xrpld => include/xrpl}/nodestore/Types.h (97%) rename {src/xrpld => include/xrpl}/nodestore/detail/BatchWriter.h (96%) rename {src/xrpld => include/xrpl}/nodestore/detail/DatabaseNodeImp.h (99%) rename {src/xrpld => include/xrpl}/nodestore/detail/DatabaseRotatingImp.h (98%) rename {src/xrpld => include/xrpl}/nodestore/detail/DecodedBlob.h (98%) rename {src/xrpld => include/xrpl}/nodestore/detail/EncodedBlob.h (99%) rename {src/xrpld => include/xrpl}/nodestore/detail/ManagerImp.h (96%) rename {src/xrpld => include/xrpl}/nodestore/detail/codec.h (99%) rename {src/xrpld => include/xrpl}/nodestore/detail/varint.h (100%) rename {src/xrpld => include/xrpl}/shamap/Family.h (95%) rename {src/xrpld => include/xrpl}/shamap/FullBelowCache.h (100%) rename {src/xrpld => include/xrpl}/shamap/README.md (100%) rename {src/xrpld => include/xrpl}/shamap/SHAMap.h (98%) rename {src/xrpld => include/xrpl}/shamap/SHAMapAccountStateLeafNode.h (97%) rename {src/xrpld => include/xrpl}/shamap/SHAMapAddNode.h (100%) rename {src/xrpld => include/xrpl}/shamap/SHAMapInnerNode.h (98%) rename {src/xrpld => include/xrpl}/shamap/SHAMapItem.h (100%) rename {src/xrpld => include/xrpl}/shamap/SHAMapLeafNode.h (96%) rename {src/xrpld => include/xrpl}/shamap/SHAMapMissingNode.h (98%) rename {src/xrpld => include/xrpl}/shamap/SHAMapNodeID.h (100%) rename {src/xrpld => include/xrpl}/shamap/SHAMapSyncFilter.h (97%) rename {src/xrpld => include/xrpl}/shamap/SHAMapTreeNode.h (98%) rename {src/xrpld => include/xrpl}/shamap/SHAMapTxLeafNode.h (97%) rename {src/xrpld => include/xrpl}/shamap/SHAMapTxPlusMetaLeafNode.h (97%) rename {src/xrpld => include/xrpl}/shamap/TreeNodeCache.h (97%) rename {src/xrpld => include/xrpl}/shamap/detail/TaggedPointer.h (99%) rename {src/xrpld => include/xrpl}/shamap/detail/TaggedPointer.ipp (99%) rename src/{xrpld/nodestore/detail => libxrpl/nodestore}/BatchWriter.cpp (98%) rename src/{xrpld/nodestore/detail => libxrpl/nodestore}/Database.cpp (99%) rename src/{xrpld/nodestore/detail => libxrpl/nodestore}/DatabaseNodeImp.cpp (99%) rename src/{xrpld/nodestore/detail => libxrpl/nodestore}/DatabaseRotatingImp.cpp (99%) rename src/{xrpld/nodestore/detail => libxrpl/nodestore}/DecodedBlob.cpp (98%) rename src/{xrpld/nodestore/detail => libxrpl/nodestore}/DummyScheduler.cpp (96%) rename src/{xrpld/nodestore/detail => libxrpl/nodestore}/ManagerImp.cpp (81%) rename src/{xrpld/nodestore/detail => libxrpl/nodestore}/NodeObject.cpp (97%) rename src/{xrpld => libxrpl}/nodestore/backend/MemoryFactory.cpp (92%) rename src/{xrpld => libxrpl}/nodestore/backend/NuDBFactory.cpp (96%) rename src/{xrpld => libxrpl}/nodestore/backend/NullFactory.cpp (90%) rename src/{xrpld => libxrpl}/nodestore/backend/RocksDBFactory.cpp (96%) rename src/{xrpld/shamap/detail => libxrpl/shamap}/SHAMap.cpp (99%) rename src/{xrpld/shamap/detail => libxrpl/shamap}/SHAMapDelta.cpp (99%) rename src/{xrpld/shamap/detail => libxrpl/shamap}/SHAMapInnerNode.cpp (99%) rename src/{xrpld/shamap/detail => libxrpl/shamap}/SHAMapLeafNode.cpp (98%) rename src/{xrpld/shamap/detail => libxrpl/shamap}/SHAMapNodeID.cpp (98%) rename src/{xrpld/shamap/detail => libxrpl/shamap}/SHAMapSync.cpp (99%) rename src/{xrpld/shamap/detail => libxrpl/shamap}/SHAMapTreeNode.cpp (95%) rename src/xrpld/shamap/{detail => }/NodeFamily.cpp (98%) diff --git a/.github/scripts/levelization/results/loops.txt b/.github/scripts/levelization/results/loops.txt index e998e962d4..d057391be9 100644 --- a/.github/scripts/levelization/results/loops.txt +++ b/.github/scripts/levelization/results/loops.txt @@ -17,7 +17,7 @@ Loop: xrpld.app xrpld.rpc xrpld.rpc > xrpld.app Loop: xrpld.app xrpld.shamap - xrpld.app > xrpld.shamap + xrpld.shamap ~= xrpld.app Loop: xrpld.core xrpld.perflog xrpld.perflog == xrpld.core diff --git a/.github/scripts/levelization/results/ordering.txt b/.github/scripts/levelization/results/ordering.txt index e17bd14bbc..251e9c1957 100644 --- a/.github/scripts/levelization/results/ordering.txt +++ b/.github/scripts/levelization/results/ordering.txt @@ -8,6 +8,10 @@ libxrpl.ledger > xrpl.ledger libxrpl.ledger > xrpl.protocol libxrpl.net > xrpl.basics libxrpl.net > xrpl.net +libxrpl.nodestore > xrpl.basics +libxrpl.nodestore > xrpl.json +libxrpl.nodestore > xrpl.nodestore +libxrpl.nodestore > xrpl.protocol libxrpl.protocol > xrpl.basics libxrpl.protocol > xrpl.json libxrpl.protocol > xrpl.protocol @@ -18,6 +22,9 @@ libxrpl.server > xrpl.basics libxrpl.server > xrpl.json libxrpl.server > xrpl.protocol libxrpl.server > xrpl.server +libxrpl.shamap > xrpl.basics +libxrpl.shamap > xrpl.protocol +libxrpl.shamap > xrpl.shamap test.app > test.jtx test.app > test.rpc test.app > test.toplevel @@ -25,11 +32,11 @@ test.app > test.unit_test test.app > xrpl.basics test.app > xrpld.app test.app > xrpld.core -test.app > xrpld.nodestore test.app > xrpld.overlay test.app > xrpld.rpc test.app > xrpl.json test.app > xrpl.ledger +test.app > xrpl.nodestore test.app > xrpl.protocol test.app > xrpl.resource test.basics > test.jtx @@ -86,8 +93,7 @@ test.nodestore > test.toplevel test.nodestore > test.unit_test test.nodestore > xrpl.basics test.nodestore > xrpld.core -test.nodestore > xrpld.nodestore -test.nodestore > xrpld.unity +test.nodestore > xrpl.nodestore test.overlay > test.jtx test.overlay > test.toplevel test.overlay > test.unit_test @@ -95,8 +101,8 @@ test.overlay > xrpl.basics test.overlay > xrpld.app test.overlay > xrpld.overlay test.overlay > xrpld.peerfinder -test.overlay > xrpld.shamap test.overlay > xrpl.protocol +test.overlay > xrpl.shamap test.peerfinder > test.beast test.peerfinder > test.unit_test test.peerfinder > xrpl.basics @@ -131,9 +137,9 @@ test.server > xrpl.json test.server > xrpl.server test.shamap > test.unit_test test.shamap > xrpl.basics -test.shamap > xrpld.nodestore -test.shamap > xrpld.shamap +test.shamap > xrpl.nodestore test.shamap > xrpl.protocol +test.shamap > xrpl.shamap test.toplevel > test.csf test.toplevel > xrpl.json test.unit_test > xrpl.basics @@ -144,6 +150,8 @@ xrpl.json > xrpl.basics xrpl.ledger > xrpl.basics xrpl.ledger > xrpl.protocol xrpl.net > xrpl.basics +xrpl.nodestore > xrpl.basics +xrpl.nodestore > xrpl.protocol xrpl.protocol > xrpl.basics xrpl.protocol > xrpl.json xrpl.resource > xrpl.basics @@ -152,17 +160,21 @@ xrpl.resource > xrpl.protocol xrpl.server > xrpl.basics xrpl.server > xrpl.json xrpl.server > xrpl.protocol +xrpl.shamap > xrpl.basics +xrpl.shamap > xrpl.nodestore +xrpl.shamap > xrpl.protocol xrpld.app > test.unit_test xrpld.app > xrpl.basics xrpld.app > xrpld.conditions xrpld.app > xrpld.consensus -xrpld.app > xrpld.nodestore xrpld.app > xrpld.perflog xrpld.app > xrpl.json xrpld.app > xrpl.ledger xrpld.app > xrpl.net +xrpld.app > xrpl.nodestore xrpld.app > xrpl.protocol xrpld.app > xrpl.resource +xrpld.app > xrpl.shamap xrpld.conditions > xrpl.basics xrpld.conditions > xrpl.protocol xrpld.consensus > xrpl.basics @@ -172,11 +184,6 @@ xrpld.core > xrpl.basics xrpld.core > xrpl.json xrpld.core > xrpl.net xrpld.core > xrpl.protocol -xrpld.nodestore > xrpl.basics -xrpld.nodestore > xrpld.core -xrpld.nodestore > xrpld.unity -xrpld.nodestore > xrpl.json -xrpld.nodestore > xrpl.protocol xrpld.overlay > xrpl.basics xrpld.overlay > xrpld.core xrpld.overlay > xrpld.peerfinder @@ -192,13 +199,11 @@ xrpld.perflog > xrpl.basics xrpld.perflog > xrpl.json xrpld.rpc > xrpl.basics xrpld.rpc > xrpld.core -xrpld.rpc > xrpld.nodestore xrpld.rpc > xrpl.json xrpld.rpc > xrpl.ledger xrpld.rpc > xrpl.net +xrpld.rpc > xrpl.nodestore xrpld.rpc > xrpl.protocol xrpld.rpc > xrpl.resource xrpld.rpc > xrpl.server -xrpld.shamap > xrpl.basics -xrpld.shamap > xrpld.nodestore -xrpld.shamap > xrpl.protocol +xrpld.shamap > xrpl.shamap diff --git a/cmake/RippledCore.cmake b/cmake/RippledCore.cmake index 481b6e3cea..05bbde8463 100644 --- a/cmake/RippledCore.cmake +++ b/cmake/RippledCore.cmake @@ -53,14 +53,15 @@ add_library(xrpl.imports.main INTERFACE) target_link_libraries(xrpl.imports.main INTERFACE - LibArchive::LibArchive - OpenSSL::Crypto - Ripple::boost - Ripple::opts - Ripple::syslibs absl::random_random date::date ed25519::ed25519 + LibArchive::LibArchive + OpenSSL::Crypto + Ripple::boost + Ripple::libs + Ripple::opts + Ripple::syslibs secp256k1::secp256k1 xrpl.libpb xxHash::xxhash @@ -111,6 +112,21 @@ target_link_libraries(xrpl.libxrpl.net PUBLIC add_module(xrpl server) target_link_libraries(xrpl.libxrpl.server PUBLIC xrpl.libxrpl.protocol) +add_module(xrpl nodestore) +target_link_libraries(xrpl.libxrpl.nodestore PUBLIC + xrpl.libxrpl.basics + xrpl.libxrpl.json + xrpl.libxrpl.protocol +) + +add_module(xrpl shamap) +target_link_libraries(xrpl.libxrpl.shamap PUBLIC + xrpl.libxrpl.basics + xrpl.libxrpl.crypto + xrpl.libxrpl.protocol + xrpl.libxrpl.nodestore +) + add_module(xrpl ledger) target_link_libraries(xrpl.libxrpl.ledger PUBLIC xrpl.libxrpl.basics @@ -136,6 +152,8 @@ target_link_modules(xrpl PUBLIC protocol resource server + nodestore + shamap net ledger ) diff --git a/cmake/RippledInstall.cmake b/cmake/RippledInstall.cmake index 50f09d5a4b..013e3f54b2 100644 --- a/cmake/RippledInstall.cmake +++ b/cmake/RippledInstall.cmake @@ -8,20 +8,23 @@ install ( TARGETS common opts - ripple_syslibs ripple_boost + ripple_libs + ripple_syslibs xrpl.imports.main xrpl.libpb + xrpl.libxrpl xrpl.libxrpl.basics xrpl.libxrpl.beast xrpl.libxrpl.crypto xrpl.libxrpl.json + xrpl.libxrpl.ledger + xrpl.libxrpl.net + xrpl.libxrpl.nodestore xrpl.libxrpl.protocol xrpl.libxrpl.resource - xrpl.libxrpl.ledger xrpl.libxrpl.server - xrpl.libxrpl.net - xrpl.libxrpl + xrpl.libxrpl.shamap antithesis-sdk-cpp EXPORT RippleExports LIBRARY DESTINATION lib diff --git a/src/xrpld/unity/rocksdb.h b/include/xrpl/basics/rocksdb.h similarity index 100% rename from src/xrpld/unity/rocksdb.h rename to include/xrpl/basics/rocksdb.h diff --git a/src/xrpld/nodestore/Backend.h b/include/xrpl/nodestore/Backend.h similarity index 99% rename from src/xrpld/nodestore/Backend.h rename to include/xrpl/nodestore/Backend.h index 1f9a62716c..23c0285a1d 100644 --- a/src/xrpld/nodestore/Backend.h +++ b/include/xrpl/nodestore/Backend.h @@ -20,7 +20,7 @@ #ifndef RIPPLE_NODESTORE_BACKEND_H_INCLUDED #define RIPPLE_NODESTORE_BACKEND_H_INCLUDED -#include +#include #include diff --git a/src/xrpld/nodestore/Database.h b/include/xrpl/nodestore/Database.h similarity index 98% rename from src/xrpld/nodestore/Database.h rename to include/xrpl/nodestore/Database.h index 403a5ea5ee..165d1f74c9 100644 --- a/src/xrpld/nodestore/Database.h +++ b/include/xrpl/nodestore/Database.h @@ -20,13 +20,12 @@ #ifndef RIPPLE_NODESTORE_DATABASE_H_INCLUDED #define RIPPLE_NODESTORE_DATABASE_H_INCLUDED -#include -#include -#include - #include #include #include +#include +#include +#include #include #include diff --git a/src/xrpld/nodestore/DatabaseRotating.h b/include/xrpl/nodestore/DatabaseRotating.h similarity index 98% rename from src/xrpld/nodestore/DatabaseRotating.h rename to include/xrpl/nodestore/DatabaseRotating.h index 3e8c6a7d5f..f63e99401d 100644 --- a/src/xrpld/nodestore/DatabaseRotating.h +++ b/include/xrpl/nodestore/DatabaseRotating.h @@ -20,7 +20,7 @@ #ifndef RIPPLE_NODESTORE_DATABASEROTATING_H_INCLUDED #define RIPPLE_NODESTORE_DATABASEROTATING_H_INCLUDED -#include +#include namespace ripple { namespace NodeStore { diff --git a/src/xrpld/nodestore/DummyScheduler.h b/include/xrpl/nodestore/DummyScheduler.h similarity index 97% rename from src/xrpld/nodestore/DummyScheduler.h rename to include/xrpl/nodestore/DummyScheduler.h index 4e6fab7827..0e9815f309 100644 --- a/src/xrpld/nodestore/DummyScheduler.h +++ b/include/xrpl/nodestore/DummyScheduler.h @@ -20,7 +20,7 @@ #ifndef RIPPLE_NODESTORE_DUMMYSCHEDULER_H_INCLUDED #define RIPPLE_NODESTORE_DUMMYSCHEDULER_H_INCLUDED -#include +#include namespace ripple { namespace NodeStore { diff --git a/src/xrpld/nodestore/Factory.h b/include/xrpl/nodestore/Factory.h similarity index 97% rename from src/xrpld/nodestore/Factory.h rename to include/xrpl/nodestore/Factory.h index b0768907da..a7ea4310fa 100644 --- a/src/xrpld/nodestore/Factory.h +++ b/include/xrpl/nodestore/Factory.h @@ -20,11 +20,10 @@ #ifndef RIPPLE_NODESTORE_FACTORY_H_INCLUDED #define RIPPLE_NODESTORE_FACTORY_H_INCLUDED -#include -#include - #include #include +#include +#include #include diff --git a/src/xrpld/nodestore/Manager.h b/include/xrpl/nodestore/Manager.h similarity index 97% rename from src/xrpld/nodestore/Manager.h rename to include/xrpl/nodestore/Manager.h index 89ed165b48..bf5213a508 100644 --- a/src/xrpld/nodestore/Manager.h +++ b/include/xrpl/nodestore/Manager.h @@ -20,8 +20,8 @@ #ifndef RIPPLE_NODESTORE_MANAGER_H_INCLUDED #define RIPPLE_NODESTORE_MANAGER_H_INCLUDED -#include -#include +#include +#include namespace ripple { diff --git a/src/xrpld/nodestore/NodeObject.h b/include/xrpl/nodestore/NodeObject.h similarity index 100% rename from src/xrpld/nodestore/NodeObject.h rename to include/xrpl/nodestore/NodeObject.h diff --git a/src/xrpld/nodestore/README.md b/include/xrpl/nodestore/README.md similarity index 100% rename from src/xrpld/nodestore/README.md rename to include/xrpl/nodestore/README.md diff --git a/src/xrpld/nodestore/Scheduler.h b/include/xrpl/nodestore/Scheduler.h similarity index 98% rename from src/xrpld/nodestore/Scheduler.h rename to include/xrpl/nodestore/Scheduler.h index 663db7eb74..fb4aa6bdfc 100644 --- a/src/xrpld/nodestore/Scheduler.h +++ b/include/xrpl/nodestore/Scheduler.h @@ -20,7 +20,7 @@ #ifndef RIPPLE_NODESTORE_SCHEDULER_H_INCLUDED #define RIPPLE_NODESTORE_SCHEDULER_H_INCLUDED -#include +#include #include diff --git a/src/xrpld/nodestore/Task.h b/include/xrpl/nodestore/Task.h similarity index 100% rename from src/xrpld/nodestore/Task.h rename to include/xrpl/nodestore/Task.h diff --git a/src/xrpld/nodestore/Types.h b/include/xrpl/nodestore/Types.h similarity index 97% rename from src/xrpld/nodestore/Types.h rename to include/xrpl/nodestore/Types.h index 5e22eb4ddf..ea17a8f1ae 100644 --- a/src/xrpld/nodestore/Types.h +++ b/include/xrpl/nodestore/Types.h @@ -20,7 +20,7 @@ #ifndef RIPPLE_NODESTORE_TYPES_H_INCLUDED #define RIPPLE_NODESTORE_TYPES_H_INCLUDED -#include +#include #include diff --git a/src/xrpld/nodestore/detail/BatchWriter.h b/include/xrpl/nodestore/detail/BatchWriter.h similarity index 96% rename from src/xrpld/nodestore/detail/BatchWriter.h rename to include/xrpl/nodestore/detail/BatchWriter.h index 15ba508f64..941ecdfe18 100644 --- a/src/xrpld/nodestore/detail/BatchWriter.h +++ b/include/xrpl/nodestore/detail/BatchWriter.h @@ -20,9 +20,9 @@ #ifndef RIPPLE_NODESTORE_BATCHWRITER_H_INCLUDED #define RIPPLE_NODESTORE_BATCHWRITER_H_INCLUDED -#include -#include -#include +#include +#include +#include #include #include diff --git a/src/xrpld/nodestore/detail/DatabaseNodeImp.h b/include/xrpl/nodestore/detail/DatabaseNodeImp.h similarity index 99% rename from src/xrpld/nodestore/detail/DatabaseNodeImp.h rename to include/xrpl/nodestore/detail/DatabaseNodeImp.h index 160bf92d5e..ff43bd253e 100644 --- a/src/xrpld/nodestore/detail/DatabaseNodeImp.h +++ b/include/xrpl/nodestore/detail/DatabaseNodeImp.h @@ -20,10 +20,9 @@ #ifndef RIPPLE_NODESTORE_DATABASENODEIMP_H_INCLUDED #define RIPPLE_NODESTORE_DATABASENODEIMP_H_INCLUDED -#include - #include #include +#include namespace ripple { namespace NodeStore { diff --git a/src/xrpld/nodestore/detail/DatabaseRotatingImp.h b/include/xrpl/nodestore/detail/DatabaseRotatingImp.h similarity index 98% rename from src/xrpld/nodestore/detail/DatabaseRotatingImp.h rename to include/xrpl/nodestore/detail/DatabaseRotatingImp.h index d9f114f503..0f81ac873c 100644 --- a/src/xrpld/nodestore/detail/DatabaseRotatingImp.h +++ b/include/xrpl/nodestore/detail/DatabaseRotatingImp.h @@ -20,7 +20,7 @@ #ifndef RIPPLE_NODESTORE_DATABASEROTATINGIMP_H_INCLUDED #define RIPPLE_NODESTORE_DATABASEROTATINGIMP_H_INCLUDED -#include +#include #include diff --git a/src/xrpld/nodestore/detail/DecodedBlob.h b/include/xrpl/nodestore/detail/DecodedBlob.h similarity index 98% rename from src/xrpld/nodestore/detail/DecodedBlob.h rename to include/xrpl/nodestore/detail/DecodedBlob.h index d52f20c4ee..635f2a196b 100644 --- a/src/xrpld/nodestore/detail/DecodedBlob.h +++ b/include/xrpl/nodestore/detail/DecodedBlob.h @@ -20,7 +20,7 @@ #ifndef RIPPLE_NODESTORE_DECODEDBLOB_H_INCLUDED #define RIPPLE_NODESTORE_DECODEDBLOB_H_INCLUDED -#include +#include namespace ripple { namespace NodeStore { diff --git a/src/xrpld/nodestore/detail/EncodedBlob.h b/include/xrpl/nodestore/detail/EncodedBlob.h similarity index 99% rename from src/xrpld/nodestore/detail/EncodedBlob.h rename to include/xrpl/nodestore/detail/EncodedBlob.h index a238f2b856..d1f465d0cd 100644 --- a/src/xrpld/nodestore/detail/EncodedBlob.h +++ b/include/xrpl/nodestore/detail/EncodedBlob.h @@ -20,9 +20,8 @@ #ifndef RIPPLE_NODESTORE_ENCODEDBLOB_H_INCLUDED #define RIPPLE_NODESTORE_ENCODEDBLOB_H_INCLUDED -#include - #include +#include #include diff --git a/src/xrpld/nodestore/detail/ManagerImp.h b/include/xrpl/nodestore/detail/ManagerImp.h similarity index 96% rename from src/xrpld/nodestore/detail/ManagerImp.h rename to include/xrpl/nodestore/detail/ManagerImp.h index 92411cc760..e8ebbc3e11 100644 --- a/src/xrpld/nodestore/detail/ManagerImp.h +++ b/include/xrpl/nodestore/detail/ManagerImp.h @@ -20,7 +20,7 @@ #ifndef RIPPLE_NODESTORE_MANAGERIMP_H_INCLUDED #define RIPPLE_NODESTORE_MANAGERIMP_H_INCLUDED -#include +#include namespace ripple { @@ -39,7 +39,7 @@ public: static void missing_backend(); - ManagerImp() = default; + ManagerImp(); ~ManagerImp() = default; diff --git a/src/xrpld/nodestore/detail/codec.h b/include/xrpl/nodestore/detail/codec.h similarity index 99% rename from src/xrpld/nodestore/detail/codec.h rename to include/xrpl/nodestore/detail/codec.h index 7bc3ab2caa..9c115277f8 100644 --- a/src/xrpld/nodestore/detail/codec.h +++ b/include/xrpl/nodestore/detail/codec.h @@ -23,11 +23,10 @@ // Disable lz4 deprecation warning due to incompatibility with clang attributes #define LZ4_DISABLE_DEPRECATE_WARNINGS -#include -#include - #include #include +#include +#include #include #include diff --git a/src/xrpld/nodestore/detail/varint.h b/include/xrpl/nodestore/detail/varint.h similarity index 100% rename from src/xrpld/nodestore/detail/varint.h rename to include/xrpl/nodestore/detail/varint.h diff --git a/src/xrpld/shamap/Family.h b/include/xrpl/shamap/Family.h similarity index 95% rename from src/xrpld/shamap/Family.h rename to include/xrpl/shamap/Family.h index eb04df05e4..5975321873 100644 --- a/src/xrpld/shamap/Family.h +++ b/include/xrpl/shamap/Family.h @@ -20,11 +20,10 @@ #ifndef RIPPLE_SHAMAP_FAMILY_H_INCLUDED #define RIPPLE_SHAMAP_FAMILY_H_INCLUDED -#include -#include -#include - #include +#include +#include +#include #include diff --git a/src/xrpld/shamap/FullBelowCache.h b/include/xrpl/shamap/FullBelowCache.h similarity index 100% rename from src/xrpld/shamap/FullBelowCache.h rename to include/xrpl/shamap/FullBelowCache.h diff --git a/src/xrpld/shamap/README.md b/include/xrpl/shamap/README.md similarity index 100% rename from src/xrpld/shamap/README.md rename to include/xrpl/shamap/README.md diff --git a/src/xrpld/shamap/SHAMap.h b/include/xrpl/shamap/SHAMap.h similarity index 98% rename from src/xrpld/shamap/SHAMap.h rename to include/xrpl/shamap/SHAMap.h index 738cf96ecc..2bc003cd82 100644 --- a/src/xrpld/shamap/SHAMap.h +++ b/include/xrpl/shamap/SHAMap.h @@ -20,21 +20,19 @@ #ifndef RIPPLE_SHAMAP_SHAMAP_H_INCLUDED #define RIPPLE_SHAMAP_SHAMAP_H_INCLUDED -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - #include #include #include #include +#include +#include +#include +#include +#include +#include +#include +#include +#include #include #include diff --git a/src/xrpld/shamap/SHAMapAccountStateLeafNode.h b/include/xrpl/shamap/SHAMapAccountStateLeafNode.h similarity index 97% rename from src/xrpld/shamap/SHAMapAccountStateLeafNode.h rename to include/xrpl/shamap/SHAMapAccountStateLeafNode.h index f6b5e0827c..a600ea222b 100644 --- a/src/xrpld/shamap/SHAMapAccountStateLeafNode.h +++ b/include/xrpl/shamap/SHAMapAccountStateLeafNode.h @@ -20,12 +20,11 @@ #ifndef RIPPLE_SHAMAP_SHAMAPACCOUNTSTATELEAFNODE_H_INCLUDED #define RIPPLE_SHAMAP_SHAMAPACCOUNTSTATELEAFNODE_H_INCLUDED -#include -#include - #include #include #include +#include +#include namespace ripple { diff --git a/src/xrpld/shamap/SHAMapAddNode.h b/include/xrpl/shamap/SHAMapAddNode.h similarity index 100% rename from src/xrpld/shamap/SHAMapAddNode.h rename to include/xrpl/shamap/SHAMapAddNode.h diff --git a/src/xrpld/shamap/SHAMapInnerNode.h b/include/xrpl/shamap/SHAMapInnerNode.h similarity index 98% rename from src/xrpld/shamap/SHAMapInnerNode.h rename to include/xrpl/shamap/SHAMapInnerNode.h index 5c064fd9da..58d7392cf4 100644 --- a/src/xrpld/shamap/SHAMapInnerNode.h +++ b/include/xrpl/shamap/SHAMapInnerNode.h @@ -20,10 +20,9 @@ #ifndef RIPPLE_SHAMAP_SHAMAPINNERNODE_H_INCLUDED #define RIPPLE_SHAMAP_SHAMAPINNERNODE_H_INCLUDED -#include -#include - #include +#include +#include #include #include diff --git a/src/xrpld/shamap/SHAMapItem.h b/include/xrpl/shamap/SHAMapItem.h similarity index 100% rename from src/xrpld/shamap/SHAMapItem.h rename to include/xrpl/shamap/SHAMapItem.h diff --git a/src/xrpld/shamap/SHAMapLeafNode.h b/include/xrpl/shamap/SHAMapLeafNode.h similarity index 96% rename from src/xrpld/shamap/SHAMapLeafNode.h rename to include/xrpl/shamap/SHAMapLeafNode.h index 383da38fd4..5c853d6127 100644 --- a/src/xrpld/shamap/SHAMapLeafNode.h +++ b/include/xrpl/shamap/SHAMapLeafNode.h @@ -20,8 +20,8 @@ #ifndef RIPPLE_SHAMAP_SHAMAPLEAFNODE_H_INCLUDED #define RIPPLE_SHAMAP_SHAMAPLEAFNODE_H_INCLUDED -#include -#include +#include +#include #include diff --git a/src/xrpld/shamap/SHAMapMissingNode.h b/include/xrpl/shamap/SHAMapMissingNode.h similarity index 98% rename from src/xrpld/shamap/SHAMapMissingNode.h rename to include/xrpl/shamap/SHAMapMissingNode.h index 73ffe0090a..c41e6f2602 100644 --- a/src/xrpld/shamap/SHAMapMissingNode.h +++ b/include/xrpl/shamap/SHAMapMissingNode.h @@ -20,9 +20,8 @@ #ifndef RIPPLE_SHAMAP_SHAMAPMISSINGNODE_H_INCLUDED #define RIPPLE_SHAMAP_SHAMAPMISSINGNODE_H_INCLUDED -#include - #include +#include #include #include diff --git a/src/xrpld/shamap/SHAMapNodeID.h b/include/xrpl/shamap/SHAMapNodeID.h similarity index 100% rename from src/xrpld/shamap/SHAMapNodeID.h rename to include/xrpl/shamap/SHAMapNodeID.h diff --git a/src/xrpld/shamap/SHAMapSyncFilter.h b/include/xrpl/shamap/SHAMapSyncFilter.h similarity index 97% rename from src/xrpld/shamap/SHAMapSyncFilter.h rename to include/xrpl/shamap/SHAMapSyncFilter.h index a2ac2f99e8..43d5941466 100644 --- a/src/xrpld/shamap/SHAMapSyncFilter.h +++ b/include/xrpl/shamap/SHAMapSyncFilter.h @@ -20,7 +20,7 @@ #ifndef RIPPLE_SHAMAP_SHAMAPSYNCFILTER_H_INCLUDED #define RIPPLE_SHAMAP_SHAMAPSYNCFILTER_H_INCLUDED -#include +#include #include diff --git a/src/xrpld/shamap/SHAMapTreeNode.h b/include/xrpl/shamap/SHAMapTreeNode.h similarity index 98% rename from src/xrpld/shamap/SHAMapTreeNode.h rename to include/xrpl/shamap/SHAMapTreeNode.h index 2c4a349019..c1351112c2 100644 --- a/src/xrpld/shamap/SHAMapTreeNode.h +++ b/include/xrpl/shamap/SHAMapTreeNode.h @@ -20,13 +20,12 @@ #ifndef RIPPLE_SHAMAP_SHAMAPTREENODE_H_INCLUDED #define RIPPLE_SHAMAP_SHAMAPTREENODE_H_INCLUDED -#include -#include - #include #include #include #include +#include +#include #include #include diff --git a/src/xrpld/shamap/SHAMapTxLeafNode.h b/include/xrpl/shamap/SHAMapTxLeafNode.h similarity index 97% rename from src/xrpld/shamap/SHAMapTxLeafNode.h rename to include/xrpl/shamap/SHAMapTxLeafNode.h index 50f426a581..f5b406d1de 100644 --- a/src/xrpld/shamap/SHAMapTxLeafNode.h +++ b/include/xrpl/shamap/SHAMapTxLeafNode.h @@ -20,12 +20,11 @@ #ifndef RIPPLE_SHAMAP_SHAMAPTXLEAFNODE_H_INCLUDED #define RIPPLE_SHAMAP_SHAMAPTXLEAFNODE_H_INCLUDED -#include -#include - #include #include #include +#include +#include namespace ripple { diff --git a/src/xrpld/shamap/SHAMapTxPlusMetaLeafNode.h b/include/xrpl/shamap/SHAMapTxPlusMetaLeafNode.h similarity index 97% rename from src/xrpld/shamap/SHAMapTxPlusMetaLeafNode.h rename to include/xrpl/shamap/SHAMapTxPlusMetaLeafNode.h index cc34d8f4ae..f14726b0f2 100644 --- a/src/xrpld/shamap/SHAMapTxPlusMetaLeafNode.h +++ b/include/xrpl/shamap/SHAMapTxPlusMetaLeafNode.h @@ -20,12 +20,11 @@ #ifndef RIPPLE_SHAMAP_SHAMAPLEAFTXPLUSMETANODE_H_INCLUDED #define RIPPLE_SHAMAP_SHAMAPLEAFTXPLUSMETANODE_H_INCLUDED -#include -#include - #include #include #include +#include +#include namespace ripple { diff --git a/src/xrpld/shamap/TreeNodeCache.h b/include/xrpl/shamap/TreeNodeCache.h similarity index 97% rename from src/xrpld/shamap/TreeNodeCache.h rename to include/xrpl/shamap/TreeNodeCache.h index b41ae5e99e..e1c612586e 100644 --- a/src/xrpld/shamap/TreeNodeCache.h +++ b/include/xrpl/shamap/TreeNodeCache.h @@ -20,10 +20,9 @@ #ifndef RIPPLE_SHAMAP_TREENODECACHE_H_INCLUDED #define RIPPLE_SHAMAP_TREENODECACHE_H_INCLUDED -#include - #include #include +#include namespace ripple { diff --git a/src/xrpld/shamap/detail/TaggedPointer.h b/include/xrpl/shamap/detail/TaggedPointer.h similarity index 99% rename from src/xrpld/shamap/detail/TaggedPointer.h rename to include/xrpl/shamap/detail/TaggedPointer.h index 11ab1f57fc..54aef85fa6 100644 --- a/src/xrpld/shamap/detail/TaggedPointer.h +++ b/include/xrpl/shamap/detail/TaggedPointer.h @@ -20,9 +20,8 @@ #ifndef RIPPLE_SHAMAP_TAGGEDPOINTER_H_INCLUDED #define RIPPLE_SHAMAP_TAGGEDPOINTER_H_INCLUDED -#include - #include +#include #include #include diff --git a/src/xrpld/shamap/detail/TaggedPointer.ipp b/include/xrpl/shamap/detail/TaggedPointer.ipp similarity index 99% rename from src/xrpld/shamap/detail/TaggedPointer.ipp rename to include/xrpl/shamap/detail/TaggedPointer.ipp index f5d40f24fa..3efcfb6ffe 100644 --- a/src/xrpld/shamap/detail/TaggedPointer.ipp +++ b/include/xrpl/shamap/detail/TaggedPointer.ipp @@ -17,10 +17,9 @@ */ //============================================================================== -#include -#include - #include +#include +#include #include diff --git a/src/xrpld/nodestore/detail/BatchWriter.cpp b/src/libxrpl/nodestore/BatchWriter.cpp similarity index 98% rename from src/xrpld/nodestore/detail/BatchWriter.cpp rename to src/libxrpl/nodestore/BatchWriter.cpp index 9fd90f82e7..14947b5d21 100644 --- a/src/xrpld/nodestore/detail/BatchWriter.cpp +++ b/src/libxrpl/nodestore/BatchWriter.cpp @@ -17,7 +17,7 @@ */ //============================================================================== -#include +#include namespace ripple { namespace NodeStore { diff --git a/src/xrpld/nodestore/detail/Database.cpp b/src/libxrpl/nodestore/Database.cpp similarity index 99% rename from src/xrpld/nodestore/detail/Database.cpp rename to src/libxrpl/nodestore/Database.cpp index 4edafcddca..14f3299d1f 100644 --- a/src/xrpld/nodestore/detail/Database.cpp +++ b/src/libxrpl/nodestore/Database.cpp @@ -17,11 +17,10 @@ */ //============================================================================== -#include - #include #include #include +#include #include #include diff --git a/src/xrpld/nodestore/detail/DatabaseNodeImp.cpp b/src/libxrpl/nodestore/DatabaseNodeImp.cpp similarity index 99% rename from src/xrpld/nodestore/detail/DatabaseNodeImp.cpp rename to src/libxrpl/nodestore/DatabaseNodeImp.cpp index 2502d52815..99b27f25b1 100644 --- a/src/xrpld/nodestore/detail/DatabaseNodeImp.cpp +++ b/src/libxrpl/nodestore/DatabaseNodeImp.cpp @@ -17,7 +17,7 @@ */ //============================================================================== -#include +#include namespace ripple { namespace NodeStore { diff --git a/src/xrpld/nodestore/detail/DatabaseRotatingImp.cpp b/src/libxrpl/nodestore/DatabaseRotatingImp.cpp similarity index 99% rename from src/xrpld/nodestore/detail/DatabaseRotatingImp.cpp rename to src/libxrpl/nodestore/DatabaseRotatingImp.cpp index a4828ab2c2..60a25ff870 100644 --- a/src/xrpld/nodestore/detail/DatabaseRotatingImp.cpp +++ b/src/libxrpl/nodestore/DatabaseRotatingImp.cpp @@ -17,7 +17,7 @@ */ //============================================================================== -#include +#include namespace ripple { namespace NodeStore { diff --git a/src/xrpld/nodestore/detail/DecodedBlob.cpp b/src/libxrpl/nodestore/DecodedBlob.cpp similarity index 98% rename from src/xrpld/nodestore/detail/DecodedBlob.cpp rename to src/libxrpl/nodestore/DecodedBlob.cpp index adec27a68f..6d3019dc20 100644 --- a/src/xrpld/nodestore/detail/DecodedBlob.cpp +++ b/src/libxrpl/nodestore/DecodedBlob.cpp @@ -17,10 +17,9 @@ */ //============================================================================== -#include - #include #include +#include #include diff --git a/src/xrpld/nodestore/detail/DummyScheduler.cpp b/src/libxrpl/nodestore/DummyScheduler.cpp similarity index 96% rename from src/xrpld/nodestore/detail/DummyScheduler.cpp rename to src/libxrpl/nodestore/DummyScheduler.cpp index 9df1374189..a70c2f35a4 100644 --- a/src/xrpld/nodestore/detail/DummyScheduler.cpp +++ b/src/libxrpl/nodestore/DummyScheduler.cpp @@ -17,7 +17,7 @@ */ //============================================================================== -#include +#include namespace ripple { namespace NodeStore { diff --git a/src/xrpld/nodestore/detail/ManagerImp.cpp b/src/libxrpl/nodestore/ManagerImp.cpp similarity index 81% rename from src/xrpld/nodestore/detail/ManagerImp.cpp rename to src/libxrpl/nodestore/ManagerImp.cpp index 24371dd58c..94abfd9aed 100644 --- a/src/xrpld/nodestore/detail/ManagerImp.cpp +++ b/src/libxrpl/nodestore/ManagerImp.cpp @@ -17,8 +17,8 @@ */ //============================================================================== -#include -#include +#include +#include #include @@ -41,6 +41,27 @@ ManagerImp::missing_backend() "please see the rippled-example.cfg file!"); } +// We shouldn't rely on global variables for lifetime management because their +// lifetime is not well-defined. ManagerImp may get destroyed before the Factory +// classes, and then, calling Manager::instance().erase() in the destructors of +// the Factory classes is an undefined behaviour. +void +registerNuDBFactory(Manager& manager); +void +registerRocksDBFactory(Manager& manager); +void +registerNullFactory(Manager& manager); +void +registerMemoryFactory(Manager& manager); + +ManagerImp::ManagerImp() +{ + registerNuDBFactory(*this); + registerRocksDBFactory(*this); + registerNullFactory(*this); + registerMemoryFactory(*this); +} + std::unique_ptr ManagerImp::make_Backend( Section const& parameters, diff --git a/src/xrpld/nodestore/detail/NodeObject.cpp b/src/libxrpl/nodestore/NodeObject.cpp similarity index 97% rename from src/xrpld/nodestore/detail/NodeObject.cpp rename to src/libxrpl/nodestore/NodeObject.cpp index 0e81c047c6..6cf69ac87f 100644 --- a/src/xrpld/nodestore/detail/NodeObject.cpp +++ b/src/libxrpl/nodestore/NodeObject.cpp @@ -17,7 +17,7 @@ */ //============================================================================== -#include +#include #include diff --git a/src/xrpld/nodestore/backend/MemoryFactory.cpp b/src/libxrpl/nodestore/backend/MemoryFactory.cpp similarity index 92% rename from src/xrpld/nodestore/backend/MemoryFactory.cpp rename to src/libxrpl/nodestore/backend/MemoryFactory.cpp index dc3106cc91..c45debe1d0 100644 --- a/src/xrpld/nodestore/backend/MemoryFactory.cpp +++ b/src/libxrpl/nodestore/backend/MemoryFactory.cpp @@ -17,10 +17,9 @@ */ //============================================================================== -#include -#include - #include +#include +#include #include #include @@ -46,10 +45,10 @@ class MemoryFactory : public Factory private: std::mutex mutex_; std::map map_; + Manager& manager_; public: - MemoryFactory(); - ~MemoryFactory() override; + explicit MemoryFactory(Manager& manager); std::string getName() const override; @@ -75,7 +74,14 @@ public: } }; -static MemoryFactory memoryFactory; +MemoryFactory* memoryFactory = nullptr; + +void +registerMemoryFactory(Manager& manager) +{ + static MemoryFactory instance{manager}; + memoryFactory = &instance; +} //------------------------------------------------------------------------------ @@ -112,9 +118,9 @@ public: } void - open(bool createIfMissing) override + open(bool) override { - db_ = &memoryFactory.open(name_); + db_ = &memoryFactory->open(name_); } bool @@ -219,14 +225,9 @@ public: //------------------------------------------------------------------------------ -MemoryFactory::MemoryFactory() +MemoryFactory::MemoryFactory(Manager& manager) : manager_(manager) { - Manager::instance().insert(*this); -} - -MemoryFactory::~MemoryFactory() -{ - Manager::instance().erase(*this); + manager_.insert(*this); } std::string diff --git a/src/xrpld/nodestore/backend/NuDBFactory.cpp b/src/libxrpl/nodestore/backend/NuDBFactory.cpp similarity index 96% rename from src/xrpld/nodestore/backend/NuDBFactory.cpp rename to src/libxrpl/nodestore/backend/NuDBFactory.cpp index 9f8217f6bf..0202dfc4e9 100644 --- a/src/xrpld/nodestore/backend/NuDBFactory.cpp +++ b/src/libxrpl/nodestore/backend/NuDBFactory.cpp @@ -17,15 +17,14 @@ */ //============================================================================== -#include -#include -#include -#include -#include - #include #include #include +#include +#include +#include +#include +#include #include @@ -427,15 +426,13 @@ private: class NuDBFactory : public Factory { -public: - NuDBFactory() - { - Manager::instance().insert(*this); - } +private: + Manager& manager_; - ~NuDBFactory() override +public: + explicit NuDBFactory(Manager& manager) : manager_(manager) { - Manager::instance().erase(*this); + manager_.insert(*this); } std::string @@ -470,7 +467,11 @@ public: } }; -static NuDBFactory nuDBFactory; +void +registerNuDBFactory(Manager& manager) +{ + static NuDBFactory instance{manager}; +} } // namespace NodeStore } // namespace ripple diff --git a/src/xrpld/nodestore/backend/NullFactory.cpp b/src/libxrpl/nodestore/backend/NullFactory.cpp similarity index 90% rename from src/xrpld/nodestore/backend/NullFactory.cpp rename to src/libxrpl/nodestore/backend/NullFactory.cpp index 5aae8ca7cf..6408e4aad5 100644 --- a/src/xrpld/nodestore/backend/NullFactory.cpp +++ b/src/libxrpl/nodestore/backend/NullFactory.cpp @@ -17,8 +17,8 @@ */ //============================================================================== -#include -#include +#include +#include #include @@ -111,15 +111,13 @@ private: class NullFactory : public Factory { -public: - NullFactory() - { - Manager::instance().insert(*this); - } +private: + Manager& manager_; - ~NullFactory() override +public: + explicit NullFactory(Manager& manager) : manager_(manager) { - Manager::instance().erase(*this); + manager_.insert(*this); } std::string @@ -140,7 +138,11 @@ public: } }; -static NullFactory nullFactory; +void +registerNullFactory(Manager& manager) +{ + static NullFactory instance{manager}; +} } // namespace NodeStore } // namespace ripple diff --git a/src/xrpld/nodestore/backend/RocksDBFactory.cpp b/src/libxrpl/nodestore/backend/RocksDBFactory.cpp similarity index 96% rename from src/xrpld/nodestore/backend/RocksDBFactory.cpp rename to src/libxrpl/nodestore/backend/RocksDBFactory.cpp index 57c136a10a..68fef494b6 100644 --- a/src/xrpld/nodestore/backend/RocksDBFactory.cpp +++ b/src/libxrpl/nodestore/backend/RocksDBFactory.cpp @@ -17,20 +17,18 @@ */ //============================================================================== -#include +#include #if RIPPLE_ROCKSDB_AVAILABLE -#include // VFALCO Bad dependency -#include -#include -#include -#include -#include - #include #include #include #include +#include +#include +#include +#include +#include #include #include @@ -461,17 +459,15 @@ public: class RocksDBFactory : public Factory { +private: + Manager& manager_; + public: RocksDBEnv m_env; - RocksDBFactory() + RocksDBFactory(Manager& manager) : manager_(manager) { - Manager::instance().insert(*this); - } - - ~RocksDBFactory() override - { - Manager::instance().erase(*this); + manager_.insert(*this); } std::string @@ -493,7 +489,11 @@ public: } }; -static RocksDBFactory rocksDBFactory; +void +registerRocksDBFactory(Manager& manager) +{ + static RocksDBFactory instance{manager}; +} } // namespace NodeStore } // namespace ripple diff --git a/src/xrpld/shamap/detail/SHAMap.cpp b/src/libxrpl/shamap/SHAMap.cpp similarity index 99% rename from src/xrpld/shamap/detail/SHAMap.cpp rename to src/libxrpl/shamap/SHAMap.cpp index 026149be56..4d2100ceab 100644 --- a/src/xrpld/shamap/detail/SHAMap.cpp +++ b/src/libxrpl/shamap/SHAMap.cpp @@ -17,15 +17,14 @@ */ //============================================================================== -#include -#include -#include -#include -#include -#include - #include #include +#include +#include +#include +#include +#include +#include namespace ripple { diff --git a/src/xrpld/shamap/detail/SHAMapDelta.cpp b/src/libxrpl/shamap/SHAMapDelta.cpp similarity index 99% rename from src/xrpld/shamap/detail/SHAMapDelta.cpp rename to src/libxrpl/shamap/SHAMapDelta.cpp index ebdaffad14..2fc3152fc3 100644 --- a/src/xrpld/shamap/detail/SHAMapDelta.cpp +++ b/src/libxrpl/shamap/SHAMapDelta.cpp @@ -17,10 +17,9 @@ */ //============================================================================== -#include - #include #include +#include #include #include diff --git a/src/xrpld/shamap/detail/SHAMapInnerNode.cpp b/src/libxrpl/shamap/SHAMapInnerNode.cpp similarity index 99% rename from src/xrpld/shamap/detail/SHAMapInnerNode.cpp rename to src/libxrpl/shamap/SHAMapInnerNode.cpp index 6e9d447cf6..1b55887098 100644 --- a/src/xrpld/shamap/detail/SHAMapInnerNode.cpp +++ b/src/libxrpl/shamap/SHAMapInnerNode.cpp @@ -17,16 +17,15 @@ */ //============================================================================== -#include -#include -#include - #include #include #include #include #include #include +#include +#include +#include namespace ripple { diff --git a/src/xrpld/shamap/detail/SHAMapLeafNode.cpp b/src/libxrpl/shamap/SHAMapLeafNode.cpp similarity index 98% rename from src/xrpld/shamap/detail/SHAMapLeafNode.cpp rename to src/libxrpl/shamap/SHAMapLeafNode.cpp index 10d61ff138..a6ba55a2c7 100644 --- a/src/xrpld/shamap/detail/SHAMapLeafNode.cpp +++ b/src/libxrpl/shamap/SHAMapLeafNode.cpp @@ -17,7 +17,7 @@ */ //============================================================================== -#include +#include namespace ripple { diff --git a/src/xrpld/shamap/detail/SHAMapNodeID.cpp b/src/libxrpl/shamap/SHAMapNodeID.cpp similarity index 98% rename from src/xrpld/shamap/detail/SHAMapNodeID.cpp rename to src/libxrpl/shamap/SHAMapNodeID.cpp index efe4f95936..189a127620 100644 --- a/src/xrpld/shamap/detail/SHAMapNodeID.cpp +++ b/src/libxrpl/shamap/SHAMapNodeID.cpp @@ -17,12 +17,11 @@ */ //============================================================================== -#include -#include - #include #include #include +#include +#include namespace ripple { diff --git a/src/xrpld/shamap/detail/SHAMapSync.cpp b/src/libxrpl/shamap/SHAMapSync.cpp similarity index 99% rename from src/xrpld/shamap/detail/SHAMapSync.cpp rename to src/libxrpl/shamap/SHAMapSync.cpp index 8a7d75ccf8..e6049f1083 100644 --- a/src/xrpld/shamap/detail/SHAMapSync.cpp +++ b/src/libxrpl/shamap/SHAMapSync.cpp @@ -17,11 +17,10 @@ */ //============================================================================== -#include -#include -#include - #include +#include +#include +#include namespace ripple { diff --git a/src/xrpld/shamap/detail/SHAMapTreeNode.cpp b/src/libxrpl/shamap/SHAMapTreeNode.cpp similarity index 95% rename from src/xrpld/shamap/detail/SHAMapTreeNode.cpp rename to src/libxrpl/shamap/SHAMapTreeNode.cpp index d1e74fd6a7..049a4da80f 100644 --- a/src/xrpld/shamap/detail/SHAMapTreeNode.cpp +++ b/src/libxrpl/shamap/SHAMapTreeNode.cpp @@ -17,18 +17,17 @@ */ //============================================================================== -#include -#include -#include -#include -#include - #include #include #include #include #include #include +#include +#include +#include +#include +#include namespace ripple { diff --git a/src/test/app/SHAMapStore_test.cpp b/src/test/app/SHAMapStore_test.cpp index 1e0ec4bcf0..29fbc8022c 100644 --- a/src/test/app/SHAMapStore_test.cpp +++ b/src/test/app/SHAMapStore_test.cpp @@ -25,8 +25,8 @@ #include #include #include -#include +#include #include namespace ripple { diff --git a/src/test/nodestore/Backend_test.cpp b/src/test/nodestore/Backend_test.cpp index f161f7a0c0..af5559b00f 100644 --- a/src/test/nodestore/Backend_test.cpp +++ b/src/test/nodestore/Backend_test.cpp @@ -20,12 +20,11 @@ #include #include -#include -#include -#include - #include +#include #include +#include +#include #include diff --git a/src/test/nodestore/Basics_test.cpp b/src/test/nodestore/Basics_test.cpp index d781bb0c78..0057d2658a 100644 --- a/src/test/nodestore/Basics_test.cpp +++ b/src/test/nodestore/Basics_test.cpp @@ -19,8 +19,8 @@ #include -#include -#include +#include +#include namespace ripple { namespace NodeStore { diff --git a/src/test/nodestore/Database_test.cpp b/src/test/nodestore/Database_test.cpp index 5ecb5b94e8..a47dfeab1a 100644 --- a/src/test/nodestore/Database_test.cpp +++ b/src/test/nodestore/Database_test.cpp @@ -24,10 +24,10 @@ #include #include -#include -#include #include +#include +#include namespace ripple { diff --git a/src/test/nodestore/NuDBFactory_test.cpp b/src/test/nodestore/NuDBFactory_test.cpp index db7d0d2999..387c83a446 100644 --- a/src/test/nodestore/NuDBFactory_test.cpp +++ b/src/test/nodestore/NuDBFactory_test.cpp @@ -20,12 +20,11 @@ #include #include -#include -#include - #include #include #include +#include +#include #include #include diff --git a/src/test/nodestore/TestBase.h b/src/test/nodestore/TestBase.h index 634f2bf005..8c44e9d918 100644 --- a/src/test/nodestore/TestBase.h +++ b/src/test/nodestore/TestBase.h @@ -20,15 +20,14 @@ #ifndef RIPPLE_NODESTORE_BASE_H_INCLUDED #define RIPPLE_NODESTORE_BASE_H_INCLUDED -#include -#include -#include - #include #include #include #include #include +#include +#include +#include #include diff --git a/src/test/nodestore/Timing_test.cpp b/src/test/nodestore/Timing_test.cpp index 1ba5903cbe..b2acb8eac3 100644 --- a/src/test/nodestore/Timing_test.cpp +++ b/src/test/nodestore/Timing_test.cpp @@ -20,9 +20,6 @@ #include #include -#include -#include - #include #include #include @@ -30,6 +27,8 @@ #include #include #include +#include +#include #include diff --git a/src/test/nodestore/import_test.cpp b/src/test/nodestore/import_test.cpp index ea5f23548a..3aeacb35f0 100644 --- a/src/test/nodestore/import_test.cpp +++ b/src/test/nodestore/import_test.cpp @@ -17,14 +17,13 @@ */ //============================================================================== -#include -#include - #include +#include #include #include #include #include +#include #include #include diff --git a/src/test/nodestore/varint_test.cpp b/src/test/nodestore/varint_test.cpp index f047616d79..3baf3c6d67 100644 --- a/src/test/nodestore/varint_test.cpp +++ b/src/test/nodestore/varint_test.cpp @@ -17,9 +17,8 @@ */ //============================================================================== -#include - #include +#include #include #include diff --git a/src/test/overlay/compression_test.cpp b/src/test/overlay/compression_test.cpp index 4bfbcae4f0..9c1ec7cd3d 100644 --- a/src/test/overlay/compression_test.cpp +++ b/src/test/overlay/compression_test.cpp @@ -30,7 +30,6 @@ #include #include #include -#include #include #include @@ -42,6 +41,7 @@ #include #include #include +#include #include #include diff --git a/src/test/shamap/FetchPack_test.cpp b/src/test/shamap/FetchPack_test.cpp index 04385f9441..7ee5c6730a 100644 --- a/src/test/shamap/FetchPack_test.cpp +++ b/src/test/shamap/FetchPack_test.cpp @@ -20,15 +20,14 @@ #include #include -#include -#include - #include #include #include #include #include #include +#include +#include #include #include diff --git a/src/test/shamap/SHAMapSync_test.cpp b/src/test/shamap/SHAMapSync_test.cpp index c3af07f036..05fc265ff9 100644 --- a/src/test/shamap/SHAMapSync_test.cpp +++ b/src/test/shamap/SHAMapSync_test.cpp @@ -20,12 +20,11 @@ #include #include -#include -#include - #include #include #include +#include +#include namespace ripple { namespace tests { diff --git a/src/test/shamap/SHAMap_test.cpp b/src/test/shamap/SHAMap_test.cpp index 1a15310b58..462b42b6cb 100644 --- a/src/test/shamap/SHAMap_test.cpp +++ b/src/test/shamap/SHAMap_test.cpp @@ -20,12 +20,11 @@ #include #include -#include - #include #include #include #include +#include namespace ripple { namespace tests { diff --git a/src/test/shamap/common.h b/src/test/shamap/common.h index 47308a82f8..6c7fcf4c39 100644 --- a/src/test/shamap/common.h +++ b/src/test/shamap/common.h @@ -20,11 +20,10 @@ #ifndef RIPPLE_SHAMAP_TESTS_COMMON_H_INCLUDED #define RIPPLE_SHAMAP_TESTS_COMMON_H_INCLUDED -#include -#include -#include - #include +#include +#include +#include namespace ripple { namespace tests { diff --git a/src/xrpld/app/consensus/RCLCensorshipDetector.h b/src/xrpld/app/consensus/RCLCensorshipDetector.h index 7cdf15949d..f291c6c31b 100644 --- a/src/xrpld/app/consensus/RCLCensorshipDetector.h +++ b/src/xrpld/app/consensus/RCLCensorshipDetector.h @@ -20,9 +20,8 @@ #ifndef RIPPLE_APP_CONSENSUS_RCLCENSORSHIPDETECTOR_H_INCLUDED #define RIPPLE_APP_CONSENSUS_RCLCENSORSHIPDETECTOR_H_INCLUDED -#include - #include +#include #include #include diff --git a/src/xrpld/app/consensus/RCLConsensus.h b/src/xrpld/app/consensus/RCLConsensus.h index 38481d2363..19568fca82 100644 --- a/src/xrpld/app/consensus/RCLConsensus.h +++ b/src/xrpld/app/consensus/RCLConsensus.h @@ -28,10 +28,10 @@ #include #include #include -#include #include #include +#include #include #include diff --git a/src/xrpld/app/consensus/RCLCxTx.h b/src/xrpld/app/consensus/RCLCxTx.h index baa0899f02..94f43113a8 100644 --- a/src/xrpld/app/consensus/RCLCxTx.h +++ b/src/xrpld/app/consensus/RCLCxTx.h @@ -20,7 +20,7 @@ #ifndef RIPPLE_APP_CONSENSUS_RCLCXTX_H_INCLUDED #define RIPPLE_APP_CONSENSUS_RCLCXTX_H_INCLUDED -#include +#include namespace ripple { diff --git a/src/xrpld/app/ledger/AccountStateSF.h b/src/xrpld/app/ledger/AccountStateSF.h index 16cc686e3d..8a4e594f4a 100644 --- a/src/xrpld/app/ledger/AccountStateSF.h +++ b/src/xrpld/app/ledger/AccountStateSF.h @@ -21,8 +21,9 @@ #define RIPPLE_APP_LEDGER_ACCOUNTSTATESF_H_INCLUDED #include -#include -#include + +#include +#include namespace ripple { diff --git a/src/xrpld/app/ledger/ConsensusTransSetSF.cpp b/src/xrpld/app/ledger/ConsensusTransSetSF.cpp index e62426b720..7d83b4c201 100644 --- a/src/xrpld/app/ledger/ConsensusTransSetSF.cpp +++ b/src/xrpld/app/ledger/ConsensusTransSetSF.cpp @@ -22,9 +22,9 @@ #include #include #include -#include #include +#include #include #include diff --git a/src/xrpld/app/ledger/ConsensusTransSetSF.h b/src/xrpld/app/ledger/ConsensusTransSetSF.h index 4a68f577ad..11add63db0 100644 --- a/src/xrpld/app/ledger/ConsensusTransSetSF.h +++ b/src/xrpld/app/ledger/ConsensusTransSetSF.h @@ -21,9 +21,9 @@ #define RIPPLE_APP_LEDGER_CONSENSUSTRANSSETSF_H_INCLUDED #include -#include #include +#include namespace ripple { diff --git a/src/xrpld/app/ledger/InboundTransactions.h b/src/xrpld/app/ledger/InboundTransactions.h index 6feee44004..fc39f195ab 100644 --- a/src/xrpld/app/ledger/InboundTransactions.h +++ b/src/xrpld/app/ledger/InboundTransactions.h @@ -21,9 +21,9 @@ #define RIPPLE_APP_LEDGER_INBOUNDTRANSACTIONS_H_INCLUDED #include -#include #include +#include #include diff --git a/src/xrpld/app/ledger/Ledger.cpp b/src/xrpld/app/ledger/Ledger.cpp index 64a6b16cbc..cf99fbfaac 100644 --- a/src/xrpld/app/ledger/Ledger.cpp +++ b/src/xrpld/app/ledger/Ledger.cpp @@ -28,13 +28,13 @@ #include #include #include -#include -#include #include #include #include #include +#include +#include #include #include #include diff --git a/src/xrpld/app/ledger/Ledger.h b/src/xrpld/app/ledger/Ledger.h index 552f59ff19..5b74ee242a 100644 --- a/src/xrpld/app/ledger/Ledger.h +++ b/src/xrpld/app/ledger/Ledger.h @@ -22,7 +22,6 @@ #include #include -#include #include #include @@ -32,6 +31,7 @@ #include #include #include +#include namespace ripple { diff --git a/src/xrpld/app/ledger/TransactionMaster.h b/src/xrpld/app/ledger/TransactionMaster.h index f6993dc0e8..13e968e2f5 100644 --- a/src/xrpld/app/ledger/TransactionMaster.h +++ b/src/xrpld/app/ledger/TransactionMaster.h @@ -21,12 +21,12 @@ #define RIPPLE_APP_LEDGER_TRANSACTIONMASTER_H_INCLUDED #include -#include -#include #include #include #include +#include +#include namespace ripple { diff --git a/src/xrpld/app/ledger/TransactionStateSF.h b/src/xrpld/app/ledger/TransactionStateSF.h index 721f187062..64dfed7142 100644 --- a/src/xrpld/app/ledger/TransactionStateSF.h +++ b/src/xrpld/app/ledger/TransactionStateSF.h @@ -21,8 +21,9 @@ #define RIPPLE_APP_LEDGER_TRANSACTIONSTATESF_H_INCLUDED #include -#include -#include + +#include +#include namespace ripple { diff --git a/src/xrpld/app/ledger/detail/InboundLedger.cpp b/src/xrpld/app/ledger/detail/InboundLedger.cpp index 47d546c3af..9bebb1ed38 100644 --- a/src/xrpld/app/ledger/detail/InboundLedger.cpp +++ b/src/xrpld/app/ledger/detail/InboundLedger.cpp @@ -25,12 +25,12 @@ #include #include #include -#include #include #include #include #include +#include #include diff --git a/src/xrpld/app/ledger/detail/SkipListAcquire.h b/src/xrpld/app/ledger/detail/SkipListAcquire.h index 0e97945174..02ab81ed6b 100644 --- a/src/xrpld/app/ledger/detail/SkipListAcquire.h +++ b/src/xrpld/app/ledger/detail/SkipListAcquire.h @@ -24,7 +24,8 @@ #include #include #include -#include + +#include namespace ripple { class InboundLedgers; diff --git a/src/xrpld/app/ledger/detail/TransactionAcquire.h b/src/xrpld/app/ledger/detail/TransactionAcquire.h index f0d9b40928..9289758727 100644 --- a/src/xrpld/app/ledger/detail/TransactionAcquire.h +++ b/src/xrpld/app/ledger/detail/TransactionAcquire.h @@ -21,7 +21,8 @@ #define RIPPLE_APP_LEDGER_TRANSACTIONACQUIRE_H_INCLUDED #include -#include + +#include namespace ripple { diff --git a/src/xrpld/app/main/Application.cpp b/src/xrpld/app/main/Application.cpp index 616afc957d..12a5b3d409 100644 --- a/src/xrpld/app/main/Application.cpp +++ b/src/xrpld/app/main/Application.cpp @@ -48,7 +48,6 @@ #include #include #include -#include #include #include #include @@ -64,6 +63,7 @@ #include #include #include +#include #include #include #include diff --git a/src/xrpld/app/main/Application.h b/src/xrpld/app/main/Application.h index b3a433fee8..6dc50097bb 100644 --- a/src/xrpld/app/main/Application.h +++ b/src/xrpld/app/main/Application.h @@ -22,11 +22,11 @@ #include #include -#include #include #include #include +#include #include #include diff --git a/src/xrpld/app/main/NodeStoreScheduler.h b/src/xrpld/app/main/NodeStoreScheduler.h index c21cc93934..ffd024fe32 100644 --- a/src/xrpld/app/main/NodeStoreScheduler.h +++ b/src/xrpld/app/main/NodeStoreScheduler.h @@ -21,7 +21,8 @@ #define RIPPLE_APP_MAIN_NODESTORESCHEDULER_H_INCLUDED #include -#include + +#include namespace ripple { diff --git a/src/xrpld/app/misc/FeeVote.h b/src/xrpld/app/misc/FeeVote.h index 543456785a..7992163567 100644 --- a/src/xrpld/app/misc/FeeVote.h +++ b/src/xrpld/app/misc/FeeVote.h @@ -20,10 +20,9 @@ #ifndef RIPPLE_APP_MISC_FEEVOTE_H_INCLUDED #define RIPPLE_APP_MISC_FEEVOTE_H_INCLUDED -#include - #include #include +#include namespace ripple { diff --git a/src/xrpld/app/misc/NegativeUNLVote.cpp b/src/xrpld/app/misc/NegativeUNLVote.cpp index 471c937472..d981ba200c 100644 --- a/src/xrpld/app/misc/NegativeUNLVote.cpp +++ b/src/xrpld/app/misc/NegativeUNLVote.cpp @@ -20,7 +20,8 @@ #include #include #include -#include + +#include namespace ripple { diff --git a/src/xrpld/app/misc/SHAMapStore.h b/src/xrpld/app/misc/SHAMapStore.h index d2836be287..1a0cbca5a5 100644 --- a/src/xrpld/app/misc/SHAMapStore.h +++ b/src/xrpld/app/misc/SHAMapStore.h @@ -21,7 +21,8 @@ #define RIPPLE_APP_MISC_SHAMAPSTORE_H_INCLUDED #include -#include + +#include #include diff --git a/src/xrpld/app/misc/SHAMapStoreImp.cpp b/src/xrpld/app/misc/SHAMapStoreImp.cpp index 52ea40cf94..d775a2063a 100644 --- a/src/xrpld/app/misc/SHAMapStoreImp.cpp +++ b/src/xrpld/app/misc/SHAMapStoreImp.cpp @@ -23,11 +23,11 @@ #include #include #include -#include -#include -#include #include +#include +#include +#include #include diff --git a/src/xrpld/app/misc/SHAMapStoreImp.h b/src/xrpld/app/misc/SHAMapStoreImp.h index 2b618f6538..411035809b 100644 --- a/src/xrpld/app/misc/SHAMapStoreImp.h +++ b/src/xrpld/app/misc/SHAMapStoreImp.h @@ -24,8 +24,9 @@ #include #include #include -#include -#include + +#include +#include #include #include diff --git a/src/xrpld/rpc/handlers/GetCounts.cpp b/src/xrpld/rpc/handlers/GetCounts.cpp index 3c1d8cccdd..659e796e1d 100644 --- a/src/xrpld/rpc/handlers/GetCounts.cpp +++ b/src/xrpld/rpc/handlers/GetCounts.cpp @@ -23,11 +23,11 @@ #include #include #include -#include #include #include #include +#include #include #include diff --git a/src/xrpld/shamap/detail/NodeFamily.cpp b/src/xrpld/shamap/NodeFamily.cpp similarity index 98% rename from src/xrpld/shamap/detail/NodeFamily.cpp rename to src/xrpld/shamap/NodeFamily.cpp index 6126534966..6d3b0e0dd5 100644 --- a/src/xrpld/shamap/detail/NodeFamily.cpp +++ b/src/xrpld/shamap/NodeFamily.cpp @@ -19,11 +19,10 @@ #include #include +#include #include #include -#include - namespace ripple { NodeFamily::NodeFamily(Application& app, CollectorManager& cm) diff --git a/src/xrpld/shamap/NodeFamily.h b/src/xrpld/shamap/NodeFamily.h index 4062ea2389..b96adb334e 100644 --- a/src/xrpld/shamap/NodeFamily.h +++ b/src/xrpld/shamap/NodeFamily.h @@ -20,8 +20,7 @@ #ifndef RIPPLE_SHAMAP_NODEFAMILY_H_INCLUDED #define RIPPLE_SHAMAP_NODEFAMILY_H_INCLUDED -#include -#include +#include namespace ripple { From 25c5e3b17f9563163c6c57e4c9c0af99d4eb1539 Mon Sep 17 00:00:00 2001 From: Bart Date: Mon, 3 Nov 2025 13:53:19 +0000 Subject: [PATCH 7/9] chore: Remove version number in find_dependency for OpenSSL (#5985) We are already using OpenSSL 3.5.2. The version number in the `find_dependency` statement is optional, and belongs in `conanfile.py` anyway. --- cmake/RippleConfig.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/RippleConfig.cmake b/cmake/RippleConfig.cmake index 445010e915..e0de962d3f 100644 --- a/cmake/RippleConfig.cmake +++ b/cmake/RippleConfig.cmake @@ -45,7 +45,7 @@ if (static OR APPLE OR MSVC) set (OPENSSL_USE_STATIC_LIBS ON) endif () set (OPENSSL_MSVC_STATIC_RT ON) -find_dependency (OpenSSL 1.1.1 REQUIRED) +find_dependency (OpenSSL REQUIRED) find_dependency (ZLIB) find_dependency (date) if (TARGET ZLIB::ZLIB) From 12c4b5a632fb04bf8daf84b7bc6bfee076859929 Mon Sep 17 00:00:00 2001 From: Bart Date: Mon, 3 Nov 2025 16:57:24 +0000 Subject: [PATCH 8/9] ci: Update CI image hashes to use netstat (#5987) To debug test failures we would like to use `netstat`, but that package wasn't installed yet in the CI images. This change uses the new CI images created by https://github.com/XRPLF/ci/pull/79. --- .github/scripts/strategy-matrix/linux.json | 48 +++++++++++----------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/.github/scripts/strategy-matrix/linux.json b/.github/scripts/strategy-matrix/linux.json index 317fa640b2..85a78c96dc 100644 --- a/.github/scripts/strategy-matrix/linux.json +++ b/.github/scripts/strategy-matrix/linux.json @@ -15,168 +15,168 @@ "distro_version": "bookworm", "compiler_name": "gcc", "compiler_version": "12", - "image_sha": "6948666" + "image_sha": "97ba375" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "gcc", "compiler_version": "13", - "image_sha": "6948666" + "image_sha": "97ba375" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "gcc", "compiler_version": "14", - "image_sha": "6948666" + "image_sha": "97ba375" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "gcc", "compiler_version": "15", - "image_sha": "6948666" + "image_sha": "97ba375" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", "compiler_version": "16", - "image_sha": "6948666" + "image_sha": "97ba375" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", "compiler_version": "17", - "image_sha": "6948666" + "image_sha": "97ba375" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", "compiler_version": "18", - "image_sha": "6948666" + "image_sha": "97ba375" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", "compiler_version": "19", - "image_sha": "6948666" + "image_sha": "97ba375" }, { "distro_name": "debian", "distro_version": "bookworm", "compiler_name": "clang", "compiler_version": "20", - "image_sha": "6948666" + "image_sha": "97ba375" }, { "distro_name": "rhel", "distro_version": "8", "compiler_name": "gcc", "compiler_version": "14", - "image_sha": "10e69b4" + "image_sha": "97ba375" }, { "distro_name": "rhel", "distro_version": "8", "compiler_name": "clang", "compiler_version": "any", - "image_sha": "10e69b4" + "image_sha": "97ba375" }, { "distro_name": "rhel", "distro_version": "9", "compiler_name": "gcc", "compiler_version": "12", - "image_sha": "10e69b4" + "image_sha": "97ba375" }, { "distro_name": "rhel", "distro_version": "9", "compiler_name": "gcc", "compiler_version": "13", - "image_sha": "10e69b4" + "image_sha": "97ba375" }, { "distro_name": "rhel", "distro_version": "9", "compiler_name": "gcc", "compiler_version": "14", - "image_sha": "10e69b4" + "image_sha": "97ba375" }, { "distro_name": "rhel", "distro_version": "9", "compiler_name": "clang", "compiler_version": "any", - "image_sha": "10e69b4" + "image_sha": "97ba375" }, { "distro_name": "rhel", "distro_version": "10", "compiler_name": "gcc", "compiler_version": "14", - "image_sha": "10e69b4" + "image_sha": "97ba375" }, { "distro_name": "rhel", "distro_version": "10", "compiler_name": "clang", "compiler_version": "any", - "image_sha": "10e69b4" + "image_sha": "97ba375" }, { "distro_name": "ubuntu", "distro_version": "jammy", "compiler_name": "gcc", "compiler_version": "12", - "image_sha": "6948666" + "image_sha": "97ba375" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "gcc", "compiler_version": "13", - "image_sha": "6948666" + "image_sha": "97ba375" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "gcc", "compiler_version": "14", - "image_sha": "6948666" + "image_sha": "97ba375" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "clang", "compiler_version": "16", - "image_sha": "6948666" + "image_sha": "97ba375" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "clang", "compiler_version": "17", - "image_sha": "6948666" + "image_sha": "97ba375" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "clang", "compiler_version": "18", - "image_sha": "6948666" + "image_sha": "97ba375" }, { "distro_name": "ubuntu", "distro_version": "noble", "compiler_name": "clang", "compiler_version": "19", - "image_sha": "6948666" + "image_sha": "97ba375" } ], "build_type": ["Debug", "Release"], From 8ac8a47c99ac8d05e89bf88fa72f0c17eb21aee5 Mon Sep 17 00:00:00 2001 From: Jingchen Date: Mon, 3 Nov 2025 17:26:12 +0000 Subject: [PATCH 9/9] refactor: Retire ImmediateOfferKilled amendment (#5973) Amendments activated for more than 2 years can be retired. This change retires the ImmediateOfferKilled amendment. --- include/xrpl/protocol/detail/features.macro | 2 +- src/test/app/Offer_test.cpp | 57 ++------------------- src/xrpld/app/tx/detail/CreateOffer.cpp | 5 +- 3 files changed, 8 insertions(+), 56 deletions(-) diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index b8398f6da5..1a0d89f895 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -82,7 +82,6 @@ XRPL_FIX (NonFungibleTokensV1_2, Supported::yes, VoteBehavior::DefaultNo XRPL_FIX (UniversalNumber, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(XRPFees, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(DisallowIncoming, Supported::yes, VoteBehavior::DefaultNo) -XRPL_FEATURE(ImmediateOfferKilled, Supported::yes, VoteBehavior::DefaultNo) XRPL_FIX (RemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYes) XRPL_FIX (TrustLinesToSelf, Supported::yes, VoteBehavior::DefaultNo) XRPL_FEATURE(NonFungibleTokensV1_1, Supported::yes, VoteBehavior::DefaultNo) @@ -149,6 +148,7 @@ XRPL_RETIRE(Escrow) XRPL_RETIRE(EnforceInvariants) XRPL_RETIRE(FeeEscalation) XRPL_RETIRE(FlowCross) +XRPL_RETIRE(ImmediateOfferKilled) XRPL_RETIRE(MultiSign) XRPL_RETIRE(PayChan) XRPL_RETIRE(SortedDirectories) diff --git a/src/test/app/Offer_test.cpp b/src/test/app/Offer_test.cpp index 0dbcfccebd..5d56a72cc7 100644 --- a/src/test/app/Offer_test.cpp +++ b/src/test/app/Offer_test.cpp @@ -855,9 +855,7 @@ public: // No cross: { - TER const expectedCode = features[featureImmediateOfferKilled] - ? static_cast(tecKILLED) - : static_cast(tesSUCCESS); + TER const expectedCode = tecKILLED; env(offer(alice, XRP(1000), USD(1000)), txflags(tfImmediateOrCancel), ter(expectedCode)); @@ -5289,34 +5287,12 @@ public: testFillOrKill(features); } - void - run(std::uint32_t instance, bool last = false) - { - using namespace jtx; - static FeatureBitset const all{testable_amendments()}; - static FeatureBitset const immediateOfferKilled{ - featureImmediateOfferKilled}; - FeatureBitset const fillOrKill{fixFillOrKill}; - FeatureBitset const permDEX{featurePermissionedDEX}; - - static std::array const feats{ - all - immediateOfferKilled - permDEX, - all - immediateOfferKilled - fillOrKill - permDEX, - all - fillOrKill - permDEX, - all - permDEX, - all}; - - if (BEAST_EXPECT(instance < feats.size())) - { - testAll(feats[instance]); - } - BEAST_EXPECT(!last || instance == feats.size() - 1); - } + FeatureBitset const allFeatures{jtx::testable_amendments()}; void run() override { - run(0); + testAll(allFeatures - featurePermissionedDEX); testFalseAssert(); } }; @@ -5326,25 +5302,7 @@ class OfferWOSmallQOffers_test : public OfferBaseUtil_test void run() override { - OfferBaseUtil_test::run(1); - } -}; - -class OfferWOFillOrKill_test : public OfferBaseUtil_test -{ - void - run() override - { - OfferBaseUtil_test::run(2); - } -}; - -class OfferWOPermDEX_test : public OfferBaseUtil_test -{ - void - run() override - { - OfferBaseUtil_test::run(3); + testAll(allFeatures - fixFillOrKill - featurePermissionedDEX); } }; @@ -5353,7 +5311,7 @@ class OfferAllFeatures_test : public OfferBaseUtil_test void run() override { - OfferBaseUtil_test::run(4, true); + testAll(allFeatures); } }; @@ -5364,12 +5322,9 @@ class Offer_manual_test : public OfferBaseUtil_test { using namespace jtx; FeatureBitset const all{testable_amendments()}; - FeatureBitset const immediateOfferKilled{featureImmediateOfferKilled}; FeatureBitset const fillOrKill{fixFillOrKill}; FeatureBitset const permDEX{featurePermissionedDEX}; - testAll(all - immediateOfferKilled - permDEX); - testAll(all - immediateOfferKilled - fillOrKill - permDEX); testAll(all - fillOrKill - permDEX); testAll(all - permDEX); testAll(all); @@ -5378,8 +5333,6 @@ class Offer_manual_test : public OfferBaseUtil_test BEAST_DEFINE_TESTSUITE_PRIO(OfferBaseUtil, app, ripple, 2); BEAST_DEFINE_TESTSUITE_PRIO(OfferWOSmallQOffers, app, ripple, 2); -BEAST_DEFINE_TESTSUITE_PRIO(OfferWOFillOrKill, app, ripple, 2); -BEAST_DEFINE_TESTSUITE_PRIO(OfferWOPermDEX, app, ripple, 2); BEAST_DEFINE_TESTSUITE_PRIO(OfferAllFeatures, app, ripple, 2); BEAST_DEFINE_TESTSUITE_MANUAL_PRIO(Offer_manual, app, ripple, 20); diff --git a/src/xrpld/app/tx/detail/CreateOffer.cpp b/src/xrpld/app/tx/detail/CreateOffer.cpp index a7884a825c..c9c78f0e82 100644 --- a/src/xrpld/app/tx/detail/CreateOffer.cpp +++ b/src/xrpld/app/tx/detail/CreateOffer.cpp @@ -789,9 +789,8 @@ CreateOffer::applyGuts(Sandbox& sb, Sandbox& sbCancel) if (bImmediateOrCancel) { JLOG(j_.trace()) << "Immediate or cancel: offer canceled"; - if (!crossed && sb.rules().enabled(featureImmediateOfferKilled)) - // If the ImmediateOfferKilled amendment is enabled, any - // ImmediateOrCancel offer that transfers absolutely no funds + if (!crossed) + // Any ImmediateOrCancel offer that transfers absolutely no funds // returns tecKILLED rather than tesSUCCESS. Motivation for the // change is here: https://github.com/ripple/rippled/issues/4115 return {tecKILLED, false};