fix: Amendment to add transaction flag checking functionality for Credentials (#5250)

CredentialCreate / CredentialAccept / CredentialDelete transactions will check sfFlags field in preflight() when the amendment is enabled.

include/xrpl/protocol/Feature.h

@@ -80,7 +80,7 @@ namespace detail {
 // Feature.cpp. Because it's only used to reserve storage, and determine how
 // large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
 // the actual number of amendments. A LogicError on startup will verify this.
-static constexpr std::size_t numFeatures = 87;
+static constexpr std::size_t numFeatures = 88;
 
 /** Amendments that this server supports and the default voting behavior.
    Whether they are enabled depends on the Rules defined in the validated

include/xrpl/protocol/detail/features.macro

@@ -29,6 +29,8 @@
 // If you add an amendment here, then do not forget to increment `numFeatures`
 // in include/xrpl/protocol/Feature.h.
 
+// Check flags in Credential transactions
+XRPL_FIX    (InvalidTxFlags,             Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FIX    (FrozenLPTokenTransfer,      Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(DeepFreeze,                 Supported::yes, VoteBehavior::DefaultNo)
 XRPL_FEATURE(PermissionedDomains,        Supported::no,  VoteBehavior::DefaultNo)

src/test/app/Credentials_test.cpp

@@ -1058,6 +1058,43 @@ struct Credentials_test : public beast::unit_test::suite
         }
     }
 
+    void
+    testFlags(FeatureBitset features)
+    {
+        using namespace test::jtx;
+
+        bool const enabled = features[fixInvalidTxFlags];
+        testcase(
+            std::string("Test flag, fix ") +
+            (enabled ? "enabled" : "disabled"));
+
+        const char credType[] = "abcde";
+        Account const issuer{"issuer"};
+        Account const subject{"subject"};
+
+        {
+            using namespace jtx;
+            Env env{*this, features};
+
+            env.fund(XRP(5000), subject, issuer);
+            env.close();
+
+            {
+                ter const expected(
+                    enabled ? TER(temINVALID_FLAG) : TER(tesSUCCESS));
+                env(credentials::create(subject, issuer, credType),
+                    txflags(tfTransferable),
+                    expected);
+                env(credentials::accept(subject, issuer, credType),
+                    txflags(tfSellNFToken),
+                    expected);
+                env(credentials::deleteCred(subject, subject, issuer, credType),
+                    txflags(tfPassive),
+                    expected);
+            }
+        }
+    }
+
     void
     run() override
     {
@@ -1069,6 +1106,8 @@ struct Credentials_test : public beast::unit_test::suite
         testAcceptFailed(all);
         testDeleteFailed(all);
         testFeatureFailed(all - featureCredentials);
+        testFlags(all - fixInvalidTxFlags);
+        testFlags(all);
         testRPC();
     }
 };

src/test/app/MultiSign_test.cpp

@@ -1672,6 +1672,29 @@ public:
         BEAST_EXPECT(env.seq(alice) == aliceSeq + 1);
     }
 
+    void
+    test_signerListSetFlags(FeatureBitset features)
+    {
+        using namespace test::jtx;
+
+        Env env{*this, features};
+        Account const alice{"alice"};
+
+        env.fund(XRP(1000), alice);
+        env.close();
+
+        bool const enabled = features[fixInvalidTxFlags];
+        testcase(
+            std::string("SignerListSet flag, fix ") +
+            (enabled ? "enabled" : "disabled"));
+
+        ter const expected(enabled ? TER(temINVALID_FLAG) : TER(tesSUCCESS));
+        env(signers(alice, 2, {{bogie, 1}, {ghost, 1}}),
+            expected,
+            txflags(tfPassive));
+        env.close();
+    }
+
     void
     testAll(FeatureBitset features)
     {
@@ -1708,6 +1731,10 @@ public:
         testAll(all - featureMultiSignReserve - featureExpandedSignerList);
         testAll(all - featureExpandedSignerList);
         testAll(all);
+
+        test_signerListSetFlags(all - fixInvalidTxFlags);
+        test_signerListSetFlags(all);
+
         test_amendmentTransition();
     }
 };

src/xrpld/app/tx/detail/Credentials.cpp

@@ -65,6 +65,13 @@ CredentialCreate::preflight(PreflightContext const& ctx)
     auto const& tx = ctx.tx;
     auto& j = ctx.j;
 
+    if (ctx.rules.enabled(fixInvalidTxFlags) &&
+        (tx.getFlags() & tfUniversalMask))
+    {
+        JLOG(ctx.j.debug()) << "CredentialCreate: invalid flags.";
+        return temINVALID_FLAG;
+    }
+
     if (!tx[sfSubject])
     {
         JLOG(j.trace()) << "Malformed transaction: Invalid Subject";
@@ -209,6 +216,13 @@ CredentialDelete::preflight(PreflightContext const& ctx)
     if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
         return ret;
 
+    if (ctx.rules.enabled(fixInvalidTxFlags) &&
+        (ctx.tx.getFlags() & tfUniversalMask))
+    {
+        JLOG(ctx.j.debug()) << "CredentialDelete: invalid flags.";
+        return temINVALID_FLAG;
+    }
+
     auto const subject = ctx.tx[~sfSubject];
     auto const issuer = ctx.tx[~sfIssuer];
 
@@ -289,6 +303,13 @@ CredentialAccept::preflight(PreflightContext const& ctx)
     if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
         return ret;
 
+    if (ctx.rules.enabled(fixInvalidTxFlags) &&
+        (ctx.tx.getFlags() & tfUniversalMask))
+    {
+        JLOG(ctx.j.debug()) << "CredentialAccept: invalid flags.";
+        return temINVALID_FLAG;
+    }
+
     if (!ctx.tx[sfIssuer])
     {
         JLOG(ctx.j.trace()) << "Malformed transaction: Issuer field zeroed.";

src/xrpld/app/tx/detail/SetSignerList.cpp

@@ -27,6 +27,8 @@
 #include <xrpl/protocol/STArray.h>
 #include <xrpl/protocol/STObject.h>
 #include <xrpl/protocol/STTx.h>
+#include <xrpl/protocol/TxFlags.h>
+
 #include <algorithm>
 #include <cstdint>
 
@@ -81,6 +83,13 @@ SetSignerList::preflight(PreflightContext const& ctx)
     if (auto const ret = preflight1(ctx); !isTesSuccess(ret))
         return ret;
 
+    if (ctx.rules.enabled(fixInvalidTxFlags) &&
+        (ctx.tx.getFlags() & tfUniversalMask))
+    {
+        JLOG(ctx.j.debug()) << "SetSignerList: invalid flags.";
+        return temINVALID_FLAG;
+    }
+
     auto const result = determineOperation(ctx.tx, ctx.flags, ctx.j);
 
     if (std::get<0>(result) != tesSUCCESS)