feat: add user version of feature RPC (#4781)

* uses same formatting as admin RPC
* hides potentially sensitive data

API-CHANGELOG.md

@@ -28,6 +28,12 @@ The `network_id` field was added in the `server_info` response in version 1.5.0
 
 ## XRP Ledger server version 2.0.0
 
+### Additions in 2.2
+
+Additions are intended to be non-breaking (because they are purely additive).
+
+- `feature`: A non-admin mode that uses the same formatting as admin RPC, but hides potentially-sensitive data.
+
 ### Additions in 2.0
 
 Additions are intended to be non-breaking (because they are purely additive).
@@ -36,7 +42,6 @@ Additions are intended to be non-breaking (because they are purely additive).
 - In `Payment` transactions, `DeliverMax` has been added. This is a replacement for the `Amount` field, which should not be used. Typically, the `delivered_amount` (in transaction metadata) should be used. To ease the transition, `DeliverMax` is present regardless of API version, since adding a field is non-breaking.
 - API version 2 has been moved from beta to supported, meaning that it is generally available (regardless of the `beta_rpc_api` setting).
 
-
 ## XRP Ledger server version 1.12.0
 
 [Version 1.12.0](https://github.com/XRPLF/rippled/releases/tag/1.12.0) was released on Sep 6, 2023.

src/ripple/app/misc/AmendmentTable.h

@@ -81,11 +81,11 @@ class AmendmentTable
     firstUnsupportedExpected() const = 0;
 
     virtual Json::Value
-    getJson() const = 0;
+    getJson(bool isAdmin) const = 0;
 
     /** Returns a Json::objectValue. */
     virtual Json::Value
-    getJson(uint256 const& amendment) const = 0;
+    getJson(uint256 const& amendment, bool isAdmin) const = 0;
 
     /** Called when a new fully-validated ledger is accepted. */
     void

src/ripple/app/misc/impl/AmendmentTable.cpp

@@ -388,6 +388,7 @@ class AmendmentTableImpl final : public AmendmentTable
         Json::Value& v,
         uint256 const& amendment,
         AmendmentState const& state,
+        bool isAdmin,
         std::lock_guard<std::mutex> const& lock) const;
 
     void
@@ -428,9 +429,9 @@ class AmendmentTableImpl final : public AmendmentTable
     firstUnsupportedExpected() const override;
 
     Json::Value
-    getJson() const override;
+    getJson(bool isAdmin) const override;
     Json::Value
-    getJson(uint256 const&) const override;
+    getJson(uint256 const&, bool isAdmin) const override;
 
     bool
     needValidatedLedger(LedgerIndex seq) const override;
@@ -906,13 +907,14 @@ AmendmentTableImpl::injectJson(
     Json::Value& v,
     const uint256& id,
     const AmendmentState& fs,
+    bool isAdmin,
     std::lock_guard<std::mutex> const&) const
 {
     if (!fs.name.empty())
         v[jss::name] = fs.name;
 
     v[jss::supported] = fs.supported;
-    if (!fs.enabled)
+    if (!fs.enabled && isAdmin)
     {
         if (fs.vote == AmendmentVote::obsolete)
             v[jss::vetoed] = "Obsolete";
@@ -921,7 +923,7 @@ AmendmentTableImpl::injectJson(
     }
     v[jss::enabled] = fs.enabled;
 
-    if (!fs.enabled && lastVote_)
+    if (!fs.enabled && lastVote_ && isAdmin)
     {
         auto const votesTotal = lastVote_->trustedValidations();
         auto const votesNeeded = lastVote_->threshold();
@@ -936,7 +938,7 @@ AmendmentTableImpl::injectJson(
 }
 
 Json::Value
-AmendmentTableImpl::getJson() const
+AmendmentTableImpl::getJson(bool isAdmin) const
 {
     Json::Value ret(Json::objectValue);
     {
@@ -947,23 +949,27 @@ AmendmentTableImpl::getJson() const
                 ret[to_string(e.first)] = Json::objectValue,
                 e.first,
                 e.second,
+                isAdmin,
                 lock);
         }
     }
     return ret;
 }
 
 Json::Value
-AmendmentTableImpl::getJson(uint256 const& amendmentID) const
+AmendmentTableImpl::getJson(uint256 const& amendmentID, bool isAdmin) const
 {
     Json::Value ret = Json::objectValue;
-    Json::Value& jAmendment = (ret[to_string(amendmentID)] = Json::objectValue);
 
     {
         std::lock_guard lock(mutex_);
         AmendmentState const* a = get(amendmentID, lock);
         if (a)
-            injectJson(jAmendment, amendmentID, *a, lock);
+        {
+            Json::Value& jAmendment =
+                (ret[to_string(amendmentID)] = Json::objectValue);
+            injectJson(jAmendment, amendmentID, *a, isAdmin, lock);
+        }
     }
 
     return ret;

src/ripple/rpc/handlers/Feature1.cpp

@@ -22,6 +22,7 @@
 #include <ripple/app/misc/AmendmentTable.h>
 #include <ripple/net/RPCErr.h>
 #include <ripple/protocol/ErrorCodes.h>
+#include <ripple/protocol/Feature.h>
 #include <ripple/protocol/jss.h>
 #include <ripple/rpc/Context.h>
 
@@ -37,6 +38,7 @@ doFeature(RPC::JsonContext& context)
     if (context.app.config().reporting())
         return rpcError(rpcREPORTING_UNSUPPORTED);
 
+    bool const isAdmin = context.role == Role::ADMIN;
     // Get majority amendment status
     majorityAmendments_t majorities;
 
@@ -47,7 +49,7 @@ doFeature(RPC::JsonContext& context)
 
     if (!context.params.isMember(jss::feature))
     {
-        auto features = table.getJson();
+        auto features = table.getJson(isAdmin);
 
         for (auto const& [h, t] : majorities)
         {
@@ -69,13 +71,18 @@ doFeature(RPC::JsonContext& context)
 
     if (context.params.isMember(jss::vetoed))
     {
+        if (!isAdmin)
+            return rpcError(rpcNO_PERMISSION);
+
         if (context.params[jss::vetoed].asBool())
             table.veto(feature);
         else
             table.unVeto(feature);
     }
 
-    Json::Value jvReply = table.getJson(feature);
+    Json::Value jvReply = table.getJson(feature, isAdmin);
+    if (!jvReply)
+        return rpcError(rpcBAD_FEATURE);
 
     auto m = majorities.find(feature);
     if (m != majorities.end())

src/ripple/rpc/impl/Handler.cpp

@@ -105,6 +105,9 @@ Handler const handlerArray[]{
      Role::USER,
      NO_CONDITION},
     {"download_shard", byRef(&doDownloadShard), Role::ADMIN, NO_CONDITION},
+    {"feature", byRef(&doFeature), Role::USER, NO_CONDITION},
+    {"fee", byRef(&doFee), Role::USER, NEEDS_CURRENT_LEDGER},
+    {"fetch_info", byRef(&doFetchInfo), Role::ADMIN, NO_CONDITION},
 #ifdef RIPPLED_REPORTING
     {"gateway_balances", byRef(&doGatewayBalances), Role::ADMIN, NO_CONDITION},
 #else
@@ -115,9 +118,6 @@ Handler const handlerArray[]{
      byRef(&doGetAggregatePrice),
      Role::USER,
      NO_CONDITION},
-    {"feature", byRef(&doFeature), Role::ADMIN, NO_CONDITION},
-    {"fee", byRef(&doFee), Role::USER, NEEDS_CURRENT_LEDGER},
-    {"fetch_info", byRef(&doFetchInfo), Role::ADMIN, NO_CONDITION},
     {"ledger_accept",
      byRef(&doLedgerAccept),
      Role::ADMIN,

src/test/app/AmendmentTable_test.cpp

@@ -288,15 +288,15 @@ class AmendmentTable_test final : public beast::unit_test::suite
         uint256 const unsupportedID = amendmentId(unsupported_[0]);
         {
             Json::Value const unsupp =
-                table->getJson(unsupportedID)[to_string(unsupportedID)];
+                table->getJson(unsupportedID, true)[to_string(unsupportedID)];
             BEAST_EXPECT(unsupp.size() == 0);
         }
 
         // After vetoing unsupportedID verify that it is in table.
         table->veto(unsupportedID);
         {
             Json::Value const unsupp =
-                table->getJson(unsupportedID)[to_string(unsupportedID)];
+                table->getJson(unsupportedID, true)[to_string(unsupportedID)];
             BEAST_EXPECT(unsupp[jss::vetoed].asBool());
         }
     }
@@ -638,6 +638,22 @@ class AmendmentTable_test final : public beast::unit_test::suite
         BEAST_EXPECT(enabled.empty());
         BEAST_EXPECT(majority.empty());
 
+        uint256 const unsupportedID = amendmentId(unsupported_[0]);
+        {
+            Json::Value const unsupp =
+                table->getJson(unsupportedID, false)[to_string(unsupportedID)];
+            BEAST_EXPECT(unsupp.size() == 0);
+        }
+
+        table->veto(unsupportedID);
+        {
+            Json::Value const unsupp =
+                table->getJson(unsupportedID, false)[to_string(unsupportedID)];
+            BEAST_EXPECT(!unsupp[jss::vetoed].asBool());
+        }
+
+        votes.emplace_back(testAmendment, validators.size());
+
         votes.emplace_back(testAmendment, validators.size());
 
         doRound(

src/test/rpc/Feature_test.cpp

@@ -205,11 +205,94 @@ class Feature_test : public beast::unit_test::suite
                     return cfg;
                 })};
 
-        auto jrr = env.rpc("feature")[jss::result];
-        // The current HTTP/S ServerHandler returns an HTTP 403 error code here
-        // rather than a noPermission JSON error. The JSONRPCClient just eats
-        // that error and returns an null result.
-        BEAST_EXPECT(jrr.isNull());
+        {
+            auto result = env.rpc("feature")[jss::result];
+            BEAST_EXPECT(result.isMember(jss::features));
+            // There should be at least 50 amendments.  Don't do exact
+            // comparison to avoid maintenance as more amendments are added in
+            // the future.
+            BEAST_EXPECT(result[jss::features].size() >= 50);
+            for (auto it = result[jss::features].begin();
+                 it != result[jss::features].end();
+                 ++it)
+            {
+                uint256 id;
+                (void)id.parseHex(it.key().asString().c_str());
+                if (!BEAST_EXPECT((*it).isMember(jss::name)))
+                    return;
+                bool expectEnabled =
+                    env.app().getAmendmentTable().isEnabled(id);
+                bool expectSupported =
+                    env.app().getAmendmentTable().isSupported(id);
+                BEAST_EXPECTS(
+                    (*it).isMember(jss::enabled) &&
+                        (*it)[jss::enabled].asBool() == expectEnabled,
+                    (*it)[jss::name].asString() + " enabled");
+                BEAST_EXPECTS(
+                    (*it).isMember(jss::supported) &&
+                        (*it)[jss::supported].asBool() == expectSupported,
+                    (*it)[jss::name].asString() + " supported");
+                BEAST_EXPECT(!(*it).isMember(jss::vetoed));
+                BEAST_EXPECT(!(*it).isMember(jss::majority));
+                BEAST_EXPECT(!(*it).isMember(jss::count));
+                BEAST_EXPECT(!(*it).isMember(jss::validations));
+                BEAST_EXPECT(!(*it).isMember(jss::threshold));
+            }
+        }
+
+        {
+            Json::Value params;
+            // invalid feature
+            params[jss::feature] =
+                "1234567890ABCDEF1234567890ABCDEF1234567890ABCDEF1234567890ABCD"
+                "EF";
+            auto const result = env.rpc(
+                "json",
+                "feature",
+                boost::lexical_cast<std::string>(params))[jss::result];
+            BEAST_EXPECTS(
+                result[jss::error] == "badFeature", result.toStyledString());
+            BEAST_EXPECT(
+                result[jss::error_message] == "Feature unknown or invalid.");
+        }
+
+        {
+            Json::Value params;
+            params[jss::feature] =
+                "93E516234E35E08CA689FA33A6D38E103881F8DCB53023F728C307AA89D515"
+                "A7";
+            // invalid param
+            params[jss::vetoed] = true;
+            auto const result = env.rpc(
+                "json",
+                "feature",
+                boost::lexical_cast<std::string>(params))[jss::result];
+            BEAST_EXPECTS(
+                result[jss::error] == "noPermission",
+                result[jss::error].asString());
+            BEAST_EXPECT(
+                result[jss::error_message] ==
+                "You don't have permission for this command.");
+        }
+
+        {
+            std::string const feature =
+                "C4483A1896170C66C098DEA5B0E024309C60DC960DE5F01CD7AF986AA3D9AD"
+                "37";
+            Json::Value params;
+            params[jss::feature] = feature;
+            auto const result = env.rpc(
+                "json",
+                "feature",
+                boost::lexical_cast<std::string>(params))[jss::result];
+            BEAST_EXPECT(result.isMember(feature));
+            auto const amendmentResult = result[feature];
+            BEAST_EXPECT(amendmentResult[jss::enabled].asBool() == false);
+            BEAST_EXPECT(amendmentResult[jss::supported].asBool() == true);
+            BEAST_EXPECT(
+                amendmentResult[jss::name].asString() ==
+                "fixMasterKeyAsRegularKey");
+        }
     }
 
     void