Merge branch 'develop' into Bronek/maximum_feature_name_size

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

include/xrpl/protocol/Feature.h

@@ -64,6 +64,48 @@
 
 namespace xrpl {
 
+// Feature names must not exceed this length (in characters, excluding the null terminator).
+static constexpr std::size_t maxFeatureNameSize = 63;
+// Reserve this exact feature-name length (in characters/bytes, excluding the null terminator)
+// so that a 32-byte uint256 (for example, in WASM or other interop contexts) can be used
+// as a compact, fixed-size feature selector without conflicting with human-readable names.
+static constexpr std::size_t reservedFeatureNameSize = 32;
+
+// Both validFeatureNameSize and validFeatureName are consteval functions that can be used in
+// static_asserts to validate feature names at compile time. They are only used inside
+// enforceValidFeatureName in Feature.cpp, but are exposed here for testing. The expected
+// parameter `auto fn` is a constexpr lambda which returns a const char*, making it available
+// for compile-time evaluation. Read more in https://accu.org/journals/overload/30/172/wu/
+consteval auto
+validFeatureNameSize(auto fn) -> bool
+{
+    constexpr char const* n = fn();
+    // Note, std::strlen is not constexpr, we need to implement our own here.
+    constexpr std::size_t N = [](auto n) {
+        std::size_t ret = 0;
+        for (auto ptr = n; *ptr != '\0'; ret++, ++ptr)
+            ;
+        return ret;
+    }(n);
+    return N != reservedFeatureNameSize &&   //
+        N != reservedFeatureNameSize + 1 &&  //
+        N <= maxFeatureNameSize;
+}
+
+consteval auto
+validFeatureName(auto fn) -> bool
+{
+    constexpr char const* n = fn();
+    // Prevent the use of visually confusable characters and enforce that feature names
+    // are always valid ASCII. This is needed because C++ allows Unicode identifiers.
+    for (auto ptr = n; *ptr != '\0'; ++ptr)
+    {
+        if (*ptr & 0x80 || *ptr < 0x20)
+            return false;
+    }
+    return true;
+}
+
 enum class VoteBehavior : int { Obsolete = -1, DefaultNo = 0, DefaultYes };
 enum class AmendmentSupport : int { Retired = -1, Supported = 0, Unsupported };
 

src/libxrpl/protocol/Feature.cpp

@@ -395,10 +395,20 @@ featureToName(uint256 const& f)
 #pragma push_macro("XRPL_RETIRE_FIX")
 #undef XRPL_RETIRE_FIX
 
+consteval auto
+enforceValidFeatureName(auto fn) -> char const*
+{
+    static_assert(validFeatureName(fn), "Invalid feature name");
+    static_assert(validFeatureNameSize(fn), "Invalid feature name size");
+    return fn();
+}
+
 #define XRPL_FEATURE(name, supported, vote) \
-    uint256 const feature##name = registerFeature(#name, supported, vote);
+    uint256 const feature##name =           \
+        registerFeature(enforceValidFeatureName([] { return #name; }), supported, vote);
 #define XRPL_FIX(name, supported, vote) \
-    uint256 const fix##name = registerFeature("fix" #name, supported, vote);
+    uint256 const fix##name =           \
+        registerFeature(enforceValidFeatureName([] { return "fix" #name; }), supported, vote);
 
 // clang-format off
 #define XRPL_RETIRE_FEATURE(name)                                       \

src/test/rpc/Feature_test.cpp

@@ -2,6 +2,7 @@
 
 #include <xrpl/ledger/AmendmentTable.h>
 #include <xrpl/protocol/Feature.h>
+#include <xrpl/protocol/digest.h>
 #include <xrpl/protocol/jss.h>
 
 namespace xrpl {
@@ -168,16 +169,18 @@ class Feature_test : public beast::unit_test::suite
         using namespace test::jtx;
         Env env{*this};
 
-        auto jrr = env.rpc("feature", "fixAMMOverflowOffer")[jss::result];
+        std::string const name = "fixAMMOverflowOffer";
+        auto jrr = env.rpc("feature", name)[jss::result];
         BEAST_EXPECTS(jrr[jss::status] == jss::success, "status");
         jrr.removeMember(jss::status);
         BEAST_EXPECT(jrr.size() == 1);
-        BEAST_EXPECT(jrr.isMember(
-            "12523DF04B553A0B1AD74F42DDB741DE8DC06A03FC089A0EF197E"
-            "2A87F1D8107"));
+        auto const expected = to_string(sha512Half(Slice(name.data(), name.size())));
+        char const sha[] = "12523DF04B553A0B1AD74F42DDB741DE8DC06A03FC089A0EF197E2A87F1D8107";
+        BEAST_EXPECT(expected == sha);
+        BEAST_EXPECT(jrr.isMember(expected));
         auto feature = *(jrr.begin());
 
-        BEAST_EXPECTS(feature[jss::name] == "fixAMMOverflowOffer", "name");
+        BEAST_EXPECTS(feature[jss::name] == name, "name");
         BEAST_EXPECTS(!feature[jss::enabled].asBool(), "enabled");
         BEAST_EXPECTS(feature[jss::vetoed].isBool() && !feature[jss::vetoed].asBool(), "vetoed");
         BEAST_EXPECTS(feature[jss::supported].asBool(), "supported");
@@ -186,6 +189,39 @@ class Feature_test : public beast::unit_test::suite
         jrr = env.rpc("feature", "fMM")[jss::result];
         BEAST_EXPECT(jrr[jss::error] == "badFeature");
         BEAST_EXPECT(jrr[jss::error_message] == "Feature unknown or invalid.");
+
+        // Test feature name size checks
+        constexpr auto ok63Name = [] {
+            return "123456789012345678901234567890123456789012345678901234567890123";
+        };
+        static_assert(validFeatureNameSize(ok63Name));
+
+        constexpr auto bad64Name = [] {
+            return "1234567890123456789012345678901234567890123456789012345678901234";
+        };
+        static_assert(!validFeatureNameSize(bad64Name));
+
+        constexpr auto ok31Name = [] { return "1234567890123456789012345678901"; };
+        static_assert(validFeatureNameSize(ok31Name));
+
+        constexpr auto bad32Name = [] { return "12345678901234567890123456789012"; };
+        static_assert(!validFeatureNameSize(bad32Name));
+        constexpr auto bad33Name = [] { return "123456789012345678901234567890123"; };
+        static_assert(!validFeatureNameSize(bad33Name));
+
+        constexpr auto ok34Name = [] { return "1234567890123456789012345678901234"; };
+        static_assert(validFeatureNameSize(ok34Name));
+
+        // Test feature character set checks
+        constexpr auto okName = [] { return "AMM_123"; };
+        static_assert(validFeatureName(okName));
+
+        // First character is Greek Capital Alpha, visually confusable with ASCII 'A'
+        constexpr auto badName = [] { return "ΑMM_123"; };
+        static_assert(!validFeatureName(badName));
+
+        constexpr auto badEmoji = [] { return "🔥"; };
+        static_assert(!validFeatureName(badEmoji));
     }
 
     void