Merge pull request #1466 from MonsieurNicolas/upgradeImprovementsSing…

…leShot Upgrade improvements Reviewed-by: MonsieurNicolas
stellar · Dec 20, 2017 · 7ad53a5 · 7ad53a5
2 parents 59482f9 + ab42d2f
commit 7ad53a5
Show file tree

Hide file tree

Showing 34 changed files with 973 additions and 504 deletions.
diff --git a/docs/software/admin.md b/docs/software/admin.md
@@ -217,35 +217,33 @@ IMPORTANT:
 
 ## Network configuration
 
-The network itself has network wide settings, for example:
+The network itself has network wide settings that can be updated. This is done by validators voting for and agreeing to new values.
+
+A node can be configured to vote for upgrades using the `upgrades` endpoint . see [`commands.md`](commands.md) for more information.
+
+The network settings are:
  * the version of the protocol used to process transactions
- * the maximum number of transactions that can be included in a ledger
+ * the maximum number of transactions that can be included in a given ledger close
  * the cost (fee) associated with processing operations
+ * the base reserve used to calculate the lumen balance needed to store things in the ledger
 
-See the section on "Network wide settings" in the [example config](https://github.com/stellar/stellar-core/blob/master/docs/stellar-core_example.cfg)
-for more details on those parameters.
-
-When a network value is not the same as the one specified in its configuration,
-a validator will start to vote to update the network to the value specified in the
-configuration during ledgers following the date specified in `PREFERRED_UPGRADE_DATETIME`.
+When the network time is later than the `upgradetime` specified in
+the upgrade settings, the validator will vote to update the network
+to the value specified in the upgrade setting.
 
-When a validator is voting to change network values, the output of `info` will
-contain information about the vote. This can be useful to spot a misconfigured
-validator (if the operator didn't know about a network wide change for example).
+When a validator is armed to change network values, the output of `info` will
+contain information about the vote.
 
 For a new value to be adopted, the same level of consensus between nodes needs
-to be reached than for transaction sets.
+to be reached as for transaction sets.
 
 ### Important notes on network wide settings
 
 Changes to network wide settings have to be orchestrated properly between
 validators as well as non validating nodes:
 * a change is vetted between operators (changes can be bundled)
-* an effective date in the future is picked for the change to take effect (controlled by `PREFERRED_UPGRADE_DATETIME`)
-  * as soon as the date is decided, a best practice is to update
-  the configuration file before upgrading the binaries (as updating the version of stellar core
-  may trigger a vote for the new protocol version supported by that version of core)
-* if applicable, communication is sent out to consumers of the network
+* an effective date in the future is picked for the change to take effect (controlled by `upgradetime`)
+* if applicable, communication is sent out to all network users
 
 An improper plan may cause issues such as:
 * nodes missing consensus (aka "getting stuck"), and having to use history to rejoin
@@ -500,6 +498,22 @@ Run:
 4. `$ stellar-core` 
   - on each node to start it.
 
+## Upgrading network settings
+
+Read the section on [`network-configuration`](admin.md#network-configuration) for process to follow.
+
+Example here is to upgrade the protocol version to version 9 on January-31-2018.
+
+1. `$ stellar-core -c 'upgrades?mode=set&upgradetime=2018-01-31T20:00:00Z&protocolversion=9'`
+
+2. `$ stellar-core -c info`
+At this point `info` will tell you that the node is setup to vote for this upgrade:
+```
+      "status" : [
+         "Armed with network upgrades: upgradetime=2018-01-31T20:00:00Z, protocolversion=9"
+      ]
+```
+
 # Understanding the availability and health of your instance
 ## General info
 Run `$ stellar-core --c 'info'`

diff --git a/docs/software/commands.md b/docs/software/commands.md
@@ -141,6 +141,27 @@ debugging purpose).
         error: set when status is "ERROR".
             Base64 encoded, XDR serialized 'TransactionResult'
 
+* **upgrades**
+  * `/upgrades?mode=get`<br>
+  retrieves the currently configured upgrade settings<br>
+  * `/upgrades?mode=clear`<br>
+  clears any upgrade settings<br>
+  * `/upgrades?mode=set&upgradetime=DATETIME&[basefee=NUM]&[basereserve=NUM]&[maxtxsize=NUM]&[protocolversion=NUM]`<br>
+  upgradetime is a required date (UTC) in the form 1970-01-01T00:00:00Z.<br>
+    * fee (uint32) This is what you would prefer the base fee to be. It is
+        in stroops<br>
+    * basereserve (uint32) This is what you would prefer the base reserve 
+        to be. It is in stroops.<br>
+    * maxtxsize (uint32) This defines the maximum number of transactions 
+        to include in a ledger. When too many transactions are pending, 
+        surge pricing is applied. The instance picks the top maxtxsize
+         transactions locally to be considered in the next ledger.Where 
+        transactions are ordered by transaction fee(lower fee transactions
+         are held for later).<br>
+    * protocolversion (uint32) defines the protocol version to upgrade to.
+         When specified it must match the protocol version supported by the
+        node<br>
+
 ### The following HTTP commands are exposed on test instances
 * **generateload**
   `/generateload[?accounts=N&txs=M&txrate=(R|auto)]`<br>

diff --git a/docs/stellar-core_example.cfg b/docs/stellar-core_example.cfg
@@ -168,37 +168,6 @@ NODE_SEED="SBI3CZU7XZEWVXU7OZLW5MMUQAP334JFOPXSLTPOH43IRTEQ2QYXU5RG self"
 # See QUORUM_SET below.
 NODE_IS_VALIDATOR=false
 
-###########################
-# Network wide settings
-# Be careful when changing those value as those settings allow to
-# update network wide settings.
-# Validators will need to reach consensus before taking effect
-# In general, you need to ensure that those values match
-# the network ones to avoid voting rollbacks.
-# The value of the protocol version is tied to the version
-# of stellar-core used to run the validator (and cannot be set in config)
-
-# Date and Time in UTC when this node should start voting
-# for upgrades
-PREFERRED_UPGRADE_DATETIME=2017-03-01T00:00:00Z
-
-# DESIRED_BASE_FEE (integer) default 100
-# This is what you would prefer the base fee to be. It is in stroops.
-DESIRED_BASE_FEE=100
-
-# DESIRED_BASE_RESERVE (integer) default 100000000
-# This is what you would prefer the base reserve to be. It is in stroops.
-DESIRED_BASE_RESERVE=100000000
-
-# DESIRED_MAX_TX_PER_LEDGER (integer) default 50
-# This defines the maximum number of transactions to include in a ledger
-# When too many transactions are pending, surge pricing is applied:
-# * The instance picks the top MAX_TX_PER_LEDGER transactions locally to be
-#   considered in the next ledger. Where transactions are ordered by
-#   transaction fee (lower fee transactions are held for later).
-
-DESIRED_MAX_TX_PER_LEDGER=50
-
 ###########################
 # Consensus settings
 

diff --git a/docs/versioning.md b/docs/versioning.md
@@ -27,8 +27,8 @@ the SCP messages for the current ledger (ie: abstain).
 
 A node considers a step invalid either because:
 * they do not understand it, for example a new upgrade type not implemented
-* its value differs for this node configuration
-* network time is before PREFERRED_UPGRADE_DATETIME
+* its value differs for this node's scheduled upgrade setting
+* network time is before the scheduled upgrade datetime
 
 Upgrades are applied after applying the transaction set. It is done this way
 because the transaction set is validated against the last closed ledger,
@@ -37,19 +37,18 @@ without risking invalidating transactions for the current ledger.
 
 Supported upgrades are encoded using LedgerUpgradeType.
 
-Following configuration options are responsible for upgrades:
-* PREFERRED_UPGRADE_DATETIME - sets minimum time for node to accept and
+Upgrades are specified with:
+* upgradetime - the minimum time for node to accept and
   nominate upgrades
-* DESIRED_BASE_FEE - upgrades value of baseFee in ledger header, uses upgrade
+* basefee - upgrades value of baseFee in ledger header, uses upgrade
   type LEDGER_UPGRADE_BASE_FEE
-* DESIRED_MAX_TX_PER_LEDGER - upgrades value of maxTxSetSize in ledger header,
+* maxtxsize - upgrades value of maxTxSetSize in ledger header,
   uses upgrade type LEDGER_UPGRADE_MAX_TX_SET_SIZE
-* DESIRED_BASE_RESERVE - upgrades value of baseReserve in ledger header, uses
+* basereserve - upgrades value of baseReserve in ledger header, uses
   upgrade type LEDGER_UPGRADE_BASE_RESERVE
-
-Additionally nodes vote for changes to the ledgerVersion field with its
-non-configurable build-in LEDGER_PROTOCOL_VERSION field (which uses upgrade
-type LEDGER_UPGRADE_VERSION).
+* protocolversion - upgrades value of ledgerVersion in ledger header, uses
+  upgrade type LEDGER_UPGRADE_VERSION (when specified it has to match the
+  supported version number)
 
 #### Limitations of the current implementation
 There is an assumption that validator operators are either paying attention to network wide proposals

diff --git a/src/herder/Herder.h b/src/herder/Herder.h
@@ -5,6 +5,7 @@
 // of this distribution or at http://www.apache.org/licenses/LICENSE-2.0
 
 #include "TxSetFrame.h"
+#include "Upgrades.h"
 #include "lib/json/json-forwards.h"
 #include "overlay/StellarXDR.h"
 #include "scp/SCP.h"
@@ -94,8 +95,8 @@ class Herder
 
     virtual void bootstrap() = 0;
 
-    // restores SCP state based on the last messages saved on disk
-    virtual void restoreSCPState() = 0;
+    // restores Herder's state from disk
+    virtual void restoreState() = 0;
 
     virtual bool recvSCPQuorumSet(Hash const& hash,
                                   SCPQuorumSet const& qset) = 0;
@@ -126,6 +127,11 @@ class Herder
     // lookup a nodeID in config and in SCP messages
     virtual bool resolveNodeID(std::string const& s, PublicKey& retKey) = 0;
 
+    // sets the upgrades that should be applied during consensus
+    virtual void setUpgrades(Upgrades::UpgradeParameters const& upgrades) = 0;
+    // gets the upgrades that are scheduled by this node
+    virtual std::string getUpgradesJson() = 0;
+
     virtual ~Herder()
     {
     }

diff --git a/src/herder/HerderImpl.cpp b/src/herder/HerderImpl.cpp
@@ -72,7 +72,6 @@ HerderImpl::SCPMetrics::SCPMetrics(Application& app)
 HerderImpl::HerderImpl(Application& app)
     : mPendingTransactions(4)
     , mPendingEnvelopes(app, *this)
-    , mUpgrades(app.getConfig())
     , mHerderSCPDriver(app, *this, mUpgrades, mPendingEnvelopes)
     , mLastSlotSaved(0)
     , mTrackingTimer(app)
@@ -190,6 +189,17 @@ HerderImpl::valueExternalized(uint64 slotIndex, StellarValue const& value)
         static_cast<uint32>(slotIndex),
         getSCP().getExternalizingState(slotIndex));
 
+    // reflect upgrades with the ones included in this SCP round
+    {
+        bool updated;
+        auto newUpgrades = mUpgrades.removeUpgrades(
+            value.upgrades.begin(), value.upgrades.end(), updated);
+        if (updated)
+        {
+            setUpgrades(newUpgrades);
+        }
+    }
+
     // tell the LedgerManager that this value got externalized
     // LedgerManager will perform the proper action based on its internal
     // state: apply, trigger catchup, etc
@@ -748,7 +758,7 @@ HerderImpl::triggerNextLedger(uint32_t ledgerSeqToTrigger)
                                   0);
 
     // see if we need to include some upgrades
-    auto upgrades = mUpgrades.upgradesFor(lcl.header);
+    auto upgrades = mUpgrades.createUpgradesFor(lcl.header);
     for (auto const& upgrade : upgrades)
     {
         Value v(xdr::xdr_to_opaque(upgrade));
@@ -765,24 +775,42 @@ HerderImpl::triggerNextLedger(uint32_t ledgerSeqToTrigger)
         }
     }
 
-    if (!upgrades.empty())
+    mHerderSCPDriver.nominate(slotIndex, newProposedValue, proposedSet,
+                              lcl.header.scpValue);
+}
+
+void
+HerderImpl::setUpgrades(Upgrades::UpgradeParameters const& upgrades)
+{
+    mUpgrades.setParameters(upgrades, mApp.getConfig());
+    persistUpgrades();
+
+    auto desc = mUpgrades.toString();
+
+    if (!desc.empty())
     {
-        auto message = fmt::format(
-            "Proposing herder configuration upgrades: {0}; network "
-            "values for LCL are: {1}",
-            Upgrades::toString(upgrades), Upgrades::toString(lcl.header));
-        CLOG(INFO, "Herder") << message;
-        mApp.getStatusManager().setStatusMessage(
-            StatusCategory::REQUIRES_UPGRADES, message);
+        auto message = fmt::format("Armed with network upgrades: {}", desc);
+        auto prev = mApp.getStatusManager().getStatusMessage(
+            StatusCategory::REQUIRES_UPGRADES);
+        if (prev != message)
+        {
+            CLOG(INFO, "Herder") << message;
+            mApp.getStatusManager().setStatusMessage(
+                StatusCategory::REQUIRES_UPGRADES, message);
+        }
     }
     else
     {
+        CLOG(INFO, "Herder") << "Network upgrades cleared";
         mApp.getStatusManager().removeStatusMessage(
             StatusCategory::REQUIRES_UPGRADES);
     }
+}
 
-    mHerderSCPDriver.nominate(slotIndex, newProposedValue, proposedSet,
-                              lcl.header.scpValue);
+std::string
+HerderImpl::getUpgradesJson()
+{
+    return mUpgrades.getParameters().toJson();
 }
 
 bool
@@ -956,6 +984,42 @@ HerderImpl::restoreSCPState()
     }
 }
 
+void
+HerderImpl::persistUpgrades()
+{
+    auto s = mUpgrades.getParameters().toJson();
+    mApp.getPersistentState().setState(PersistentState::kLedgerUpgrades, s);
+}
+
+void
+HerderImpl::restoreUpgrades()
+{
+    std::string s =
+        mApp.getPersistentState().getState(PersistentState::kLedgerUpgrades);
+    if (!s.empty())
+    {
+        Upgrades::UpgradeParameters p;
+        p.fromJson(s);
+        try
+        {
+            // use common code to set status
+            setUpgrades(p);
+        }
+        catch (std::exception e)
+        {
+            CLOG(INFO, "Herder") << "Error restoring upgrades '" << e.what()
+                                 << "' with upgrades '" << s << "'";
+        }
+    }
+}
+
+void
+HerderImpl::restoreState()
+{
+    restoreSCPState();
+    restoreUpgrades();
+}
+
 void
 HerderImpl::trackingHeartBeat()
 {

diff --git a/src/herder/HerderImpl.h b/src/herder/HerderImpl.h
@@ -47,8 +47,7 @@ class HerderImpl : public Herder
     // Bootstraps the HerderImpl if we're creating a new Network
     void bootstrap() override;
 
-    // restores SCP state based on the last messages saved on disk
-    void restoreSCPState() override;
+    void restoreState() override;
 
     SCP& getSCP();
     HerderSCPDriver&
@@ -81,6 +80,9 @@ class HerderImpl : public Herder
 
     void triggerNextLedger(uint32_t ledgerSeqToTrigger) override;
 
+    void setUpgrades(Upgrades::UpgradeParameters const& upgrades) override;
+    std::string getUpgradesJson() override;
+
     bool resolveNodeID(std::string const& s, PublicKey& retKey) override;
 
     void dumpInfo(Json::Value& ret, size_t limit) override;
@@ -133,6 +135,12 @@ class HerderImpl : public Herder
 
     // saves the SCP messages that the instance sent out last
     void persistSCPState(uint64 slot);
+    // restores SCP state based on the last messages saved on disk
+    void restoreSCPState();
+
+    // saves upgrade parameters
+    void persistUpgrades();
+    void restoreUpgrades();
 
     // called every time we get ledger externalized
     // ensures that if we don't hear from the network, we throw the herder into