diff --git a/include/xrpl/protocol/Feature.h b/include/xrpl/protocol/Feature.h index c52f312cbf..1c476df617 100644 --- a/include/xrpl/protocol/Feature.h +++ b/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 diff --git a/include/xrpl/protocol/detail/features.macro b/include/xrpl/protocol/detail/features.macro index f82a05a7c1..aa0782b137 100644 --- a/include/xrpl/protocol/detail/features.macro +++ b/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) diff --git a/src/test/app/Credentials_test.cpp b/src/test/app/Credentials_test.cpp index e5d90d9766..481850562f 100644 --- a/src/test/app/Credentials_test.cpp +++ b/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(); } }; diff --git a/src/test/app/MultiSign_test.cpp b/src/test/app/MultiSign_test.cpp index 77d85d9011..9648bed886 100644 --- a/src/test/app/MultiSign_test.cpp +++ b/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(); } }; diff --git a/src/xrpld/app/tx/detail/Credentials.cpp b/src/xrpld/app/tx/detail/Credentials.cpp index 4da875f8d7..ca80bc159e 100644 --- a/src/xrpld/app/tx/detail/Credentials.cpp +++ b/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."; diff --git a/src/xrpld/app/tx/detail/SetSignerList.cpp b/src/xrpld/app/tx/detail/SetSignerList.cpp index a74b0f7351..173107e02a 100644 --- a/src/xrpld/app/tx/detail/SetSignerList.cpp +++ b/src/xrpld/app/tx/detail/SetSignerList.cpp @@ -27,6 +27,8 @@ #include #include #include +#include + #include #include @@ -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)