feat(server_definitions): distinguish config-forced from ledger-enabled amendments (#765)

Replace redundant custom consequence factories that returned normal consequences with the built-in Normal factory.

include/xrpl/protocol/jss.h

@@ -292,7 +292,9 @@ JSS(duration_us);             // out: NetworkOPs
 JSS(effective);               // out: ValidatorList
                               // in: UNL
 JSS(elapsed_seconds);
-JSS(enabled);                // out: AmendmentTable
+JSS(enabled);                // out: AmendmentTable (on-ledger);
+                             //      ServerDefinitions (this server)
+JSS(ledger_enabled);         // out: ServerDefinitions (on-ledger)
 JSS(engine_result);          // out: NetworkOPs, TransactionSign, Submit
 JSS(engine_result_code);     // out: NetworkOPs, TransactionSign, Submit
 JSS(engine_result_message);  // out: NetworkOPs, TransactionSign, Submit

src/test/rpc/ServerDefinitions_test.cpp

@@ -163,7 +163,7 @@ public:
     void
     testNoParams(FeatureBitset features)
     {
-        testcase("No Params, None Enabled");
+        testcase("Default Env: config-forced, none on-ledger");
 
         using namespace test::jtx;
         Env env{*this};
@@ -178,8 +178,6 @@ public:
         {
             if (!BEAST_EXPECT(feature.isMember(jss::name)))
                 return;
-            // default config - so all should be disabled, and
-            // supported. Some may be vetoed.
             bool expectVeto =
                 (votes.at(feature[jss::name].asString()) ==
                  VoteBehavior::DefaultNo);
@@ -188,8 +186,12 @@ public:
                  VoteBehavior::Obsolete);
             BEAST_EXPECTS(
                 feature.isMember(jss::enabled) &&
-                    !feature[jss::enabled].asBool(),
+                    feature[jss::enabled].asBool(),
                 feature[jss::name].asString() + " enabled");
+            BEAST_EXPECTS(
+                feature.isMember(jss::ledger_enabled) &&
+                    !feature[jss::ledger_enabled].asBool(),
+                feature[jss::name].asString() + " ledger_enabled");
             BEAST_EXPECTS(
                 feature.isMember(jss::vetoed) &&
                     feature[jss::vetoed].isBool() == !expectObsolete &&
@@ -208,7 +210,7 @@ public:
     void
     testSomeEnabled(FeatureBitset features)
     {
-        testcase("No Params, Some Enabled");
+        testcase("Two config-forced, none on-ledger");
 
         using namespace test::jtx;
         Env env{
@@ -228,7 +230,10 @@ public:
             (void)id.parseHex(it.key().asString().c_str());
             if (!BEAST_EXPECT((*it).isMember(jss::name)))
                 return;
-            bool expectEnabled = env.app().getAmendmentTable().isEnabled(id);
+            bool const expectOnLedger =
+                env.app().getAmendmentTable().isEnabled(id);
+            bool const expectForced =
+                id == featureDepositAuth || id == featureDepositPreauth;
             bool expectSupported =
                 env.app().getAmendmentTable().isSupported(id);
             bool expectVeto =
@@ -239,9 +244,14 @@ public:
                  VoteBehavior::Obsolete);
             BEAST_EXPECTS(
                 (*it).isMember(jss::enabled) &&
-                    (*it)[jss::enabled].asBool() == expectEnabled,
+                    (*it)[jss::enabled].asBool() ==
+                        (expectOnLedger || expectForced),
                 (*it)[jss::name].asString() + " enabled");
-            if (expectEnabled)
+            BEAST_EXPECTS(
+                (*it).isMember(jss::ledger_enabled) &&
+                    (*it)[jss::ledger_enabled].asBool() == expectOnLedger,
+                (*it)[jss::name].asString() + " ledger_enabled");
+            if (expectOnLedger)
                 BEAST_EXPECTS(
                     !(*it).isMember(jss::vetoed),
                     (*it)[jss::name].asString() + " vetoed");
@@ -360,12 +370,122 @@ public:
         }
     }
 
+    void
+    testConfigForced(FeatureBitset features)
+    {
+        testcase("Config-forced features ([features] stanza)");
+
+        using namespace test::jtx;
+
+        auto const forced = featurePriceOracle;
+        auto const forcedHex = to_string(forced);
+
+        Env env{*this, FeatureBitset(forced)};
+
+        auto jrr = env.rpc("server_definitions")[jss::result];
+        if (!BEAST_EXPECT(jrr.isMember(jss::features)))
+            return;
+
+        bool sawForced = false;
+        for (auto it = jrr[jss::features].begin();
+             it != jrr[jss::features].end();
+             ++it)
+        {
+            auto const& f = *it;
+            auto const name = f[jss::name].asString();
+
+            if (!BEAST_EXPECTS(
+                    f.isMember(jss::enabled) && f.isMember(jss::ledger_enabled),
+                    name + " enabled/ledger_enabled"))
+                return;
+
+            BEAST_EXPECTS(
+                !f[jss::ledger_enabled].asBool(), name + " ledger_enabled");
+
+            if (it.key().asString() == forcedHex)
+            {
+                sawForced = true;
+                BEAST_EXPECTS(f[jss::enabled].asBool(), name + " enabled");
+            }
+            else
+            {
+                BEAST_EXPECTS(!f[jss::enabled].asBool(), name + " enabled");
+            }
+        }
+        BEAST_EXPECT(sawForced);
+    }
+
+    void
+    testOnLedger(FeatureBitset features)
+    {
+        testcase("On-ledger plus one config-forced");
+
+        using namespace test::jtx;
+
+        // Veto XahauGenesis: FRESH would otherwise enable it via pseudo-tx.
+        auto const forced = featurePriceOracle;
+        auto const forcedHex = to_string(forced);
+
+        Env env{
+            *this,
+            envconfig([](std::unique_ptr<Config> cfg) {
+                cfg->START_UP = Config::FRESH;
+                cfg->section(SECTION_VETO_AMENDMENTS)
+                    .append(to_string(featureXahauGenesis) + " XahauGenesis");
+                return cfg;
+            }),
+            FeatureBitset(forced)};
+
+        auto jrr = env.rpc("server_definitions")[jss::result];
+        if (!BEAST_EXPECT(jrr.isMember(jss::features)))
+            return;
+
+        bool sawOnLedger = false;
+        bool sawForced = false;
+        for (auto it = jrr[jss::features].begin();
+             it != jrr[jss::features].end();
+             ++it)
+        {
+            uint256 id;
+            (void)id.parseHex(it.key().asString().c_str());
+            auto const& f = *it;
+            auto const name = f[jss::name].asString();
+
+            if (!BEAST_EXPECTS(
+                    f.isMember(jss::enabled) && f.isMember(jss::ledger_enabled),
+                    name + " enabled/ledger_enabled"))
+                return;
+
+            bool const onLedger = env.app().getAmendmentTable().isEnabled(id);
+            bool const isForced = it.key().asString() == forcedHex;
+
+            BEAST_EXPECTS(
+                f[jss::ledger_enabled].asBool() == onLedger,
+                name + " ledger_enabled");
+            BEAST_EXPECTS(
+                f[jss::enabled].asBool() == (onLedger || isForced),
+                name + " enabled");
+
+            if (onLedger)
+                sawOnLedger = true;
+            if (isForced)
+            {
+                sawForced = true;
+                BEAST_EXPECTS(!onLedger, name + " forced not on-ledger");
+            }
+        }
+        BEAST_EXPECT(sawOnLedger);
+        BEAST_EXPECT(sawForced);
+    }
+
     void
     testServerFeatures(FeatureBitset features)
     {
         testNoParams(features);
         testSomeEnabled(features);
         testWithMajorities(features);
+        testConfigForced(features);
+        testOnLedger(features);
     }
 
     void

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

@@ -30,12 +30,6 @@
 
 namespace ripple {
 
-TxConsequences
-ClaimReward::makeTxConsequences(PreflightContext const& ctx)
-{
-    return TxConsequences{ctx.tx, TxConsequences::normal};
-}
-
 NotTEC
 ClaimReward::preflight(PreflightContext const& ctx)
 {

src/xrpld/app/tx/detail/ClaimReward.h

@@ -32,15 +32,12 @@ namespace ripple {
 class ClaimReward : public Transactor
 {
 public:
-    static constexpr ConsequencesFactoryType ConsequencesFactory{Custom};
+    static constexpr ConsequencesFactoryType ConsequencesFactory{Normal};
 
     explicit ClaimReward(ApplyContext& ctx) : Transactor(ctx)
     {
     }
 
-    static TxConsequences
-    makeTxConsequences(PreflightContext const& ctx);
-
     static NotTEC
     preflight(PreflightContext const& ctx);
 

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

@@ -30,12 +30,6 @@
 
 namespace ripple {
 
-TxConsequences
-Cron::makeTxConsequences(PreflightContext const& ctx)
-{
-    return TxConsequences{ctx.tx, TxConsequences::normal};
-}
-
 NotTEC
 Cron::preflight(PreflightContext const& ctx)
 {

src/xrpld/app/tx/detail/Cron.h

@@ -30,7 +30,7 @@ namespace ripple {
 class Cron : public Transactor
 {
 public:
-    static constexpr ConsequencesFactoryType ConsequencesFactory{Custom};
+    static constexpr ConsequencesFactoryType ConsequencesFactory{Normal};
 
     explicit Cron(ApplyContext& ctx) : Transactor(ctx)
     {
@@ -39,9 +39,6 @@ public:
     static XRPAmount
     calculateBaseFee(ReadView const& view, STTx const& tx);
 
-    static TxConsequences
-    makeTxConsequences(PreflightContext const& ctx);
-
     static NotTEC
     preflight(PreflightContext const& ctx);
 

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

@@ -27,12 +27,6 @@
 
 namespace ripple {
 
-TxConsequences
-CronSet::makeTxConsequences(PreflightContext const& ctx)
-{
-    return TxConsequences{ctx.tx, TxConsequences::normal};
-}
-
 NotTEC
 CronSet::preflight(PreflightContext const& ctx)
 {

src/xrpld/app/tx/detail/CronSet.h

@@ -29,7 +29,7 @@ namespace ripple {
 class CronSet : public Transactor
 {
 public:
-    static constexpr ConsequencesFactoryType ConsequencesFactory{Custom};
+    static constexpr ConsequencesFactoryType ConsequencesFactory{Normal};
 
     explicit CronSet(ApplyContext& ctx) : Transactor(ctx)
     {
@@ -38,9 +38,6 @@ public:
     static XRPAmount
     calculateBaseFee(ReadView const& view, STTx const& tx);
 
-    static TxConsequences
-    makeTxConsequences(PreflightContext const& ctx);
-
     static NotTEC
     preflight(PreflightContext const& ctx);
 

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

@@ -26,12 +26,6 @@
 
 namespace ripple {
 
-TxConsequences
-Invoke::makeTxConsequences(PreflightContext const& ctx)
-{
-    return TxConsequences{ctx.tx, TxConsequences::normal};
-}
-
 NotTEC
 Invoke::preflight(PreflightContext const& ctx)
 {

src/xrpld/app/tx/detail/Invoke.h

@@ -30,7 +30,7 @@ namespace ripple {
 class Invoke : public Transactor
 {
 public:
-    static constexpr ConsequencesFactoryType ConsequencesFactory{Custom};
+    static constexpr ConsequencesFactoryType ConsequencesFactory{Normal};
 
     explicit Invoke(ApplyContext& ctx) : Transactor(ctx)
     {
@@ -39,9 +39,6 @@ public:
     static XRPAmount
     calculateBaseFee(ReadView const& view, STTx const& tx);
 
-    static TxConsequences
-    makeTxConsequences(PreflightContext const& ctx);
-
     static NotTEC
     preflight(PreflightContext const& ctx);
 

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

@@ -30,12 +30,6 @@
 
 namespace ripple {
 
-TxConsequences
-SetRemarks::makeTxConsequences(PreflightContext const& ctx)
-{
-    return TxConsequences{ctx.tx, TxConsequences::normal};
-}
-
 NotTEC
 SetRemarks::validateRemarks(STArray const& remarks, beast::Journal const& j)
 {

src/xrpld/app/tx/detail/SetRemarks.h

@@ -30,7 +30,7 @@ namespace ripple {
 class SetRemarks : public Transactor
 {
 public:
-    static constexpr ConsequencesFactoryType ConsequencesFactory{Custom};
+    static constexpr ConsequencesFactoryType ConsequencesFactory{Normal};
 
     explicit SetRemarks(ApplyContext& ctx) : Transactor(ctx)
     {
@@ -39,9 +39,6 @@ public:
     static XRPAmount
     calculateBaseFee(ReadView const& view, STTx const& tx);
 
-    static TxConsequences
-    makeTxConsequences(PreflightContext const& ctx);
-
     static NotTEC
     preflight(PreflightContext const& ctx);
 

src/xrpld/rpc/handlers/ServerDefinitions.cpp

@@ -22,6 +22,7 @@
 #include <xrpld/app/main/Application.h>
 #include <xrpld/app/misc/AmendmentTable.h>
 #include <xrpld/app/misc/NetworkOPs.h>
+#include <xrpld/core/Config.h>
 #include <xrpld/rpc/detail/TransactionSign.h>
 #include <xrpl/json/json_value.h>
 #include <xrpl/json/json_writer.h>
@@ -545,6 +546,25 @@ doServerDefinitions(RPC::JsonContext& context)
             features[to_string(h)][jss::majority] =
                 t.time_since_epoch().count();
 
+        // getJson's enabled is on-ledger; [features] also apply here.
+        for (auto const& name : features.getMemberNames())
+        {
+            Json::Value& entry = features[name];
+            entry[jss::ledger_enabled] = entry[jss::enabled].asBool();
+        }
+        for (auto const& h : context.app.config().features)
+        {
+            Json::Value& entry = features[to_string(h)];
+            if (!entry.isMember(jss::name))
+            {
+                if (auto const fname = featureToName(h); !fname.empty())
+                    entry[jss::name] = fname;
+            }
+            if (!entry.isMember(jss::ledger_enabled))
+                entry[jss::ledger_enabled] = false;
+            entry[jss::enabled] = true;
+        }
+
         lastFeatures = features;
         {
             const std::string out = Json::FastWriter().write(features);