fix: add fatal log on amendment-blocked shutdown

- signalStop() for graceful shutdown when unsupported amendment activates
- early shutdown ~1 minute before expected activation to avoid race
- try/catch in switchLastClosedLedger to survive unknown field crashes
  during shutdown window
- show amendment warning to all RPC users, not just admin

Fixes: #706

build-core.sh

@@ -95,16 +95,8 @@ if [[ "$4" == "" ]]; then
   echo "Non GH, local building, no Action runner magic"
 else
   # GH Action, runner
-    if [[ "$(git rev-parse --abbrev-ref HEAD)" == "release" ]]; then
-        echo "building on the release branch... placing it in builds/candidate"
-        mkdir /data/builds/candidate
-        cp /io/release-build/xahaud /data/builds/candidate/$(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4
-        cp /io/release-build/release.info /data/builds/candidate/$(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4.releaseinfo
-    else
-        echo "building non-release branch, placing it in builds root"
-        cp /io/release-build/xahaud /data/builds/$(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4
-        cp /io/release-build/release.info /data/builds/$(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4.releaseinfo
-    fi  
+  cp /io/release-build/xahaud /data/builds/$(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4
+  cp /io/release-build/release.info /data/builds/$(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4.releaseinfo
   echo "Published build to: http://build.xahau.tech/"
   echo $(date +%Y).$(date +%-m).$(date +%-d)-$(git rev-parse --abbrev-ref HEAD)+$4
 fi

include/xrpl/protocol/jss.h

@@ -293,8 +293,6 @@ JSS(effective);               // out: ValidatorList
                               // in: UNL
 JSS(elapsed_seconds);
 JSS(enabled);                // out: AmendmentTable
-JSS(ledger_enabled);         // out: ServerDefinitions (amendment on-ledger)
-JSS(cfg_forced);             // out: ServerDefinitions ([features] config stanza)
 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

@@ -186,13 +186,10 @@ public:
             bool expectObsolete =
                 (votes.at(feature[jss::name].asString()) ==
                  VoteBehavior::Obsolete);
-            // "enabled" is now the effective value (ledger_voted || forced);
-            // this default env votes nothing onto the ledger, so assert on the
-            // canonical on-ledger flag.
             BEAST_EXPECTS(
-                feature.isMember(jss::ledger_enabled) &&
-                    !feature[jss::ledger_enabled].asBool(),
-                feature[jss::name].asString() + " ledger_enabled");
+                feature.isMember(jss::enabled) &&
+                    !feature[jss::enabled].asBool(),
+                feature[jss::name].asString() + " enabled");
             BEAST_EXPECTS(
                 feature.isMember(jss::vetoed) &&
                     feature[jss::vetoed].isBool() == !expectObsolete &&
@@ -240,12 +237,10 @@ public:
             bool expectObsolete =
                 (votes.at((*it)[jss::name].asString()) ==
                  VoteBehavior::Obsolete);
-            // expectEnabled reflects the on-ledger amendment table, so compare
-            // against ledger_enabled (enabled is now ledger_voted || forced).
             BEAST_EXPECTS(
-                (*it).isMember(jss::ledger_enabled) &&
-                    (*it)[jss::ledger_enabled].asBool() == expectEnabled,
-                (*it)[jss::name].asString() + " ledger_enabled");
+                (*it).isMember(jss::enabled) &&
+                    (*it)[jss::enabled].asBool() == expectEnabled,
+                (*it)[jss::name].asString() + " enabled");
             if (expectEnabled)
                 BEAST_EXPECTS(
                     !(*it).isMember(jss::vetoed),
@@ -365,78 +360,12 @@ public:
         }
     }
 
-    void
-    testConfigForced(FeatureBitset features)
-    {
-        testcase("Config-forced features ([features] stanza)");
-
-        using namespace test::jtx;
-
-        // jtx enables amendments by inserting them into config.features (the
-        // same presets mechanism as the [features] config stanza), so passing
-        // a single-feature bitset gives us exactly one config-forced amendment
-        // and votes nothing onto the ledger. server_definitions must then
-        // report that one as effectively enabled, distinguishing the source:
-        //   enabled        = ledger_enabled || cfg_forced
-        //   ledger_enabled = false (never voted onto the ledger)
-        //   cfg_forced     = true  (forced via config) for the one feature only
-        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();
-
-            // every entry now carries the split flags
-            if (!BEAST_EXPECTS(
-                    f.isMember(jss::enabled) &&
-                        f.isMember(jss::ledger_enabled) &&
-                        f.isMember(jss::cfg_forced),
-                    name + " split flags"))
-                return;
-
-            // nothing is enabled on-ledger in a fresh env
-            BEAST_EXPECTS(
-                !f[jss::ledger_enabled].asBool(), name + " ledger_enabled");
-
-            if (it.key().asString() == forcedHex)
-            {
-                sawForced = true;
-                BEAST_EXPECTS(
-                    f[jss::cfg_forced].asBool(), name + " cfg_forced");
-                // ledger_enabled(false) || cfg_forced(true) == true
-                BEAST_EXPECTS(f[jss::enabled].asBool(), name + " enabled");
-            }
-            else
-            {
-                BEAST_EXPECTS(
-                    !f[jss::cfg_forced].asBool(), name + " cfg_forced");
-                // not forced and not on-ledger => not effectively enabled
-                BEAST_EXPECTS(
-                    f[jss::enabled].asBool() == f[jss::ledger_enabled].asBool(),
-                    name + " enabled==ledger_enabled");
-            }
-        }
-        BEAST_EXPECT(sawForced);
-    }
-
     void
     testServerFeatures(FeatureBitset features)
     {
         testNoParams(features);
         testSomeEnabled(features);
         testWithMajorities(features);
-        testConfigForced(features);
     }
 
     void

src/xrpld/app/ledger/detail/LedgerMaster.cpp

@@ -311,12 +311,28 @@ LedgerMaster::setValidLedger(std::shared_ptr<Ledger const> const& l)
             if (auto const first =
                     app_.getAmendmentTable().firstUnsupportedExpected())
             {
-                JLOG(m_journal.error()) << "One or more unsupported amendments "
-                                           "reached majority. Upgrade before "
-                                        << to_string(*first)
-                                        << " to prevent your server from "
-                                           "becoming amendment blocked.";
-                app_.getOPs().setAmendmentWarned();
+                using namespace std::chrono_literals;
+                auto const now = app_.timeKeeper().closeTime();
+                if (*first > now && (*first - now) <= 1min)
+                {
+                    // Shut down just before the amendment activates to
+                    // avoid processing ledgers with unknown fields.
+                    JLOG(m_journal.error())
+                        << "Unsupported amendment activating imminently "
+                           "at "
+                        << to_string(*first) << ". Shutting down.";
+                    app_.getOPs().setAmendmentBlocked();
+                }
+                else
+                {
+                    JLOG(m_journal.error())
+                        << "One or more unsupported amendments "
+                           "reached majority. Upgrade before "
+                        << to_string(*first)
+                        << " to prevent your server from "
+                           "becoming amendment blocked.";
+                    app_.getOPs().setAmendmentWarned();
+                }
             }
             else
                 app_.getOPs().clearAmendmentWarned();

src/xrpld/app/misc/NetworkOPs.cpp

@@ -1634,6 +1634,16 @@ NetworkOPsImp::setAmendmentBlocked()
 {
     amendmentBlocked_ = true;
     setMode(OperatingMode::CONNECTED);
+    if (!app_.config().standalone())
+    {
+        JLOG(m_journal.fatal())
+            << "One or more unsupported amendments activated. "
+               "Shutting down. Upgrade the server to remain "
+               "compatible with the network.";
+        app_.signalStop(
+            "One or more unsupported amendments activated. "
+            "Server must be upgraded to remain compatible with the network.");
+    }
 }
 
 inline bool
@@ -1789,8 +1799,23 @@ NetworkOPsImp::switchLastClosedLedger(
 
     clearNeedNetworkLedger();
 
-    // Update fee computations.
-    app_.getTxQ().processClosedLedger(app_, *newLCL, true);
+    // Update fee computations. May throw if the ledger contains
+    // transactions with fields unknown to this binary (e.g. after an
+    // unsupported amendment activates). Catch to allow graceful shutdown.
+    //@@start process-closed-ledger-catch
+    try
+    {
+        app_.getTxQ().processClosedLedger(app_, *newLCL, true);
+    }
+    catch (std::runtime_error const& e)
+    {
+        if (!amendmentBlocked_)
+            throw;
+        JLOG(m_journal.error())
+            << "Failed to process closed ledger: " << e.what();
+        return;
+    }
+    //@@end process-closed-ledger-catch
 
     // Caller must own master lock
     {
@@ -2449,7 +2474,7 @@ NetworkOPsImp::getServerInfo(bool human, bool admin, bool counters)
                 "may be incorrectly configured or some [validator_list_sites] "
                 "may be unreachable.";
         }
-        if (admin && isAmendmentWarned())
+        if (isAmendmentWarned())
         {
             Json::Value& w = warnings.append(Json::objectValue);
             w[jss::id] = warnRPC_UNSUPPORTED_MAJORITY;
@@ -2893,6 +2918,7 @@ NetworkOPsImp::pubLedger(std::shared_ptr<ReadView const> const& lpAccepted)
     // Ledgers are published only when they acquire sufficient validations
     // Holes are filled across connection loss or other catastrophe
 
+    //@@start pubLedger-accepted-ledger-construction
     std::shared_ptr<AcceptedLedger> alpAccepted =
         app_.getAcceptedLedgerCache().fetch(lpAccepted->info().hash);
     if (!alpAccepted)
@@ -2901,6 +2927,7 @@ NetworkOPsImp::pubLedger(std::shared_ptr<ReadView const> const& lpAccepted)
         app_.getAcceptedLedgerCache().canonicalize_replace_client(
             lpAccepted->info().hash, alpAccepted);
     }
+    //@@end pubLedger-accepted-ledger-construction
 
     XRPL_ASSERT(
         alpAccepted->getLedger().get() == lpAccepted.get(),

src/xrpld/rpc/handlers/ServerDefinitions.cpp

@@ -22,7 +22,6 @@
 #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>
@@ -546,39 +545,6 @@ doServerDefinitions(RPC::JsonContext& context)
             features[to_string(h)][jss::majority] =
                 t.time_since_epoch().count();
 
-        // Amendment activation has two independent sources; surface both so a
-        // consumer isn't misled by a node that force-enables amendments:
-        //   ledger_enabled : recorded in the on-ledger Amendments object
-        //                    (network-canonical; what the table reports as
-        //                    "enabled")
-        //   cfg_forced     : force-activated via the [features] config stanza
-        //                    (node-local; active in the Rules regardless of the
-        //                    ledger, casts no votes, never written on-ledger)
-        //   enabled        : effective for transaction processing on this
-        //                    server, i.e. ledger_enabled || cfg_forced
-        for (auto const& name : features.getMemberNames())
-        {
-            Json::Value& entry = features[name];
-            bool const ledgerEnabled = entry[jss::enabled].asBool();
-            entry[jss::ledger_enabled] = ledgerEnabled;
-            entry[jss::cfg_forced] = false;
-            // entry[jss::enabled] is left == ledgerEnabled here; only
-            // cfg_forced amendments below flip it.
-        }
-        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::cfg_forced] = true;
-            entry[jss::enabled] = true;  // ledger_enabled || cfg_forced
-        }
-
         lastFeatures = features;
         {
             const std::string out = Json::FastWriter().write(features);