diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index 573dca951d..1ccb60d3af 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/include/xrpl/protocol/detail/features.macro @@ -64,7 +64,6 @@ XRPL_FEATURE(DID, Supported::Yes, VoteBehavior::DefaultNo 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_FEATURE(XRPFees, Supported::Yes, VoteBehavior::DefaultNo) XRPL_FIX (RemoveNFTokenAutoTrustLine, Supported::Yes, VoteBehavior::DefaultYes) @@ -116,6 +115,7 @@ XRPL_RETIRE_FIX(UniversalNumber) XRPL_RETIRE_FEATURE(Checks) XRPL_RETIRE_FEATURE(CheckCashMakesTrustLine) +XRPL_RETIRE_FEATURE(Clawback) XRPL_RETIRE_FEATURE(CryptoConditions) XRPL_RETIRE_FEATURE(CryptoConditionsSuite) XRPL_RETIRE_FEATURE(DeletableAccounts) diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index 450e2558cc..dbaaf46083 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -395,7 +395,7 @@ TRANSACTION(ttNFTOKEN_ACCEPT_OFFER, 29, NFTokenAcceptOffer, #endif TRANSACTION(ttCLAWBACK, 30, Clawback, Delegation::Delegable, - featureClawback, + uint256{}, NoPriv, ({ {sfAmount, SoeRequired, SoeMptSupported}, diff --git a/include/xrpl/protocol_autogen/transactions/Clawback.h b/include/xrpl/protocol_autogen/transactions/Clawback.h index bbf1c411f3..ecd7ebe7a2 100644 --- a/include/xrpl/protocol_autogen/transactions/Clawback.h +++ b/include/xrpl/protocol_autogen/transactions/Clawback.h @@ -20,7 +20,7 @@ class ClawbackBuilder; * * Type: ttCLAWBACK (30) * Delegable: Delegation::Delegable - * Amendment: featureClawback + * Amendment: uint256{} * Privileges: NoPriv * * Immutable wrapper around STTx providing type-safe field access. diff --git a/src/libxrpl/ledger/CanonicalTXSet.cpp b/src/libxrpl/ledger/CanonicalTXSet.cpp index df4e88e346..12cec5e321 100644 --- a/src/libxrpl/ledger/CanonicalTXSet.cpp +++ b/src/libxrpl/ledger/CanonicalTXSet.cpp @@ -42,7 +42,8 @@ CanonicalTXSet::accountKey(AccountID const& account) void CanonicalTXSet::insert(std::shared_ptr txn) { - Key key(accountKey(txn->getAccountID(sfAccount)), txn->getSeqProxy(), txn->getTransactionID()); + Key const key( + accountKey(txn->getAccountID(sfAccount)), txn->getSeqProxy(), txn->getTransactionID()); map_.emplace(key, std::move(txn)); } diff --git a/src/libxrpl/tx/transactors/account/AccountSet.cpp b/src/libxrpl/tx/transactors/account/AccountSet.cpp index dfe7ec5b5f..f0ad5b113a 100644 --- a/src/libxrpl/tx/transactors/account/AccountSet.cpp +++ b/src/libxrpl/tx/transactors/account/AccountSet.cpp @@ -194,30 +194,27 @@ AccountSet::preclaim(PreclaimContext const& ctx) // // Clawback // - if (ctx.view.rules().enabled(featureClawback)) + if (uSetFlag == asfAllowTrustLineClawback) { - if (uSetFlag == asfAllowTrustLineClawback) + if (sle->isFlag(lsfNoFreeze)) { - if (sle->isFlag(lsfNoFreeze)) - { - JLOG(ctx.j.trace()) << "Can't set Clawback if NoFreeze is set"; - return tecNO_PERMISSION; - } - - if (!dirIsEmpty(ctx.view, keylet::ownerDir(id))) - { - JLOG(ctx.j.trace()) << "Owner directory not empty."; - return tecOWNERS; - } + JLOG(ctx.j.trace()) << "Can't set Clawback if NoFreeze is set"; + return tecNO_PERMISSION; } - else if (uSetFlag == asfNoFreeze) + + if (!dirIsEmpty(ctx.view, keylet::ownerDir(id))) { - // Cannot set NoFreeze if clawback is enabled - if (sle->isFlag(lsfAllowTrustLineClawback)) - { - JLOG(ctx.j.trace()) << "Can't set NoFreeze if clawback is enabled"; - return tecNO_PERMISSION; - } + JLOG(ctx.j.trace()) << "Owner directory not empty."; + return tecOWNERS; + } + } + else if (uSetFlag == asfNoFreeze) + { + // Cannot set NoFreeze if clawback is enabled + if (sle->isFlag(lsfAllowTrustLineClawback)) + { + JLOG(ctx.j.trace()) << "Can't set NoFreeze if clawback is enabled"; + return tecNO_PERMISSION; } } @@ -576,7 +573,7 @@ AccountSet::doApply() } // Set flag for clawback - if (ctx_.view().rules().enabled(featureClawback) && uSetFlag == asfAllowTrustLineClawback) + if (uSetFlag == asfAllowTrustLineClawback) { JLOG(j_.trace()) << "set allow clawback"; uFlagsOut |= lsfAllowTrustLineClawback; diff --git a/src/test/app/Clawback_test.cpp b/src/test/app/Clawback_test.cpp index b1a756382d..2dadd9f503 100644 --- a/src/test/app/Clawback_test.cpp +++ b/src/test/app/Clawback_test.cpp @@ -161,35 +161,6 @@ class Clawback_test : public beast::unit_test::Suite BEAST_EXPECT(ownerCount(env, alice) == 0); BEAST_EXPECT(ownerCount(env, bob) == 0); } - - // Test that one cannot enable asfAllowTrustLineClawback when - // featureClawback amendment is disabled - { - Env env(*this, features - featureClawback); - - Account const alice{"alice"}; - - env.fund(XRP(1000), alice); - env.close(); - - env.require(Nflags(alice, asfAllowTrustLineClawback)); - - // alice attempts to set asfAllowTrustLineClawback flag while - // amendment is disabled. no error is returned, but the flag remains - // to be unset. - env(fset(alice, asfAllowTrustLineClawback)); - env.close(); - env.require(Nflags(alice, asfAllowTrustLineClawback)); - - // now enable clawback amendment - env.enableFeature(featureClawback); - env.close(); - - // asfAllowTrustLineClawback can be set - env(fset(alice, asfAllowTrustLineClawback)); - env.close(); - env.require(Flags(alice, asfAllowTrustLineClawback)); - } } void @@ -198,46 +169,6 @@ class Clawback_test : public beast::unit_test::Suite testcase("Validation"); using namespace test::jtx; - // Test that Clawback tx fails for the following: - // 1. when amendment is disabled - // 2. when asfAllowTrustLineClawback flag has not been set - { - Env env(*this, features - featureClawback); - - Account const alice{"alice"}; - Account const bob{"bob"}; - - env.fund(XRP(1000), alice, bob); - env.close(); - - env.require(Nflags(alice, asfAllowTrustLineClawback)); - - auto const usd = alice["USD"]; - - // alice issues 10 USD to bob - env.trust(usd(1000), bob); - env(pay(alice, bob, usd(10))); - env.close(); - - env.require(Balance(bob, alice["USD"](10))); - env.require(Balance(alice, bob["USD"](-10))); - - // clawback fails because amendment is disabled - env(claw(alice, bob["USD"](5)), Ter(temDISABLED)); - env.close(); - - // now enable clawback amendment - env.enableFeature(featureClawback); - env.close(); - - // clawback fails because asfAllowTrustLineClawback has not been set - env(claw(alice, bob["USD"](5)), Ter(tecNO_PERMISSION)); - env.close(); - - env.require(Balance(bob, alice["USD"](10))); - env.require(Balance(alice, bob["USD"](-10))); - } - // Test that Clawback tx fails for the following: // 1. invalid flag // 2. negative STAmount diff --git a/src/test/app/Delegate_test.cpp b/src/test/app/Delegate_test.cpp index 1516219e46..6e80577797 100644 --- a/src/test/app/Delegate_test.cpp +++ b/src/test/app/Delegate_test.cpp @@ -2340,7 +2340,6 @@ class Delegate_test : public beast::unit_test::Suite // NFTokenMint, NFTokenBurn, NFTokenCreateOffer, NFTokenCancelOffer, // NFTokenAcceptOffer are not included, they are tested separately. std::unordered_map txRequiredFeatures{ - {"Clawback", featureClawback}, {"AMMClawback", featureAMMClawback}, {"AMMCreate", featureAMM}, {"AMMDeposit", featureAMM}, diff --git a/src/test/rpc/AccountInfo_test.cpp b/src/test/rpc/AccountInfo_test.cpp index 385fc2f58a..d061be1f0e 100644 --- a/src/test/rpc/AccountInfo_test.cpp +++ b/src/test/rpc/AccountInfo_test.cpp @@ -583,24 +583,17 @@ public: static constexpr std::pair kAllowTrustLineClawbackFlag{ "allowTrustLineClawback", asfAllowTrustLineClawback}; - if (features[featureClawback]) - { - // must use bob's account because alice has noFreeze set - auto const f1 = getAccountFlag(kAllowTrustLineClawbackFlag.first, bob); - BEAST_EXPECT(f1.has_value()); - BEAST_EXPECT(!f1.value()); // NOLINT(bugprone-unchecked-optional-access) + // must use bob's account because alice has noFreeze set + auto const f1 = getAccountFlag(kAllowTrustLineClawbackFlag.first, bob); + BEAST_EXPECT(f1.has_value()); + BEAST_EXPECT(!f1.value()); // NOLINT(bugprone-unchecked-optional-access) - // Set allowTrustLineClawback - env(fset(bob, kAllowTrustLineClawbackFlag.second)); - env.close(); - auto const f2 = getAccountFlag(kAllowTrustLineClawbackFlag.first, bob); - BEAST_EXPECT(f2.has_value()); - BEAST_EXPECT(f2.value()); // NOLINT(bugprone-unchecked-optional-access) - } - else - { - BEAST_EXPECT(!getAccountFlag(kAllowTrustLineClawbackFlag.first, bob)); - } + // Set allowTrustLineClawback + env(fset(bob, kAllowTrustLineClawbackFlag.second)); + env.close(); + auto const f2 = getAccountFlag(kAllowTrustLineClawbackFlag.first, bob); + BEAST_EXPECT(f2.has_value()); + BEAST_EXPECT(f2.value()); // NOLINT(bugprone-unchecked-optional-access) static constexpr std::pair kAllowTrustLineLockingFlag{ "allowTrustLineLocking", asfAllowTrustLineLocking}; @@ -634,8 +627,7 @@ public: FeatureBitset const allFeatures{xrpl::test::jtx::testableAmendments()}; testAccountFlags(allFeatures); - testAccountFlags(allFeatures - featureClawback); - testAccountFlags(allFeatures - featureClawback - featureTokenEscrow); + testAccountFlags(allFeatures - featureTokenEscrow); } }; diff --git a/src/xrpld/rpc/handlers/account/AccountInfo.cpp b/src/xrpld/rpc/handlers/account/AccountInfo.cpp index fba48ea93e..3a96593452 100644 --- a/src/xrpld/rpc/handlers/account/AccountInfo.cpp +++ b/src/xrpld/rpc/handlers/account/AccountInfo.cpp @@ -169,11 +169,8 @@ doAccountInfo(RPC::JsonContext& context) for (auto const& lsf : kDisallowIncomingFlags) acctFlags[lsf.first.data()] = sleAccepted->isFlag(lsf.second); - if (ledger->rules().enabled(featureClawback)) - { - acctFlags[kAllowTrustLineClawbackFlag.first.data()] = - sleAccepted->isFlag(kAllowTrustLineClawbackFlag.second); - } + acctFlags[kAllowTrustLineClawbackFlag.first.data()] = + sleAccepted->isFlag(kAllowTrustLineClawbackFlag.second); if (ledger->rules().enabled(featureTokenEscrow)) {