From 683e9ccc1a2c7e609ac4e875c58edee01e8899cd Mon Sep 17 00:00:00 2001 From: Shawn Xie <35279399+shawnxie999@users.noreply.github.com> Date: Thu, 13 Jul 2023 21:00:32 -0400 Subject: [PATCH] Rename `allowClawback` flag to `allowTrustLineClawback` (#4617) Reason for this change is here XRPLF/XRPL-Standards#119 We would want to be explicit that this flag is exclusively for trustline. For new token types(eg. CFT), they will not utilize this flag for clawback, instead, they will turn clawback on/off on the token-level, which is more versatile. --- src/ripple/app/tx/impl/Clawback.cpp | 6 +- src/ripple/app/tx/impl/SetAccount.cpp | 8 +- src/ripple/protocol/LedgerFormats.h | 2 +- src/ripple/protocol/TxFlags.h | 2 +- src/ripple/rpc/handlers/AccountInfo.cpp | 7 +- src/test/app/Clawback_test.cpp | 166 ++++++++++++------------ src/test/jtx/flags.h | 4 +- src/test/rpc/AccountInfo_test.cpp | 16 ++- src/test/rpc/AccountSet_test.cpp | 6 +- 9 files changed, 113 insertions(+), 104 deletions(-) diff --git a/src/ripple/app/tx/impl/Clawback.cpp b/src/ripple/app/tx/impl/Clawback.cpp index d4a16e7ad..4fb4d4bc8 100644 --- a/src/ripple/app/tx/impl/Clawback.cpp +++ b/src/ripple/app/tx/impl/Clawback.cpp @@ -65,8 +65,10 @@ Clawback::preclaim(PreclaimContext const& ctx) std::uint32_t const issuerFlagsIn = sleIssuer->getFieldU32(sfFlags); - // If AllowClawback is not set or NoFreeze is set, return no permission - if (!(issuerFlagsIn & lsfAllowClawback) || (issuerFlagsIn & lsfNoFreeze)) + // If AllowTrustLineClawback is not set or NoFreeze is set, return no + // permission + if (!(issuerFlagsIn & lsfAllowTrustLineClawback) || + (issuerFlagsIn & lsfNoFreeze)) return tecNO_PERMISSION; auto const sleRippleState = diff --git a/src/ripple/app/tx/impl/SetAccount.cpp b/src/ripple/app/tx/impl/SetAccount.cpp index 36b3b069b..8cbee5fd7 100644 --- a/src/ripple/app/tx/impl/SetAccount.cpp +++ b/src/ripple/app/tx/impl/SetAccount.cpp @@ -223,7 +223,7 @@ SetAccount::preclaim(PreclaimContext const& ctx) // if (ctx.view.rules().enabled(featureClawback)) { - if (uSetFlag == asfAllowClawback) + if (uSetFlag == asfAllowTrustLineClawback) { if (uFlagsIn & lsfNoFreeze) { @@ -240,7 +240,7 @@ SetAccount::preclaim(PreclaimContext const& ctx) else if (uSetFlag == asfNoFreeze) { // Cannot set NoFreeze if clawback is enabled - if (uFlagsIn & lsfAllowClawback) + if (uFlagsIn & lsfAllowTrustLineClawback) { JLOG(ctx.j.trace()) << "Can't set NoFreeze if clawback is enabled"; @@ -620,10 +620,10 @@ SetAccount::doApply() // Set flag for clawback if (ctx_.view().rules().enabled(featureClawback) && - uSetFlag == asfAllowClawback) + uSetFlag == asfAllowTrustLineClawback) { JLOG(j_.trace()) << "set allow clawback"; - uFlagsOut |= lsfAllowClawback; + uFlagsOut |= lsfAllowTrustLineClawback; } if (uFlagsIn != uFlagsOut) diff --git a/src/ripple/protocol/LedgerFormats.h b/src/ripple/protocol/LedgerFormats.h index 0903d4fb1..d1efed0ff 100644 --- a/src/ripple/protocol/LedgerFormats.h +++ b/src/ripple/protocol/LedgerFormats.h @@ -294,7 +294,7 @@ enum LedgerSpecificFlags { lsfDisallowIncomingRemit = // True, no remits allowed to this account 0x80000000, lsfAMM [[maybe_unused]] = 0x0004000, // True, AMM account - lsfAllowClawback = + lsfAllowTrustLineClawback = 0x00008000, // True, enable clawback // ltOFFER diff --git a/src/ripple/protocol/TxFlags.h b/src/ripple/protocol/TxFlags.h index e4c1489ca..253a422f1 100644 --- a/src/ripple/protocol/TxFlags.h +++ b/src/ripple/protocol/TxFlags.h @@ -92,7 +92,7 @@ enum AccountFlags : uint32_t { asfDisallowIncomingPayChan = 14, asfDisallowIncomingTrustline = 15, asfDisallowIncomingRemit = 16, - asfAllowClawback = 17, + asfAllowTrustLineClawback = 17, }; // OfferCreate flags: diff --git a/src/ripple/rpc/handlers/AccountInfo.cpp b/src/ripple/rpc/handlers/AccountInfo.cpp index cb4ce914c..fbb826256 100644 --- a/src/ripple/rpc/handlers/AccountInfo.cpp +++ b/src/ripple/rpc/handlers/AccountInfo.cpp @@ -100,7 +100,8 @@ doAccountInfo(RPC::JsonContext& context) {"disallowIncomingRemit", lsfDisallowIncomingRemit}}}; static constexpr std::pair - allowClawbackFlag{"allowClawback", lsfAllowClawback}; + allowTrustLineClawbackFlag{ + "allowTrustLineClawback", lsfAllowTrustLineClawback}; auto const sleAccepted = ledger->read(keylet::account(accountID)); if (sleAccepted) @@ -131,8 +132,8 @@ doAccountInfo(RPC::JsonContext& context) } if (ledger->rules().enabled(featureClawback)) - acctFlags[allowClawbackFlag.first.data()] = - sleAccepted->isFlag(allowClawbackFlag.second); + acctFlags[allowTrustLineClawbackFlag.first.data()] = + sleAccepted->isFlag(allowTrustLineClawbackFlag.second); result[jss::account_flags] = std::move(acctFlags); diff --git a/src/test/app/Clawback_test.cpp b/src/test/app/Clawback_test.cpp index 33b904979..630e4836d 100644 --- a/src/test/app/Clawback_test.cpp +++ b/src/test/app/Clawback_test.cpp @@ -75,14 +75,14 @@ class Clawback_test : public beast::unit_test::suite } void - testAllowClawbackFlag(FeatureBitset features) + testAllowTrustLineClawbackFlag(FeatureBitset features) { - testcase("Enable AllowClawback flag"); + testcase("Enable AllowTrustLineClawback flag"); using namespace test::jtx; - // Test that one can successfully set asfAllowClawback flag. + // Test that one can successfully set asfAllowTrustLineClawback flag. // If successful, asfNoFreeze can no longer be set. - // Also, asfAllowClawback cannot be cleared. + // Also, asfAllowTrustLineClawback cannot be cleared. { Env env(*this, features); Account alice{"alice"}; @@ -90,23 +90,23 @@ class Clawback_test : public beast::unit_test::suite env.fund(XRP(1000), alice); env.close(); - // set asfAllowClawback - env(fset(alice, asfAllowClawback)); + // set asfAllowTrustLineClawback + env(fset(alice, asfAllowTrustLineClawback)); env.close(); - env.require(flags(alice, asfAllowClawback)); + env.require(flags(alice, asfAllowTrustLineClawback)); - // clear asfAllowClawback does nothing - env(fclear(alice, asfAllowClawback)); + // clear asfAllowTrustLineClawback does nothing + env(fclear(alice, asfAllowTrustLineClawback)); env.close(); - env.require(flags(alice, asfAllowClawback)); + env.require(flags(alice, asfAllowTrustLineClawback)); - // asfNoFreeze cannot be set when asfAllowClawback is set + // asfNoFreeze cannot be set when asfAllowTrustLineClawback is set env.require(nflags(alice, asfNoFreeze)); env(fset(alice, asfNoFreeze), ter(tecNO_PERMISSION)); env.close(); } - // Test that asfAllowClawback cannot be set when + // Test that asfAllowTrustLineClawback cannot be set when // asfNoFreeze has been set { Env env(*this, features); @@ -124,14 +124,15 @@ class Clawback_test : public beast::unit_test::suite // NoFreeze is set env.require(flags(alice, asfNoFreeze)); - // asfAllowClawback cannot be set if asfNoFreeze is set - env(fset(alice, asfAllowClawback), ter(tecNO_PERMISSION)); + // asfAllowTrustLineClawback cannot be set if asfNoFreeze is set + env(fset(alice, asfAllowTrustLineClawback), ter(tecNO_PERMISSION)); env.close(); - env.require(nflags(alice, asfAllowClawback)); + env.require(nflags(alice, asfAllowTrustLineClawback)); } - // Test that asfAllowClawback is not allowed when owner dir is non-empty + // Test that asfAllowTrustLineClawback is not allowed when owner dir is + // non-empty { Env env(*this, features); @@ -142,7 +143,7 @@ class Clawback_test : public beast::unit_test::suite env.close(); auto const USD = alice["USD"]; - env.require(nflags(alice, asfAllowClawback)); + env.require(nflags(alice, asfAllowTrustLineClawback)); // alice issues 10 USD to bob env.trust(USD(1000), bob); @@ -153,7 +154,7 @@ class Clawback_test : public beast::unit_test::suite BEAST_EXPECT(ownerCount(env, bob) == 1); // alice fails to enable clawback because she has trustline with bob - env(fset(alice, asfAllowClawback), ter(tecOWNERS)); + env(fset(alice, asfAllowTrustLineClawback), ter(tecOWNERS)); env.close(); // bob sets trustline to default limit and pays alice back to delete @@ -164,16 +165,16 @@ class Clawback_test : public beast::unit_test::suite BEAST_EXPECT(ownerCount(env, alice) == 0); BEAST_EXPECT(ownerCount(env, bob) == 0); - // alice now is able to set asfAllowClawback - env(fset(alice, asfAllowClawback)); + // alice now is able to set asfAllowTrustLineClawback + env(fset(alice, asfAllowTrustLineClawback)); env.close(); - env.require(flags(alice, asfAllowClawback)); + env.require(flags(alice, asfAllowTrustLineClawback)); BEAST_EXPECT(ownerCount(env, alice) == 0); BEAST_EXPECT(ownerCount(env, bob) == 0); } - // Test that one cannot enable asfAllowClawback when + // Test that one cannot enable asfAllowTrustLineClawback when // featureClawback amendment is disabled { Env env(*this, features - featureClawback); @@ -183,22 +184,23 @@ class Clawback_test : public beast::unit_test::suite env.fund(XRP(1000), alice); env.close(); - env.require(nflags(alice, asfAllowClawback)); + env.require(nflags(alice, asfAllowTrustLineClawback)); - // alice attempts to set asfAllowClawback flag while amendment is - // disabled. no error is returned, but the flag remains to be unset. - env(fset(alice, asfAllowClawback)); + // 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, asfAllowClawback)); + env.require(nflags(alice, asfAllowTrustLineClawback)); // now enable clawback amendment env.enableFeature(featureClawback); env.close(); - // asfAllowClawback can be set - env(fset(alice, asfAllowClawback)); + // asfAllowTrustLineClawback can be set + env(fset(alice, asfAllowTrustLineClawback)); env.close(); - env.require(flags(alice, asfAllowClawback)); + env.require(flags(alice, asfAllowTrustLineClawback)); } } @@ -210,7 +212,7 @@ class Clawback_test : public beast::unit_test::suite // Test that Clawback tx fails for the following: // 1. when amendment is disabled - // 2. when asfAllowClawback flag has not been set + // 2. when asfAllowTrustLineClawback flag has not been set { Env env(*this, features - featureClawback); @@ -220,7 +222,7 @@ class Clawback_test : public beast::unit_test::suite env.fund(XRP(1000), alice, bob); env.close(); - env.require(nflags(alice, asfAllowClawback)); + env.require(nflags(alice, asfAllowTrustLineClawback)); auto const USD = alice["USD"]; @@ -240,7 +242,7 @@ class Clawback_test : public beast::unit_test::suite env.enableFeature(featureClawback); env.close(); - // clawback fails because asfAllowClawback has not been set + // clawback fails because asfAllowTrustLineClawback has not been set env(claw(alice, bob["USD"](5)), ter(tecNO_PERMISSION)); env.close(); @@ -265,10 +267,10 @@ class Clawback_test : public beast::unit_test::suite env.fund(XRP(1000), alice, bob); env.close(); - // alice sets asfAllowClawback - env(fset(alice, asfAllowClawback)); + // alice sets asfAllowTrustLineClawback + env(fset(alice, asfAllowTrustLineClawback)); env.close(); - env.require(flags(alice, asfAllowClawback)); + env.require(flags(alice, asfAllowTrustLineClawback)); auto const USD = alice["USD"]; @@ -350,10 +352,10 @@ class Clawback_test : public beast::unit_test::suite env.fund(XRP(1000), alice); env.close(); - // alice sets asfAllowClawback - env(fset(alice, asfAllowClawback)); + // alice sets asfAllowTrustLineClawback + env(fset(alice, asfAllowTrustLineClawback)); env.close(); - env.require(flags(alice, asfAllowClawback)); + env.require(flags(alice, asfAllowTrustLineClawback)); // bob, the token holder, does not exist env(claw(alice, bob["USD"](5)), ter(terNO_ACCOUNT)); @@ -374,15 +376,15 @@ class Clawback_test : public beast::unit_test::suite auto const USD = alice["USD"]; - // alice sets asfAllowClawback - env(fset(alice, asfAllowClawback)); + // alice sets asfAllowTrustLineClawback + env(fset(alice, asfAllowTrustLineClawback)); env.close(); - env.require(flags(alice, asfAllowClawback)); + env.require(flags(alice, asfAllowTrustLineClawback)); - // cindy sets asfAllowClawback - env(fset(cindy, asfAllowClawback)); + // cindy sets asfAllowTrustLineClawback + env(fset(cindy, asfAllowTrustLineClawback)); env.close(); - env.require(flags(cindy, asfAllowClawback)); + env.require(flags(cindy, asfAllowTrustLineClawback)); // alice issues 1000 USD to bob env.trust(USD(1000), bob); @@ -417,15 +419,15 @@ class Clawback_test : public beast::unit_test::suite auto const USD = alice["USD"]; auto const CAD = bob["CAD"]; - // alice sets asfAllowClawback - env(fset(alice, asfAllowClawback)); + // alice sets asfAllowTrustLineClawback + env(fset(alice, asfAllowTrustLineClawback)); env.close(); - env.require(flags(alice, asfAllowClawback)); + env.require(flags(alice, asfAllowTrustLineClawback)); - // bob sets asfAllowClawback - env(fset(bob, asfAllowClawback)); + // bob sets asfAllowTrustLineClawback + env(fset(bob, asfAllowTrustLineClawback)); env.close(); - env.require(flags(bob, asfAllowClawback)); + env.require(flags(bob, asfAllowTrustLineClawback)); // alice issues 10 USD to bob. // bob then attempts to submit a clawback tx to claw USD from alice. @@ -486,10 +488,10 @@ class Clawback_test : public beast::unit_test::suite auto const USD = alice["USD"]; - // alice sets asfAllowClawback - env(fset(alice, asfAllowClawback)); + // alice sets asfAllowTrustLineClawback + env(fset(alice, asfAllowTrustLineClawback)); env.close(); - env.require(flags(alice, asfAllowClawback)); + env.require(flags(alice, asfAllowTrustLineClawback)); // alice issues 1000 USD to bob env.trust(USD(1000), bob); @@ -536,15 +538,15 @@ class Clawback_test : public beast::unit_test::suite env.fund(XRP(1000), alice, bob, cindy); env.close(); - // alice sets asfAllowClawback - env(fset(alice, asfAllowClawback)); + // alice sets asfAllowTrustLineClawback + env(fset(alice, asfAllowTrustLineClawback)); env.close(); - env.require(flags(alice, asfAllowClawback)); + env.require(flags(alice, asfAllowTrustLineClawback)); - // bob sets asfAllowClawback - env(fset(bob, asfAllowClawback)); + // bob sets asfAllowTrustLineClawback + env(fset(bob, asfAllowTrustLineClawback)); env.close(); - env.require(flags(bob, asfAllowClawback)); + env.require(flags(bob, asfAllowTrustLineClawback)); // alice sends 1000 USD to cindy env.trust(alice["USD"](1000), cindy); @@ -596,10 +598,10 @@ class Clawback_test : public beast::unit_test::suite auto const USD = alice["USD"]; - // alice sets asfAllowClawback - env(fset(alice, asfAllowClawback)); + // alice sets asfAllowTrustLineClawback + env(fset(alice, asfAllowTrustLineClawback)); env.close(); - env.require(flags(alice, asfAllowClawback)); + env.require(flags(alice, asfAllowTrustLineClawback)); // alice sends 600 USD to bob env.trust(USD(1000), bob); @@ -663,15 +665,15 @@ class Clawback_test : public beast::unit_test::suite env.fund(XRP(1000), alice, bob); env.close(); - // alice sets asfAllowClawback - env(fset(alice, asfAllowClawback)); + // alice sets asfAllowTrustLineClawback + env(fset(alice, asfAllowTrustLineClawback)); env.close(); - env.require(flags(alice, asfAllowClawback)); + env.require(flags(alice, asfAllowTrustLineClawback)); - // bob sets asfAllowClawback - env(fset(bob, asfAllowClawback)); + // bob sets asfAllowTrustLineClawback + env(fset(bob, asfAllowTrustLineClawback)); env.close(); - env.require(flags(bob, asfAllowClawback)); + env.require(flags(bob, asfAllowTrustLineClawback)); // alice issues 1000 USD to bob env.trust(alice["USD"](1000), bob); @@ -752,10 +754,10 @@ class Clawback_test : public beast::unit_test::suite auto const USD = alice["USD"]; - // alice sets asfAllowClawback - env(fset(alice, asfAllowClawback)); + // alice sets asfAllowTrustLineClawback + env(fset(alice, asfAllowTrustLineClawback)); env.close(); - env.require(flags(alice, asfAllowClawback)); + env.require(flags(alice, asfAllowTrustLineClawback)); // alice issues 1000 USD to bob env.trust(USD(1000), bob); @@ -802,10 +804,10 @@ class Clawback_test : public beast::unit_test::suite auto const USD = alice["USD"]; - // alice sets asfAllowClawback - env(fset(alice, asfAllowClawback)); + // alice sets asfAllowTrustLineClawback + env(fset(alice, asfAllowTrustLineClawback)); env.close(); - env.require(flags(alice, asfAllowClawback)); + env.require(flags(alice, asfAllowTrustLineClawback)); // alice issues 1000 USD to bob env.trust(USD(1000), bob); @@ -848,10 +850,10 @@ class Clawback_test : public beast::unit_test::suite auto const USD = alice["USD"]; - // alice sets asfAllowClawback - env(fset(alice, asfAllowClawback)); + // alice sets asfAllowTrustLineClawback + env(fset(alice, asfAllowTrustLineClawback)); env.close(); - env.require(flags(alice, asfAllowClawback)); + env.require(flags(alice, asfAllowTrustLineClawback)); // alice issues 1000 USD to bob env.trust(USD(1000), bob); @@ -900,10 +902,10 @@ class Clawback_test : public beast::unit_test::suite auto const USD = alice["USD"]; - // alice sets asfAllowClawback - env(fset(alice, asfAllowClawback)); + // alice sets asfAllowTrustLineClawback + env(fset(alice, asfAllowTrustLineClawback)); env.close(); - env.require(flags(alice, asfAllowClawback)); + env.require(flags(alice, asfAllowTrustLineClawback)); // alice issues 100 USD to bob env.trust(USD(1000), bob); @@ -944,7 +946,7 @@ class Clawback_test : public beast::unit_test::suite void testWithFeats(FeatureBitset features) { - testAllowClawbackFlag(features); + testAllowTrustLineClawbackFlag(features); testValidation(features); testPermission(features); testEnabled(features); diff --git a/src/test/jtx/flags.h b/src/test/jtx/flags.h index a6f4345cf..27b6ea956 100644 --- a/src/test/jtx/flags.h +++ b/src/test/jtx/flags.h @@ -80,8 +80,8 @@ private: case asfDepositAuth: mask_ |= lsfDepositAuth; break; - case asfAllowClawback: - mask_ |= lsfAllowClawback; + case asfAllowTrustLineClawback: + mask_ |= lsfAllowTrustLineClawback; break; default: Throw("unknown flag"); diff --git a/src/test/rpc/AccountInfo_test.cpp b/src/test/rpc/AccountInfo_test.cpp index 976290ab2..331611cf2 100644 --- a/src/test/rpc/AccountInfo_test.cpp +++ b/src/test/rpc/AccountInfo_test.cpp @@ -611,25 +611,29 @@ public: } static constexpr std::pair - allowClawbackFlag{"allowClawback", asfAllowClawback}; + allowTrustLineClawbackFlag{ + "allowTrustLineClawback", asfAllowTrustLineClawback}; if (features[featureClawback]) { // must use bob's account because alice has noFreeze set - auto const f1 = getAccountFlag(allowClawbackFlag.first, bob); + auto const f1 = + getAccountFlag(allowTrustLineClawbackFlag.first, bob); BEAST_EXPECT(f1.has_value()); BEAST_EXPECT(!f1.value()); - // Set allowClawback - env(fset(bob, allowClawbackFlag.second)); + // Set allowTrustLineClawback + env(fset(bob, allowTrustLineClawbackFlag.second)); env.close(); - auto const f2 = getAccountFlag(allowClawbackFlag.first, bob); + auto const f2 = + getAccountFlag(allowTrustLineClawbackFlag.first, bob); BEAST_EXPECT(f2.has_value()); BEAST_EXPECT(f2.value()); } else { - BEAST_EXPECT(!getAccountFlag(allowClawbackFlag.first, bob)); + BEAST_EXPECT( + !getAccountFlag(allowTrustLineClawbackFlag.first, bob)); } } diff --git a/src/test/rpc/AccountSet_test.cpp b/src/test/rpc/AccountSet_test.cpp index 02240b213..953943445 100644 --- a/src/test/rpc/AccountSet_test.cpp +++ b/src/test/rpc/AccountSet_test.cpp @@ -94,10 +94,10 @@ public: // and are tested elsewhere continue; } - if (flag == asfAllowClawback) + if (flag == asfAllowTrustLineClawback) { - // The asfAllowClawback flag can't be cleared. It is tested - // elsewhere. + // The asfAllowTrustLineClawback flag can't be cleared. It + // is tested elsewhere. continue; }