From 3b1df286cd7142480d1fd813d2ddc8b7f000b915 Mon Sep 17 00:00:00 2001
From: MonsieurNicolas <nicolas@stellar.org>
Date: Mon, 26 Oct 2015 15:42:05 -0700
Subject: [PATCH 01/16] clang-format debt

---
 src/bucket/BucketTests.cpp            |  2 +-
 src/herder/HerderImpl.cpp             |  3 +--
 src/herder/HerderTests.cpp            |  4 ++--
 src/history/HistoryTests.cpp          |  4 ++--
 src/ledger/LedgerHeaderTests.cpp      |  2 +-
 src/ledger/LedgerPerformanceTests.cpp |  3 +--
 src/main/Application.cpp              |  3 ++-
 src/main/Application.h                |  3 ++-
 src/main/ApplicationImpl.cpp          | 10 +++++-----
 src/main/Config.cpp                   | 21 +++++++++++----------
 src/main/ExternalQueue.cpp            |  5 +++--
 src/main/PersistentState.cpp          |  5 ++---
 src/main/main.cpp                     | 10 +++++-----
 src/overlay/OverlayManagerTests.cpp   |  1 -
 src/simulation/Simulation.cpp         |  2 +-
 src/simulation/Simulation.h           |  2 +-
 src/util/Logging.cpp                  |  3 +--
 17 files changed, 41 insertions(+), 42 deletions(-)

diff --git a/src/bucket/BucketTests.cpp b/src/bucket/BucketTests.cpp
index 0f2f276b8c..980dfcd1c9 100644
--- a/src/bucket/BucketTests.cpp
+++ b/src/bucket/BucketTests.cpp
@@ -814,7 +814,7 @@ TEST_CASE("bucket persistence over app restart", "[bucket][bucketpersist]")
     // pick up the bucket list correctly.
     cfg1.FORCE_SCP = false;
     {
-        Application::pointer app = Application::create(clock, cfg1,false);
+        Application::pointer app = Application::create(clock, cfg1, false);
         app->start();
         BucketList& bl = app->getBucketManager().getBucketList();
 
diff --git a/src/herder/HerderImpl.cpp b/src/herder/HerderImpl.cpp
index 9e10c2cf24..b958ab7e9d 100644
--- a/src/herder/HerderImpl.cpp
+++ b/src/herder/HerderImpl.cpp
@@ -1478,8 +1478,7 @@ HerderImpl::acceptedCommit(uint64 slotIndex, SCPBallot const& ballot)
 void
 HerderImpl::dumpInfo(Json::Value& ret)
 {
-    ret["you"] =
-        mApp.getConfig().toStrKey(mSCP.getSecretKey().getPublicKey());
+    ret["you"] = mApp.getConfig().toStrKey(mSCP.getSecretKey().getPublicKey());
 
     mSCP.dumpInfo(ret);
 
diff --git a/src/herder/HerderTests.cpp b/src/herder/HerderTests.cpp
index 72fab4656a..b4e7b6d28f 100644
--- a/src/herder/HerderTests.cpp
+++ b/src/herder/HerderTests.cpp
@@ -652,8 +652,8 @@ TEST_CASE("SCP State", "[herder]")
         // forwarded to node 2 when they connect to it
         // causing node 2 to externalize ledger #2
 
-        sim->addNode(nodeKeys[0], qSetAll, *clock, &nodeCfgs[0],false);
-        sim->addNode(nodeKeys[1], qSetAll, *clock, &nodeCfgs[1],false);
+        sim->addNode(nodeKeys[0], qSetAll, *clock, &nodeCfgs[0], false);
+        sim->addNode(nodeKeys[1], qSetAll, *clock, &nodeCfgs[1], false);
         sim->getNode(nodeIDs[0])->start();
         sim->getNode(nodeIDs[1])->start();
 
diff --git a/src/history/HistoryTests.cpp b/src/history/HistoryTests.cpp
index 0bb1ceb128..c1b072efd0 100644
--- a/src/history/HistoryTests.cpp
+++ b/src/history/HistoryTests.cpp
@@ -405,7 +405,7 @@ HistoryTests::catchupNewApplication(uint32_t initLedger,
         getTestConfig(static_cast<int>(mCfgs.size()) + 1, dbMode));
     Application::pointer app2 = Application::create(
         clock, mConfigurator->configure(mCfgs.back(), false));
-    
+
     app2->start();
     CHECK(catchupApplication(initLedger, resumeMode, app2) == true);
     return app2;
@@ -818,7 +818,7 @@ TEST_CASE_METHOD(HistoryTests, "Repair missing buckets via history",
         Application::create(clock, mConfigurator->configure(cfg2, false));
     app2->getPersistentState().setState(PersistentState::kHistoryArchiveState,
                                         state);
-   
+
     app2->start();
 
     auto hash1 = appPtr->getBucketManager().getBucketList().getHash();
diff --git a/src/ledger/LedgerHeaderTests.cpp b/src/ledger/LedgerHeaderTests.cpp
index d65e72ad48..28aeb647dc 100644
--- a/src/ledger/LedgerHeaderTests.cpp
+++ b/src/ledger/LedgerHeaderTests.cpp
@@ -47,7 +47,7 @@ TEST_CASE("ledgerheader", "[ledger]")
         Config cfg2(cfg);
         cfg2.FORCE_SCP = false;
         VirtualClock clock2;
-        Application::pointer app2 = Application::create(clock2, cfg2,false);
+        Application::pointer app2 = Application::create(clock2, cfg2, false);
         app2->start();
 
         REQUIRE(saved ==
diff --git a/src/ledger/LedgerPerformanceTests.cpp b/src/ledger/LedgerPerformanceTests.cpp
index f28f35ba8d..1b68c05c20 100644
--- a/src/ledger/LedgerPerformanceTests.cpp
+++ b/src/ledger/LedgerPerformanceTests.cpp
@@ -164,14 +164,13 @@ TEST_CASE("ledger performance test", "[performance][hide]")
     qSet0.threshold = 1;
     qSet0.validators.push_back(v10NodeID);
 
-   
     cfg.DATABASE = "postgresql://host=localhost dbname=performance_test "
                    "user=test password=test";
     cfg.BUCKET_DIR_PATH = "performance-test.db.buckets";
     cfg.MANUAL_CLOSE = true;
     sim.addNode(v10SecretKey, qSet0, sim.getClock(), &cfg);
     sim.mApp = sim.getNodes().front();
-    
+
     sim.startAllNodes();
 
     Timer& ledgerTimer = sim.mApp->getMetrics().NewTimer(
diff --git a/src/main/Application.cpp b/src/main/Application.cpp
index dc1dd2d621..81eb12dda0 100644
--- a/src/main/Application.cpp
+++ b/src/main/Application.cpp
@@ -13,7 +13,8 @@ Application::pointer
 Application::create(VirtualClock& clock, Config const& cfg, bool newDB)
 {
     Application::pointer ret = make_shared<ApplicationImpl>(clock, cfg);
-    if(newDB || cfg.DATABASE == "sqlite3://:memory:") ret->newDB();
+    if (newDB || cfg.DATABASE == "sqlite3://:memory:")
+        ret->newDB();
 
     return ret;
 }
diff --git a/src/main/Application.h b/src/main/Application.h
index ffe38430d8..c47d44c4b0 100644
--- a/src/main/Application.h
+++ b/src/main/Application.h
@@ -239,7 +239,8 @@ class Application
 
     // Factory: create a new Application object bound to `clock`, with a local
     // copy made of `cfg`.
-    static pointer create(VirtualClock& clock, Config const& cfg,bool newDB=true);
+    static pointer create(VirtualClock& clock, Config const& cfg,
+                          bool newDB = true);
 
   protected:
     Application()
diff --git a/src/main/ApplicationImpl.cpp b/src/main/ApplicationImpl.cpp
index e5a027b42a..92bf58a6c6 100644
--- a/src/main/ApplicationImpl.cpp
+++ b/src/main/ApplicationImpl.cpp
@@ -67,7 +67,7 @@ ApplicationImpl::ApplicationImpl(VirtualClock& clock, Config const& cfg)
 
     unsigned t = std::thread::hardware_concurrency();
     LOG(DEBUG) << "Application constructing "
-              << "(worker threads: " << t << ")";
+               << "(worker threads: " << t << ")";
     mStopSignals.async_wait([this](asio::error_code const& ec, int sig)
                             {
                                 if (!ec)
@@ -82,7 +82,7 @@ ApplicationImpl::ApplicationImpl(VirtualClock& clock, Config const& cfg)
     // into App.getFoo() to get information / start up.
     mDatabase = make_unique<Database>(*this);
     mPersistentState = make_unique<PersistentState>(*this);
-   
+
     mTmpDirManager = make_unique<TmpDirManager>(cfg.TMP_DIR_PATH);
     mOverlayManager = OverlayManager::create(*this);
     mLedgerManager = LedgerManager::create(*this);
@@ -103,7 +103,7 @@ ApplicationImpl::ApplicationImpl(VirtualClock& clock, Config const& cfg)
     LOG(DEBUG) << "Application constructed";
 }
 
-void 
+void
 ApplicationImpl::newDB()
 {
     mDatabase->initialize();
@@ -209,8 +209,8 @@ ApplicationImpl::start()
 {
     mDatabase->upgradeToCurrentSchema();
 
-    if(mPersistentState->getState(
-        PersistentState::kForceSCPOnNextLaunch) == "true")
+    if (mPersistentState->getState(PersistentState::kForceSCPOnNextLaunch) ==
+        "true")
     {
         mConfig.FORCE_SCP = true;
     }
diff --git a/src/main/Config.cpp b/src/main/Config.cpp
index 9735c183f4..009b5dfc14 100644
--- a/src/main/Config.cpp
+++ b/src/main/Config.cpp
@@ -636,8 +636,8 @@ Config::validateConfig()
             {
                 LOG(ERROR)
                     << "Not enough nodes / thresholds too strict in your "
-                    "Quorum set to ensure your  desired level of "
-                    "FAILURE_SAFETY.";
+                       "Quorum set to ensure your  desired level of "
+                       "FAILURE_SAFETY.";
                 throw std::invalid_argument("SCP unsafe");
             }
 
@@ -713,7 +713,7 @@ Config::toShortString(PublicKey const& pk) const
     std::string ret = PubKeyUtils::toStrKey(pk);
     auto it = VALIDATOR_NAMES.find(ret);
     if (it == VALIDATOR_NAMES.end())
-        return ret.substr(0,5);
+        return ret.substr(0, 5);
     else
         return it->second;
 }
@@ -739,18 +739,19 @@ Config::resolveNodeID(std::string const& s, PublicKey& retKey) const
         if (s[0] == '$')
         {
             it = std::find_if(VALIDATOR_NAMES.begin(), VALIDATOR_NAMES.end(),
-                                   [&](std::pair<std::string, std::string> const& p)
-            {
-                return p.second == arg;
-            });
+                              [&](std::pair<std::string, std::string> const& p)
+                              {
+                                  return p.second == arg;
+                              });
         }
         else if (s[0] == '@')
         {
             it = std::find_if(VALIDATOR_NAMES.begin(), VALIDATOR_NAMES.end(),
                               [&](std::pair<std::string, std::string> const& p)
-            {
-                return p.first.compare(0, arg.size(), arg) == 0;
-            });
+                              {
+                                  return p.first.compare(0, arg.size(), arg) ==
+                                         0;
+                              });
         }
 
         if (it == VALIDATOR_NAMES.end())
diff --git a/src/main/ExternalQueue.cpp b/src/main/ExternalQueue.cpp
index 5fe58915ec..9d0f78a51e 100644
--- a/src/main/ExternalQueue.cpp
+++ b/src/main/ExternalQueue.cpp
@@ -134,8 +134,9 @@ ExternalQueue::process()
     // publication and the requirements of our pubsub subscribers.
     uint32_t cmin = std::min(lmin, rmin);
 
-    CLOG(DEBUG,"History") << "Trimming history <= ledger " << cmin << " (rmin=" << rmin
-              << ", qmin=" << qmin << ", lmin=" << lmin << ")";
+    CLOG(DEBUG, "History") << "Trimming history <= ledger " << cmin
+                           << " (rmin=" << rmin << ", qmin=" << qmin
+                           << ", lmin=" << lmin << ")";
 
     mApp.getLedgerManager().deleteOldEntries(mApp.getDatabase(), cmin);
 }
diff --git a/src/main/PersistentState.cpp b/src/main/PersistentState.cpp
index 7d936e6a85..1f4c381745 100644
--- a/src/main/PersistentState.cpp
+++ b/src/main/PersistentState.cpp
@@ -13,8 +13,8 @@ namespace stellar
 using namespace std;
 
 string PersistentState::mapping[kLastEntry] = {
-    "lastclosedledger",    "historyarchivestate", "forcescponnextlaunch",
-     "lastscpdata",         "databaseschema"};
+    "lastclosedledger", "historyarchivestate", "forcescponnextlaunch",
+    "lastscpdata", "databaseschema"};
 
 string PersistentState::kSQLCreateStatement =
     "CREATE TABLE IF NOT EXISTS storestate ("
@@ -24,7 +24,6 @@ string PersistentState::kSQLCreateStatement =
 
 PersistentState::PersistentState(Application& app) : mApp(app)
 {
-    
 }
 
 void
diff --git a/src/main/main.cpp b/src/main/main.cpp
index eeee7ea552..c27b9ec8c3 100644
--- a/src/main/main.cpp
+++ b/src/main/main.cpp
@@ -202,7 +202,7 @@ void
 loadXdr(Config const& cfg, std::string const& bucketFile)
 {
     VirtualClock clock;
-    Application::pointer app = Application::create(clock, cfg,false);
+    Application::pointer app = Application::create(clock, cfg, false);
     if (checkInitialized(app))
     {
         uint256 zero;
@@ -235,8 +235,8 @@ initializeHistories(Config& cfg, vector<string> newHistories)
 
     for (auto const& arch : newHistories)
     {
-        if(!HistoryManager::initializeHistoryArchive(*app, arch))
-                    return 1;           
+        if (!HistoryManager::initializeHistoryArchive(*app, arch))
+            return 1;
     }
     return 0;
 }
@@ -394,7 +394,8 @@ main(int argc, char* const* argv)
             s += cfgFile + " found";
             throw std::invalid_argument(s);
         }
-        Logging::setFmt(PubKeyUtils::toShortString(cfg.NODE_SEED.getPublicKey()));
+        Logging::setFmt(
+            PubKeyUtils::toShortString(cfg.NODE_SEED.getPublicKey()));
         Logging::setLogLevel(logLevel, nullptr);
 
         if (command.size())
@@ -408,7 +409,6 @@ main(int argc, char* const* argv)
             Logging::setLoggingToFile(cfg.LOG_FILE_PATH);
         Logging::setLogLevel(logLevel, nullptr);
 
-        
         cfg.REPORT_METRICS = metrics;
 
         if (forceSCP || newDB || getInfo || !loadXdrBucket.empty())
diff --git a/src/overlay/OverlayManagerTests.cpp b/src/overlay/OverlayManagerTests.cpp
index 8478ac2628..01b5718ae2 100644
--- a/src/overlay/OverlayManagerTests.cpp
+++ b/src/overlay/OverlayManagerTests.cpp
@@ -108,7 +108,6 @@ class OverlayManagerTests
     {
         OverlayManagerStub& pm = app.getOverlayManager();
 
-
         pm.storePeerList(fourPeers);
 
         rowset<row> rs = app.getDatabase().getSession().prepare
diff --git a/src/simulation/Simulation.cpp b/src/simulation/Simulation.cpp
index 266e085e74..0a47a35483 100644
--- a/src/simulation/Simulation.cpp
+++ b/src/simulation/Simulation.cpp
@@ -54,7 +54,7 @@ Simulation::getClock()
 
 NodeID
 Simulation::addNode(SecretKey nodeKey, SCPQuorumSet qSet, VirtualClock& clock,
-                    Config const* cfg2, bool newDB )
+                    Config const* cfg2, bool newDB)
 {
     std::shared_ptr<Config> cfg;
     if (!cfg2)
diff --git a/src/simulation/Simulation.h b/src/simulation/Simulation.h
index 8e40ea769e..68b4336765 100644
--- a/src/simulation/Simulation.h
+++ b/src/simulation/Simulation.h
@@ -44,7 +44,7 @@ class Simulation : public LoadGenerator
     VirtualClock& getClock();
 
     NodeID addNode(SecretKey nodeKey, SCPQuorumSet qSet, VirtualClock& clock,
-                   Config const* cfg = nullptr,bool newDB=true);
+                   Config const* cfg = nullptr, bool newDB = true);
     Application::pointer getNode(NodeID nodeID);
     std::vector<Application::pointer> getNodes();
     std::vector<NodeID> getNodeIDs();
diff --git a/src/util/Logging.cpp b/src/util/Logging.cpp
index f266da55d8..3c57c2bfb4 100644
--- a/src/util/Logging.cpp
+++ b/src/util/Logging.cpp
@@ -28,8 +28,7 @@ Logging::setFmt(std::string const& peerID, bool timestamps)
     {
         datetime = "%datetime{%Y-%M-%dT%H:%m:%s.%g}";
     }
-    std::string shortFmt =
-        datetime + " " + peerID + " [%logger %level] %msg";
+    std::string shortFmt = datetime + " " + peerID + " [%logger %level] %msg";
     std::string longFmt = shortFmt + " [%fbase:%line]";
 
     gDefaultConf.setGlobally(el::ConfigurationType::Format, shortFmt);

From 8db21098e221cc8f47a70eac62b2cb4e98e69061 Mon Sep 17 00:00:00 2001
From: MonsieurNicolas <nicolas@stellar.org>
Date: Mon, 26 Oct 2015 15:13:17 -0700
Subject: [PATCH 02/16] added "isFullyValidated" property to SCP slot

---
 src/scp/Slot.cpp | 14 ++++++++++++++
 src/scp/Slot.h   |  6 ++++++
 2 files changed, 20 insertions(+)

diff --git a/src/scp/Slot.cpp b/src/scp/Slot.cpp
index 7a6b9c5786..9c171d3812 100644
--- a/src/scp/Slot.cpp
+++ b/src/scp/Slot.cpp
@@ -26,6 +26,7 @@ Slot::Slot(uint64 slotIndex, SCP& scp)
     , mSCP(scp)
     , mBallotProtocol(*this)
     , mNominationProtocol(*this)
+    , mFullyValidated(scp.getLocalNode()->isValidator())
 {
 }
 
@@ -148,6 +149,18 @@ Slot::nominate(Value const& value, Value const& previousValue, bool timedout)
     return mNominationProtocol.nominate(value, previousValue, timedout);
 }
 
+bool
+Slot::isFullyValidated() const
+{
+    return mFullyValidated;
+}
+
+void
+Slot::setFullyValidated(bool fullyValidated)
+{
+    mFullyValidated = fullyValidated;
+}
+
 SCPEnvelope
 Slot::createEnvelope(SCPStatement const& statement)
 {
@@ -249,6 +262,7 @@ Slot::dumpInfo(Json::Value& ret)
         slotValue["statements"][count++] = envToStr(item);
     }
 
+    slotValue["validated"] = mFullyValidated;
     mNominationProtocol.dumpInfo(slotValue);
     mBallotProtocol.dumpInfo(slotValue);
 }
diff --git a/src/scp/Slot.h b/src/scp/Slot.h
index 4645a3d689..67015a73f1 100644
--- a/src/scp/Slot.h
+++ b/src/scp/Slot.h
@@ -34,6 +34,9 @@ class Slot : public std::enable_shared_from_this<Slot>
     // it is used for debugging purpose
     std::vector<SCPStatement> mStatementsHistory;
 
+    // true if the Slot was fully validated
+    bool mFullyValidated;
+
   public:
     Slot(uint64 slotIndex, SCP& SCP);
 
@@ -100,6 +103,9 @@ class Slot : public std::enable_shared_from_this<Slot>
     bool nominate(Value const& value, Value const& previousValue,
                   bool timedout);
 
+    bool isFullyValidated() const;
+    void setFullyValidated(bool fullyValidated);
+
     // ** status methods
 
     size_t

From a891ce4de0d25d305b9a660dbc550ac181703ee6 Mon Sep 17 00:00:00 2001
From: MonsieurNicolas <nicolas@stellar.org>
Date: Mon, 26 Oct 2015 15:14:27 -0700
Subject: [PATCH 03/16] getCurrentState: only include messages from self for
 fully validated slots

---
 src/scp/BallotProtocol.cpp     | 10 ++++++++--
 src/scp/NominationProtocol.cpp | 10 ++++++++--
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/src/scp/BallotProtocol.cpp b/src/scp/BallotProtocol.cpp
index d7b2b60cc3..84074beac9 100644
--- a/src/scp/BallotProtocol.cpp
+++ b/src/scp/BallotProtocol.cpp
@@ -1430,9 +1430,15 @@ std::vector<SCPEnvelope>
 BallotProtocol::getCurrentState() const
 {
     std::vector<SCPEnvelope> res;
-    for (auto it : mLatestEnvelopes)
+    res.reserve(mLatestEnvelopes.size());
+    for (auto const& n : mLatestEnvelopes)
     {
-        res.emplace_back(it.second);
+        // only return messages for self if the slot is fully validated
+        if (!(n.first == mSlot.getSCP().getLocalNodeID()) ||
+            mSlot.isFullyValidated())
+        {
+            res.emplace_back(n.second);
+        }
     }
     return res;
 }
diff --git a/src/scp/NominationProtocol.cpp b/src/scp/NominationProtocol.cpp
index 080106ee01..572f1badc2 100644
--- a/src/scp/NominationProtocol.cpp
+++ b/src/scp/NominationProtocol.cpp
@@ -570,9 +570,15 @@ std::vector<SCPEnvelope>
 NominationProtocol::getCurrentState() const
 {
     std::vector<SCPEnvelope> res;
-    for (auto it : mLatestNominations)
+    res.reserve(mLatestNominations.size());
+    for (auto const& n : mLatestNominations)
     {
-        res.emplace_back(it.second);
+        // only return messages for self if the slot is fully validated
+        if (!(n.first == mSlot.getSCP().getLocalNodeID()) ||
+            mSlot.isFullyValidated())
+        {
+            res.emplace_back(n.second);
+        }
     }
     return res;
 }

From 0e8412e93c8119e0a4e1de7863f4b8e0f3ce4d39 Mon Sep 17 00:00:00 2001
From: MonsieurNicolas <nicolas@stellar.org>
Date: Mon, 26 Oct 2015 15:30:39 -0700
Subject: [PATCH 04/16] SCP: validate value returns a `maybe` value when value
 cannot be rejected for sure

a maybe value causes the slot to be marked as non validating
---
 src/herder/HerderImpl.cpp      | 39 +++++++++++++++++-----------------
 src/herder/HerderImpl.h        |  6 ++++--
 src/scp/BallotProtocol.cpp     |  9 +++++++-
 src/scp/NominationProtocol.cpp |  8 ++++---
 src/scp/NominationProtocol.h   |  2 +-
 src/scp/SCPDriver.h            | 19 +++++++++++++----
 src/scp/SCPTests.cpp           |  4 ++--
 7 files changed, 54 insertions(+), 33 deletions(-)

diff --git a/src/herder/HerderImpl.cpp b/src/herder/HerderImpl.cpp
index b958ab7e9d..f8ce5443ad 100644
--- a/src/herder/HerderImpl.cpp
+++ b/src/herder/HerderImpl.cpp
@@ -188,7 +188,7 @@ HerderImpl::isSlotCompatibleWithCurrentState(uint64 slotIndex)
     return res;
 }
 
-bool
+SCPDriver::ValidationLevel
 HerderImpl::validateValueHelper(uint64 slotIndex, StellarValue const& b)
 {
     uint64 lastCloseTime;
@@ -206,7 +206,7 @@ HerderImpl::validateValueHelper(uint64 slotIndex, StellarValue const& b)
         {
             // if we're not tracking, there is not much more we can do to
             // validate
-            return true;
+            return SCPDriver::kMaybeValidValue;
         }
 
         // Check slotIndex.
@@ -214,7 +214,7 @@ HerderImpl::validateValueHelper(uint64 slotIndex, StellarValue const& b)
         {
             // we already moved on from this slot
             // still send it through for emitting the final messages
-            return true;
+            return SCPDriver::kMaybeValidValue;
         }
         if (nextConsensusLedgerIndex() < slotIndex)
         {
@@ -226,7 +226,7 @@ HerderImpl::validateValueHelper(uint64 slotIndex, StellarValue const& b)
                 << " i: " << slotIndex
                 << " processing a future message while tracking";
 
-            return false;
+            return SCPDriver::kInvalidValue;
         }
         lastCloseTime = mTrackingSCP->mConsensusValue.closeTime;
     }
@@ -234,20 +234,20 @@ HerderImpl::validateValueHelper(uint64 slotIndex, StellarValue const& b)
     // Check closeTime (not too old)
     if (b.closeTime <= lastCloseTime)
     {
-        return false;
+        return SCPDriver::kInvalidValue;
     }
 
     // Check closeTime (not too far in future)
     uint64_t timeNow = mApp.timeNow();
     if (b.closeTime > timeNow + MAX_TIME_SLIP_SECONDS.count())
     {
-        return false;
+        return SCPDriver::kInvalidValue;
     }
 
     if (!compat)
     {
         // this is as far as we can go if we don't have the state
-        return true;
+        return SCPDriver::kMaybeValidValue;
     }
 
     Hash txSetHash = b.txSetHash;
@@ -256,21 +256,21 @@ HerderImpl::validateValueHelper(uint64 slotIndex, StellarValue const& b)
 
     TxSetFramePtr txSet = mPendingEnvelopes.getTxSet(txSetHash);
 
-    bool res;
+    SCPDriver::ValidationLevel res;
 
     if (!txSet)
     {
         CLOG(ERROR, "Herder") << "HerderImpl::validateValue"
                               << " i: " << slotIndex << " txSet not found?";
 
-        res = false;
+        res = SCPDriver::kInvalidValue;
     }
     else if (!txSet->checkValid(mApp))
     {
         CLOG(DEBUG, "Herder") << "HerderImpl::validateValue"
                               << " i: " << slotIndex << " Invalid txSet:"
                               << " " << hexAbbrev(txSet->getContentsHash());
-        res = false;
+        res = SCPDriver::kInvalidValue;
     }
     else
     {
@@ -278,7 +278,7 @@ HerderImpl::validateValueHelper(uint64 slotIndex, StellarValue const& b)
             << "HerderImpl::validateValue"
             << " i: " << slotIndex
             << " txSet: " << hexAbbrev(txSet->getContentsHash()) << " OK";
-        res = true;
+        res = SCPDriver::kFullyValidatedValue;
     }
     return res;
 }
@@ -361,7 +361,7 @@ HerderImpl::verifyEnvelope(SCPEnvelope const& envelope)
     return b;
 }
 
-bool
+SCPDriver::ValidationLevel
 HerderImpl::validateValue(uint64 slotIndex, Value const& value)
 {
     StellarValue b;
@@ -372,11 +372,11 @@ HerderImpl::validateValue(uint64 slotIndex, Value const& value)
     catch (...)
     {
         mSCPMetrics.mValueInvalid.Mark();
-        return false;
+        return SCPDriver::kInvalidValue;
     }
 
-    bool res = validateValueHelper(slotIndex, b);
-    if (res)
+    SCPDriver::ValidationLevel res = validateValueHelper(slotIndex, b);
+    if (res != SCPDriver::kInvalidValue)
     {
         LedgerUpgradeType lastUpgradeType = LEDGER_UPGRADE_VERSION;
         // check upgrades
@@ -387,13 +387,13 @@ HerderImpl::validateValue(uint64 slotIndex, Value const& value)
             {
                 CLOG(TRACE, "Herder")
                     << "HerderImpl::validateValue invalid step at index " << i;
-                res = false;
+                res = SCPDriver::kInvalidValue;
             }
             if (i != 0 && (lastUpgradeType >= thisUpgradeType))
             {
                 CLOG(TRACE, "Herder") << "HerderImpl::validateValue out of "
                                          "order upgrade step at index " << i;
-                res = false;
+                res = SCPDriver::kInvalidValue;
             }
 
             lastUpgradeType = thisUpgradeType;
@@ -424,10 +424,9 @@ HerderImpl::extractValidValue(uint64 slotIndex, Value const& value)
         return Value();
     }
     Value res;
-    if (validateValueHelper(slotIndex, b))
+    if (validateValueHelper(slotIndex, b) == SCPDriver::kFullyValidatedValue)
     {
-        // value was not valid because of one of the upgrade steps,
-        // remove the ones we don't like
+        // remove the upgrade steps we don't like
         LedgerUpgradeType thisUpgradeType;
         for (auto it = b.upgrades.begin(); it != b.upgrades.end();)
         {
diff --git a/src/herder/HerderImpl.h b/src/herder/HerderImpl.h
index 7ac7a8e228..d4ba2ae517 100644
--- a/src/herder/HerderImpl.h
+++ b/src/herder/HerderImpl.h
@@ -61,7 +61,8 @@ class HerderImpl : public Herder, public SCPDriver
     void signEnvelope(SCPEnvelope& envelope) override;
     bool verifyEnvelope(SCPEnvelope const& envelope) override;
 
-    bool validateValue(uint64 slotIndex, Value const& value) override;
+    SCPDriver::ValidationLevel validateValue(uint64 slotIndex,
+                                             Value const& value) override;
 
     Value extractValidValue(uint64 slotIndex, Value const& value) override;
 
@@ -141,7 +142,8 @@ class HerderImpl : public Herder, public SCPDriver
     // in which case it also sets upgradeType
     bool validateUpgradeStep(uint64 slotIndex, UpgradeType const& upgrade,
                              LedgerUpgradeType& upgradeType);
-    bool validateValueHelper(uint64 slotIndex, StellarValue const& sv);
+    SCPDriver::ValidationLevel validateValueHelper(uint64 slotIndex,
+                                                   StellarValue const& sv);
 
     void startRebroadcastTimer();
     void rebroadcast();
diff --git a/src/scp/BallotProtocol.cpp b/src/scp/BallotProtocol.cpp
index 84074beac9..91f90f8d10 100644
--- a/src/scp/BallotProtocol.cpp
+++ b/src/scp/BallotProtocol.cpp
@@ -164,13 +164,20 @@ BallotProtocol::processEnvelope(SCPEnvelope const& envelope)
 
     SCPBallot wb = getWorkingBallot(statement);
 
-    if (mSlot.getSCPDriver().validateValue(mSlot.getSlotIndex(), wb.value))
+    auto validationRes =
+        mSlot.getSCPDriver().validateValue(mSlot.getSlotIndex(), wb.value);
+    if (validationRes != SCPDriver::kInvalidValue)
     {
         bool processed = false;
         SCPBallot tickBallot = getWorkingBallot(statement);
 
         if (mPhase != SCP_PHASE_EXTERNALIZE)
         {
+            if (validationRes == SCPDriver::kMaybeValidValue)
+            {
+                mSlot.setFullyValidated(false);
+            }
+
             switch (statement.pledges.type())
             {
             case SCPStatementType::SCP_ST_PREPARE:
diff --git a/src/scp/NominationProtocol.cpp b/src/scp/NominationProtocol.cpp
index 572f1badc2..006dcf01e2 100644
--- a/src/scp/NominationProtocol.cpp
+++ b/src/scp/NominationProtocol.cpp
@@ -71,7 +71,7 @@ NominationProtocol::isSubsetHelper(xdr::xvector<Value> const& p,
     return res;
 }
 
-bool
+SCPDriver::ValidationLevel
 NominationProtocol::validateValue(Value const& v)
 {
     return mSlot.getSCPDriver().validateValue(mSlot.getSlotIndex(), v);
@@ -274,7 +274,8 @@ NominationProtocol::getNewValueFromNomination(SCPNomination const& nom)
     applyAll(nom, [&](Value const& value)
              {
                  Value valueToNominate;
-                 if (validateValue(value))
+                 auto vl = validateValue(value);
+                 if (vl == SCPDriver::kFullyValidatedValue)
                  {
                      valueToNominate = value;
                  }
@@ -340,7 +341,8 @@ NominationProtocol::processEnvelope(SCPEnvelope const& envelope)
                                       _1),
                             mLatestNominations))
                     {
-                        if (validateValue(v))
+                        auto vl = validateValue(v);
+                        if (vl == SCPDriver::kFullyValidatedValue)
                         {
                             mAccepted.emplace(v);
                             mVotes.emplace(v);
diff --git a/src/scp/NominationProtocol.h b/src/scp/NominationProtocol.h
index c8554808cb..c085148414 100644
--- a/src/scp/NominationProtocol.h
+++ b/src/scp/NominationProtocol.h
@@ -49,7 +49,7 @@ class NominationProtocol
     static bool isSubsetHelper(xdr::xvector<Value> const& p,
                                xdr::xvector<Value> const& v, bool& notEqual);
 
-    bool validateValue(Value const& v);
+    SCPDriver::ValidationLevel validateValue(Value const& v);
     Value extractValidValue(Value const& value);
 
     bool isSane(SCPStatement const& st);
diff --git a/src/scp/SCPDriver.h b/src/scp/SCPDriver.h
index cd43350600..898c5e0660 100644
--- a/src/scp/SCPDriver.h
+++ b/src/scp/SCPDriver.h
@@ -46,16 +46,27 @@ class SCPDriver
     // is done. It should be used to filter out values that are not compatible
     // with the current state of that node. Unvalidated values can never
     // externalize.
-    virtual bool
+    // If the value cannot be validated (node is missing some context) but
+    // passes
+    // the validity checks, kMaybeValidValue can be returned. This will cause
+    // the current slot to be marked as a non validating slot: the local node
+    // will abstain from emiting its position.
+    enum ValidationLevel
+    {
+        kInvalidValue,        // value is invalid for sure
+        kFullyValidatedValue, // value is valid for sure
+        kMaybeValidValue      // value may be valid
+    };
+    virtual ValidationLevel
     validateValue(uint64 slotIndex, Value const& value)
     {
-        return true;
+        return kMaybeValidValue;
     }
 
     // `extractValidValue` transforms the value, if possible to a different
-    // value that the local node would agree to.
+    // value that the local node would agree to (fully validated).
     // This is used during nomination when encountering an invalid value (ie
-    // validateValue returned `false` for this value).
+    // validateValue did not return `kFullyValidatedValue` for this value).
     // returning Value() means no valid value could be extracted
     virtual Value
     extractValidValue(uint64 slotIndex, Value const& value)
diff --git a/src/scp/SCPTests.cpp b/src/scp/SCPTests.cpp
index d099e712eb..c520e51aa0 100644
--- a/src/scp/SCPTests.cpp
+++ b/src/scp/SCPTests.cpp
@@ -67,10 +67,10 @@ class TestSCP : public SCPDriver
         mQuorumSets[qSetHash] = qSet;
     }
 
-    bool
+    SCPDriver::ValidationLevel
     validateValue(uint64 slotIndex, Value const& value) override
     {
-        return true;
+        return SCPDriver::kFullyValidatedValue;
     }
 
     void

From 7c5e72c43d65be1127a46ba9a6f85219c1eb8cbc Mon Sep 17 00:00:00 2001
From: MonsieurNicolas <nicolas@stellar.org>
Date: Mon, 26 Oct 2015 15:31:14 -0700
Subject: [PATCH 05/16] don't emit non fully validated slots

---
 src/herder/HerderImpl.cpp      | 15 ------------
 src/scp/BallotProtocol.cpp     |  5 +++-
 src/scp/NominationProtocol.cpp |  5 +++-
 src/scp/SCPTests.cpp           | 44 ++++++++++++++++++++++++++++------
 src/scp/Slot.cpp               | 11 +++++++++
 src/scp/Slot.h                 |  4 ++++
 6 files changed, 60 insertions(+), 24 deletions(-)

diff --git a/src/herder/HerderImpl.cpp b/src/herder/HerderImpl.cpp
index f8ce5443ad..47068da364 100644
--- a/src/herder/HerderImpl.cpp
+++ b/src/herder/HerderImpl.cpp
@@ -868,23 +868,8 @@ HerderImpl::startRebroadcastTimer()
 void
 HerderImpl::emitEnvelope(SCPEnvelope const& envelope)
 {
-    // this should not happen: if we're just watching consensus
-    // don't send out SCP messages
-    if (!mSCP.isValidator())
-    {
-        return;
-    }
-
     uint64 slotIndex = envelope.statement.slotIndex;
 
-    // SCP may emit envelopes as our instance changes state
-    // yet, we do not want to send those out when we don't do full validation
-    if (!isSlotCompatibleWithCurrentState(slotIndex) &&
-        (!mTrackingSCP || !mLedgerManager.isSynced()))
-    {
-        return;
-    }
-
     CLOG(DEBUG, "Herder") << "emitEnvelope"
                           << " s:" << envelope.statement.pledges.type()
                           << " i:" << slotIndex
diff --git a/src/scp/BallotProtocol.cpp b/src/scp/BallotProtocol.cpp
index 91f90f8d10..6d30001b21 100644
--- a/src/scp/BallotProtocol.cpp
+++ b/src/scp/BallotProtocol.cpp
@@ -588,7 +588,10 @@ BallotProtocol::emitCurrentStateStatement()
             isNewerStatement(mLastEnvelope->statement, envelope.statement))
         {
             mLastEnvelope = make_unique<SCPEnvelope>(envelope);
-            mSlot.getSCPDriver().emitEnvelope(envelope);
+            if (mSlot.isFullyValidated())
+            {
+                mSlot.getSCPDriver().emitEnvelope(envelope);
+            }
         }
     }
     else
diff --git a/src/scp/NominationProtocol.cpp b/src/scp/NominationProtocol.cpp
index 006dcf01e2..be7f71f03a 100644
--- a/src/scp/NominationProtocol.cpp
+++ b/src/scp/NominationProtocol.cpp
@@ -166,7 +166,10 @@ NominationProtocol::emitNomination()
                              st.pledges.nominate()))
         {
             mLastEnvelope = make_unique<SCPEnvelope>(envelope);
-            mSlot.getSCPDriver().emitEnvelope(envelope);
+            if (mSlot.isFullyValidated())
+            {
+                mSlot.getSCPDriver().emitEnvelope(envelope);
+            }
         }
     }
     else
diff --git a/src/scp/SCPTests.cpp b/src/scp/SCPTests.cpp
index c520e51aa0..d393fbdaff 100644
--- a/src/scp/SCPTests.cpp
+++ b/src/scp/SCPTests.cpp
@@ -184,6 +184,34 @@ class TestSCP : public SCPDriver
     {
         mSCP.receiveEnvelope(envelope);
     }
+
+    Slot&
+    getSlot(uint64 index)
+    {
+        return *mSCP.getSlot(index, false);
+    }
+
+    std::vector<SCPEnvelope>
+    getEntireState(uint64 index)
+    {
+        auto v = mSCP.getSlot(index, false)->getEntireCurrentState();
+        return v;
+    }
+
+    SCPEnvelope
+    getCurrentEnvelope(uint64 index, NodeID const& id)
+    {
+        auto r = getEntireState(index);
+        auto it = std::find_if(r.begin(), r.end(), [&](SCPEnvelope const& e)
+                               {
+                                   return e.statement.nodeID == id;
+                               });
+        if (it != r.end())
+        {
+            return *it;
+        }
+        throw std::runtime_error("not found");
+    }
 };
 
 static SCPEnvelope
@@ -588,8 +616,9 @@ TEST_CASE("ballot protocol core5", "[scp][ballotprotocol]")
 
         SCPBallot b(1, xValue);
         REQUIRE(scpNV.bumpState(0, xValue));
-        REQUIRE(scpNV.mEnvs.size() == 1);
-        verifyPrepare(scpNV.mEnvs[0], vNVSecretKey, qSetHash, 0, b);
+        REQUIRE(scpNV.mEnvs.size() == 0);
+        verifyPrepare(scpNV.getCurrentEnvelope(0, vNVNodeID), vNVSecretKey,
+                      qSetHash, 0, b);
         auto ext1 = makeExternalize(v1SecretKey, qSetHash, 0, b, 1);
         auto ext2 = makeExternalize(v2SecretKey, qSetHash, 0, b, 1);
         auto ext3 = makeExternalize(v3SecretKey, qSetHash, 0, b, 1);
@@ -597,12 +626,13 @@ TEST_CASE("ballot protocol core5", "[scp][ballotprotocol]")
         scpNV.receiveEnvelope(ext1);
         scpNV.receiveEnvelope(ext2);
         scpNV.receiveEnvelope(ext3);
-        REQUIRE(scpNV.mEnvs.size() == 2);
-        verifyConfirm(scpNV.mEnvs[1], vNVSecretKey, qSetHash, 0, 1, b, 1);
+        REQUIRE(scpNV.mEnvs.size() == 0);
+        verifyConfirm(scpNV.getCurrentEnvelope(0, vNVNodeID), vNVSecretKey,
+                      qSetHash, 0, 1, b, 1);
         scpNV.receiveEnvelope(ext4);
-        REQUIRE(scpNV.mEnvs.size() == 3);
-        verifyExternalize(scpNV.mEnvs[2], vNVSecretKey, qSetHash, 0, b,
-                          b.counter);
+        REQUIRE(scpNV.mEnvs.size() == 0);
+        verifyExternalize(scpNV.getCurrentEnvelope(0, vNVNodeID), vNVSecretKey,
+                          qSetHash, 0, b, b.counter);
     }
 
     SECTION("bumpState x")
diff --git a/src/scp/Slot.cpp b/src/scp/Slot.cpp
index 9c171d3812..52a39b4818 100644
--- a/src/scp/Slot.cpp
+++ b/src/scp/Slot.cpp
@@ -418,4 +418,15 @@ Slot::getLocalNode()
 {
     return mSCP.getLocalNode();
 }
+
+std::vector<SCPEnvelope>
+Slot::getEntireCurrentState()
+{
+    bool old = mFullyValidated;
+    // fake fully validated to force returning all envelopes
+    mFullyValidated = true;
+    auto r = getCurrentState();
+    mFullyValidated = old;
+    return r;
+}
 }
diff --git a/src/scp/Slot.h b/src/scp/Slot.h
index 67015a73f1..5592815c7a 100644
--- a/src/scp/Slot.h
+++ b/src/scp/Slot.h
@@ -162,5 +162,9 @@ class Slot : public std::enable_shared_from_this<Slot>
         NOMINATION_TIMER = 0,
         BALLOT_PROTOCOL_TIMER = 1
     };
+
+  protected:
+    std::vector<SCPEnvelope> getEntireCurrentState();
+    friend class TestSCP;
 };
 }

From 426b30f00a0a87158000266873445226cc5dd2ec Mon Sep 17 00:00:00 2001
From: MonsieurNicolas <nicolas@stellar.org>
Date: Mon, 26 Oct 2015 15:40:07 -0700
Subject: [PATCH 06/16] Herder: only save the latest message of the most recent
 slot

---
 src/herder/HerderImpl.cpp | 16 ++++++++++++----
 src/herder/HerderImpl.h   |  6 +++++-
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/src/herder/HerderImpl.cpp b/src/herder/HerderImpl.cpp
index 47068da364..4dfbd726fb 100644
--- a/src/herder/HerderImpl.cpp
+++ b/src/herder/HerderImpl.cpp
@@ -104,6 +104,7 @@ HerderImpl::HerderImpl(Application& app)
            app.getConfig().QUORUM_SET)
     , mReceivedTransactions(4)
     , mPendingEnvelopes(app, *this)
+    , mLastSlotSaved(0)
     , mLastStateChange(app.getClock().now())
     , mTrackingTimer(app)
     , mLastTrigger(app.getClock().now())
@@ -875,7 +876,7 @@ HerderImpl::emitEnvelope(SCPEnvelope const& envelope)
                           << " i:" << slotIndex
                           << " a:" << mApp.getStateHuman();
 
-    persistSCPState();
+    persistSCPState(slotIndex);
 
     broadcast(envelope);
 
@@ -1479,15 +1480,21 @@ HerderImpl::dumpQuorumInfo(Json::Value& ret, NodeID const& id, bool summary,
 }
 
 void
-HerderImpl::persistSCPState()
+HerderImpl::persistSCPState(uint64 slot)
 {
+    if (slot < mLastSlotSaved)
+    {
+        return;
+    }
+
+    mLastSlotSaved = slot;
+
     // saves SCP messages and related data (transaction sets, quorum sets)
     xdr::xvector<SCPEnvelope> latestEnvs;
     std::map<Hash, TxSetFramePtr> txSets;
     std::map<Hash, SCPQuorumSetPtr> quorumSets;
 
-    for (auto const& e :
-         mSCP.getLatestMessagesSend(mLedgerManager.getLedgerNum()))
+    for (auto const& e : mSCP.getLatestMessagesSend(slot))
     {
         latestEnvs.emplace_back(e);
 
@@ -1569,6 +1576,7 @@ HerderImpl::restoreSCPState()
 
     if (latestEnvs.size() != 0)
     {
+        mLastSlotSaved = latestEnvs.back().statement.slotIndex;
         startRebroadcastTimer();
     }
 }
diff --git a/src/herder/HerderImpl.h b/src/herder/HerderImpl.h
index d4ba2ae517..2e9b980de1 100644
--- a/src/herder/HerderImpl.h
+++ b/src/herder/HerderImpl.h
@@ -186,6 +186,10 @@ class HerderImpl : public Herder, public SCPDriver
     // reached consensus it can
     std::unique_ptr<ConsensusData> mTrackingSCP;
 
+    // last slot that was persisted into the database
+    // only keep track of the most recent slot
+    uint64 mLastSlotSaved;
+
     // Mark changes to mTrackingSCP in metrics.
     void stateChanged();
     VirtualClock::time_point mLastStateChange;
@@ -209,7 +213,7 @@ class HerderImpl : public Herder, public SCPDriver
     VirtualTimer mTrackingTimer;
 
     // saves the SCP messages that the instance sent out last
-    void persistSCPState();
+    void persistSCPState(uint64 slot);
 
     // called every time we get ledger externalized
     // ensures that if we don't hear from the network, we throw the herder into

From 1ec199ec4e75df9469d279585b52db859d573df6 Mon Sep 17 00:00:00 2001
From: MonsieurNicolas <nicolas@stellar.org>
Date: Mon, 26 Oct 2015 16:13:17 -0700
Subject: [PATCH 07/16] records full validation state in statement history

---
 src/scp/Slot.cpp | 8 +++++---
 src/scp/Slot.h   | 3 ++-
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/scp/Slot.cpp b/src/scp/Slot.cpp
index 52a39b4818..9e650dcb06 100644
--- a/src/scp/Slot.cpp
+++ b/src/scp/Slot.cpp
@@ -89,7 +89,7 @@ Slot::getCurrentState() const
 void
 Slot::recordStatement(SCPStatement const& st)
 {
-    mStatementsHistory.emplace_back(st);
+    mStatementsHistory.emplace_back(std::make_pair(st, mFullyValidated));
 }
 
 SCP::EnvelopeState
@@ -257,9 +257,11 @@ Slot::dumpInfo(Json::Value& ret)
     Json::Value& slotValue = slots[std::to_string(mSlotIndex)];
 
     int count = 0;
-    for (auto& item : mStatementsHistory)
+    for (auto const& item : mStatementsHistory)
     {
-        slotValue["statements"][count++] = envToStr(item);
+        Json::Value& v = slotValue["statements"][count++];
+        v.append(envToStr(item.first));
+        v.append(item.second);
     }
 
     slotValue["validated"] = mFullyValidated;
diff --git a/src/scp/Slot.h b/src/scp/Slot.h
index 5592815c7a..263fe92ece 100644
--- a/src/scp/Slot.h
+++ b/src/scp/Slot.h
@@ -32,7 +32,8 @@ class Slot : public std::enable_shared_from_this<Slot>
 
     // keeps track of all statements seen so far for this slot.
     // it is used for debugging purpose
-    std::vector<SCPStatement> mStatementsHistory;
+    // second: if the slot was fully validated at the time
+    std::vector<std::pair<SCPStatement, bool>> mStatementsHistory;
 
     // true if the Slot was fully validated
     bool mFullyValidated;

From 024e5941ec1b168eb5443181dc93e5eaf3a4da22 Mon Sep 17 00:00:00 2001
From: MonsieurNicolas <nicolas@stellar.org>
Date: Tue, 27 Oct 2015 12:49:00 -0700
Subject: [PATCH 08/16] cleanup metrics in Herder

---
 src/herder/HerderImpl.cpp | 27 +++++++++------------------
 src/herder/HerderImpl.h   |  6 ------
 2 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/src/herder/HerderImpl.cpp b/src/herder/HerderImpl.cpp
index 4dfbd726fb..04c26b5f6d 100644
--- a/src/herder/HerderImpl.cpp
+++ b/src/herder/HerderImpl.cpp
@@ -75,9 +75,6 @@ HerderImpl::SCPMetrics::SCPMetrics(Application& app)
     , mEnvelopeInvalidSig(app.getMetrics().NewMeter(
           {"scp", "envelope", "invalidsig"}, "envelope"))
 
-    , mBallotValidationTimersSize(app.getMetrics().NewCounter(
-          {"scp", "memory", "ballot-validation-timers"}))
-
     , mKnownSlotsSize(
           app.getMetrics().NewCounter({"scp", "memory", "known-slots"}))
     , mCumulativeStatements(app.getMetrics().NewCounter(
@@ -630,14 +627,6 @@ HerderImpl::valueExternalized(uint64 slotIndex, Value const& value)
     }
 
     assert(mReceivedTransactions.size() >= 4);
-    mSCPMetrics.mHerderPendingTxs0.set_count(
-        countTxs(mReceivedTransactions[0]));
-    mSCPMetrics.mHerderPendingTxs1.set_count(
-        countTxs(mReceivedTransactions[1]));
-    mSCPMetrics.mHerderPendingTxs2.set_count(
-        countTxs(mReceivedTransactions[2]));
-    mSCPMetrics.mHerderPendingTxs3.set_count(
-        countTxs(mReceivedTransactions[3]));
 
     // Move all the remaining to the next highest level don't move the
     // largest array.
@@ -658,6 +647,15 @@ HerderImpl::valueExternalized(uint64 slotIndex, Value const& value)
         prev.clear();
     }
 
+    mSCPMetrics.mHerderPendingTxs0.set_count(
+        countTxs(mReceivedTransactions[0]));
+    mSCPMetrics.mHerderPendingTxs1.set_count(
+        countTxs(mReceivedTransactions[1]));
+    mSCPMetrics.mHerderPendingTxs2.set_count(
+        countTxs(mReceivedTransactions[2]));
+    mSCPMetrics.mHerderPendingTxs3.set_count(
+        countTxs(mReceivedTransactions[3]));
+
     ledgerClosed();
 }
 
@@ -1130,13 +1128,6 @@ HerderImpl::ledgerClosed()
 
     mApp.getOverlayManager().ledgerClosed(lastConsensusLedgerIndex());
 
-    // As the current slotIndex changes we cancel all pending validation
-    // timers. Since the value externalized, the messages that this generates
-    // wont' have any impact.
-    mBallotValidationTimers.clear();
-    mSCPMetrics.mBallotValidationTimersSize.set_count(
-        mBallotValidationTimers.size());
-
     uint64_t nextIndex = nextConsensusLedgerIndex();
 
     // process any statements up to this slot (this may trigger externalize)
diff --git a/src/herder/HerderImpl.h b/src/herder/HerderImpl.h
index 2e9b980de1..3e56db611c 100644
--- a/src/herder/HerderImpl.h
+++ b/src/herder/HerderImpl.h
@@ -164,10 +164,6 @@ class HerderImpl : public Herder, public SCPDriver
 
     PendingEnvelopes mPendingEnvelopes;
 
-    std::map<SCPBallot,
-             std::map<NodeID, std::vector<std::shared_ptr<VirtualTimer>>>>
-        mBallotValidationTimers;
-
     void herderOutOfSync();
 
     struct ConsensusData
@@ -258,8 +254,6 @@ class HerderImpl : public Herder, public SCPDriver
         medida::Meter& mEnvelopeValidSig;
         medida::Meter& mEnvelopeInvalidSig;
 
-        medida::Counter& mBallotValidationTimersSize;
-
         // Counters for stuff in parent class (SCP)
         // that we monitor on a best-effort basis from
         // here.

From 6d8c7612de9f2c39474d06a35cf644cd8e2e5f30 Mon Sep 17 00:00:00 2001
From: MonsieurNicolas <nicolas@stellar.org>
Date: Tue, 27 Oct 2015 12:50:48 -0700
Subject: [PATCH 09/16] SCP: don't return messages sent for non validated slots

---
 src/scp/Slot.cpp | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/scp/Slot.cpp b/src/scp/Slot.cpp
index 9e650dcb06..2ad44f5031 100644
--- a/src/scp/Slot.cpp
+++ b/src/scp/Slot.cpp
@@ -40,16 +40,19 @@ std::vector<SCPEnvelope>
 Slot::getLatestMessagesSend() const
 {
     std::vector<SCPEnvelope> res;
-    SCPEnvelope* e;
-    e = mNominationProtocol.getLastMessageSend();
-    if (e)
+    if (mFullyValidated)
     {
-        res.emplace_back(*e);
-    }
-    e = mBallotProtocol.getLastMessageSend();
-    if (e)
-    {
-        res.emplace_back(*e);
+        SCPEnvelope* e;
+        e = mNominationProtocol.getLastMessageSend();
+        if (e)
+        {
+            res.emplace_back(*e);
+        }
+        e = mBallotProtocol.getLastMessageSend();
+        if (e)
+        {
+            res.emplace_back(*e);
+        }
     }
     return res;
 }

From 2103e676328ed8583b61bef957831dfa72b6093b Mon Sep 17 00:00:00 2001
From: MonsieurNicolas <nicolas@stellar.org>
Date: Tue, 27 Oct 2015 12:53:09 -0700
Subject: [PATCH 10/16] enforce that slots get externalized in strictly
 monotonic fashion

---
 src/herder/HerderImpl.cpp | 33 +++++++++++++++++++++++----------
 src/herder/HerderImpl.h   |  6 +++++-
 2 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/src/herder/HerderImpl.cpp b/src/herder/HerderImpl.cpp
index 04c26b5f6d..56d428eeef 100644
--- a/src/herder/HerderImpl.cpp
+++ b/src/herder/HerderImpl.cpp
@@ -540,6 +540,18 @@ HerderImpl::valueExternalized(uint64 slotIndex, Value const& value)
     updateSCPCounters();
     mSCPMetrics.mValueExternalize.Mark();
     mSCPTimers.erase(slotIndex); // cancels all timers for this slot
+    if (slotIndex <= getCurrentLedgerSeq())
+    {
+        // externalize may trigger on older slots:
+        //  * when the current instance starts up
+        //  * when getting back in sync (a gap potentially opened)
+        // in both cases it's safe to just ignore those as we're already
+        // tracking a more recent state
+        CLOG(DEBUG, "Herder") << "Ignoring old ledger externalize "
+                              << slotIndex;
+        return;
+    }
+
     StellarValue b;
     try
     {
@@ -574,11 +586,6 @@ HerderImpl::valueExternalized(uint64 slotIndex, Value const& value)
     {
         stateChanged();
     }
-    else if (slotIndex <= mTrackingSCP->mConsensusIndex)
-    {
-        // don't do anything for older slots
-        return;
-    }
 
     mTrackingSCP = make_unique<ConsensusData>(slotIndex, b);
     trackingHeartBeat();
@@ -1262,14 +1269,17 @@ HerderImpl::getQSet(const Hash& qSetHash)
 uint32_t
 HerderImpl::getCurrentLedgerSeq() const
 {
-    if (mTrackingSCP)
+    uint32_t res = mLedgerManager.getLastClosedLedgerNum();
+
+    if (mTrackingSCP && res < mTrackingSCP->mConsensusIndex)
     {
-        return static_cast<uint32_t>(mTrackingSCP->mConsensusIndex);
+        res = static_cast<uint32_t>(mTrackingSCP->mConsensusIndex);
     }
-    else
+    if (mLastTrackingSCP && res < mLastTrackingSCP->mConsensusIndex)
     {
-        return mLedgerManager.getLastClosedLedgerNum();
+        res = static_cast<uint32_t>(mLastTrackingSCP->mConsensusIndex);
     }
+    return res;
 }
 
 SequenceNumber
@@ -1593,7 +1603,10 @@ HerderImpl::herderOutOfSync()
     CLOG(INFO, "Herder") << "Lost track of consensus";
     mSCPMetrics.mLostSync.Mark();
     stateChanged();
-    mTrackingSCP.reset();
+
+    // transfer ownership to mLastTrackingSCP
+    mLastTrackingSCP.reset(mTrackingSCP.release());
+
     processSCPQueue();
 }
 }
diff --git a/src/herder/HerderImpl.h b/src/herder/HerderImpl.h
index 3e56db611c..13e21aa730 100644
--- a/src/herder/HerderImpl.h
+++ b/src/herder/HerderImpl.h
@@ -179,9 +179,13 @@ class HerderImpl : public Herder, public SCPDriver
     // if the local instance is tracking the current state of SCP
     // herder keeps track of the consensus index and ballot
     // when not set, it just means that herder will try to snap to any slot that
-    // reached consensus it can
+    // reached consensus
     std::unique_ptr<ConsensusData> mTrackingSCP;
 
+    // when losing track of consensus, records where we left off so that we
+    // ignore older ledgers (as we potentially receive old messages)
+    std::unique_ptr<ConsensusData> mLastTrackingSCP;
+
     // last slot that was persisted into the database
     // only keep track of the most recent slot
     uint64 mLastSlotSaved;

From f43478e92d8b5ef9557638a08c6e610fb082c481 Mon Sep 17 00:00:00 2001
From: MonsieurNicolas <nicolas@stellar.org>
Date: Tue, 27 Oct 2015 12:56:15 -0700
Subject: [PATCH 11/16] herder: better tracking of SCP timers

erase / don't create timers for old slots
---
 src/herder/HerderImpl.cpp | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/herder/HerderImpl.cpp b/src/herder/HerderImpl.cpp
index 56d428eeef..f530dbdf25 100644
--- a/src/herder/HerderImpl.cpp
+++ b/src/herder/HerderImpl.cpp
@@ -217,8 +217,7 @@ HerderImpl::validateValueHelper(uint64 slotIndex, StellarValue const& b)
         if (nextConsensusLedgerIndex() < slotIndex)
         {
             // this is probably a bug as "tracking" means we're processing
-            // messages
-            // only for the right slot
+            // messages only for smaller slots
             CLOG(ERROR, "Herder")
                 << "HerderImpl::validateValue"
                 << " i: " << slotIndex
@@ -539,7 +538,13 @@ HerderImpl::valueExternalized(uint64 slotIndex, Value const& value)
 {
     updateSCPCounters();
     mSCPMetrics.mValueExternalize.Mark();
-    mSCPTimers.erase(slotIndex); // cancels all timers for this slot
+
+    auto it = mSCPTimers.begin(); // cancel all timers below this slot
+    while (it != mSCPTimers.end() && it->first <= slotIndex)
+    {
+        it = mSCPTimers.erase(it);
+    }
+
     if (slotIndex <= getCurrentLedgerSeq())
     {
         // externalize may trigger on older slots:
@@ -814,7 +819,7 @@ HerderImpl::setupTimer(uint64 slotIndex, int timerID,
                        std::function<void()> cb)
 {
     // don't setup timers for old slots
-    if (mTrackingSCP && slotIndex < mTrackingSCP->mConsensusIndex)
+    if (slotIndex <= getCurrentLedgerSeq())
     {
         mSCPTimers.erase(slotIndex);
         return;

From 3965e985e48b9abd5e71e7820822137ddd7902ae Mon Sep 17 00:00:00 2001
From: MonsieurNicolas <nicolas@stellar.org>
Date: Tue, 27 Oct 2015 12:57:29 -0700
Subject: [PATCH 12/16] Herder: ignore old SCP messages even when out of sync

---
 src/herder/HerderImpl.cpp | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/src/herder/HerderImpl.cpp b/src/herder/HerderImpl.cpp
index f530dbdf25..2d87369000 100644
--- a/src/herder/HerderImpl.cpp
+++ b/src/herder/HerderImpl.cpp
@@ -1010,28 +1010,30 @@ HerderImpl::recvSCPEnvelope(SCPEnvelope const& envelope)
 
     mSCPMetrics.mEnvelopeReceive.Mark();
 
+    uint32_t minLedgerSeq = getCurrentLedgerSeq();
+    if (minLedgerSeq > MAX_SLOTS_TO_REMEMBER)
+    {
+        minLedgerSeq -= MAX_SLOTS_TO_REMEMBER;
+    }
+
+    uint32_t maxLedgerSeq = std::numeric_limits<uint32>::max();
+
     if (mTrackingSCP)
     {
         // when tracking, we can filter messages based on the information we got
-        // from consensus
-        uint32_t minLedgerSeq = nextConsensusLedgerIndex();
-        if (minLedgerSeq > MAX_SLOTS_TO_REMEMBER)
-        {
-            minLedgerSeq -= MAX_SLOTS_TO_REMEMBER;
-        }
-        uint32_t maxLedgerSeq =
-            nextConsensusLedgerIndex() + LEDGER_VALIDITY_BRACKET;
+        // from consensus for the max ledger
 
-        // If we are fully synced and the envelopes are out of our validity
-        // brackets, we just ignore them.
-        if (envelope.statement.slotIndex > maxLedgerSeq ||
-            envelope.statement.slotIndex < minLedgerSeq)
-        {
-            CLOG(DEBUG, "Herder") << "Ignoring SCPEnvelope outside of range: "
-                                  << envelope.statement.slotIndex << "( "
-                                  << minLedgerSeq << "," << maxLedgerSeq << ")";
-            return;
-        }
+        maxLedgerSeq = nextConsensusLedgerIndex() + LEDGER_VALIDITY_BRACKET;
+    }
+
+    // If envelopes are out of our validity brackets, we just ignore them.
+    if (envelope.statement.slotIndex > maxLedgerSeq ||
+        envelope.statement.slotIndex < minLedgerSeq)
+    {
+        CLOG(DEBUG, "Herder") << "Ignoring SCPEnvelope outside of range: "
+                              << envelope.statement.slotIndex << "( "
+                              << minLedgerSeq << "," << maxLedgerSeq << ")";
+        return;
     }
 
     mPendingEnvelopes.recvSCPEnvelope(envelope);

From d7c8ac370f3f2a0662f4af8e63aee152d269fc39 Mon Sep 17 00:00:00 2001
From: MonsieurNicolas <nicolas@stellar.org>
Date: Tue, 27 Oct 2015 13:00:09 -0700
Subject: [PATCH 13/16] Herder: reduce SCP validity window

as Herder keeps times out consensus after some time, there is no need for a huge future window
---
 src/herder/Herder.cpp     | 4 +++-
 src/herder/HerderImpl.cpp | 4 ++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/herder/Herder.cpp b/src/herder/Herder.cpp
index b541116eac..9f2af0a4f3 100755
--- a/src/herder/Herder.cpp
+++ b/src/herder/Herder.cpp
@@ -9,6 +9,8 @@ std::chrono::seconds const Herder::MAX_SCP_TIMEOUT_SECONDS(240);
 std::chrono::seconds const Herder::CONSENSUS_STUCK_TIMEOUT_SECONDS(35);
 std::chrono::seconds const Herder::MAX_TIME_SLIP_SECONDS(60);
 std::chrono::seconds const Herder::NODE_EXPIRATION_SECONDS(240);
-uint32 const Herder::LEDGER_VALIDITY_BRACKET = 1000;
+// the value of LEDGER_VALIDITY_BRACKET should be in the order of
+// how many ledgers can close ahead given CONSENSUS_STUCK_TIMEOUT_SECONDS
+uint32 const Herder::LEDGER_VALIDITY_BRACKET = 100;
 uint32 const Herder::MAX_SLOTS_TO_REMEMBER = 4;
 }
diff --git a/src/herder/HerderImpl.cpp b/src/herder/HerderImpl.cpp
index 2d87369000..a9b613e5d1 100644
--- a/src/herder/HerderImpl.cpp
+++ b/src/herder/HerderImpl.cpp
@@ -1023,6 +1023,10 @@ HerderImpl::recvSCPEnvelope(SCPEnvelope const& envelope)
         // when tracking, we can filter messages based on the information we got
         // from consensus for the max ledger
 
+        // note that this filtering will cause a node started with force scp
+        // to potentially drop messages outside of the bracket
+        // (so in general, nodes should not be started with force scp if
+        // their state is very old)
         maxLedgerSeq = nextConsensusLedgerIndex() + LEDGER_VALIDITY_BRACKET;
     }
 

From e0c35130a22b4fc07eb318cab227bbbe9ad9f876 Mon Sep 17 00:00:00 2001
From: MonsieurNicolas <nicolas@stellar.org>
Date: Tue, 27 Oct 2015 13:01:22 -0700
Subject: [PATCH 14/16] herder: include latest SCP messages when exchanging SCP
 state

---
 src/herder/HerderImpl.cpp | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/src/herder/HerderImpl.cpp b/src/herder/HerderImpl.cpp
index a9b613e5d1..b799170ebb 100644
--- a/src/herder/HerderImpl.cpp
+++ b/src/herder/HerderImpl.cpp
@@ -1051,14 +1051,18 @@ HerderImpl::sendSCPStateToPeer(uint32 ledgerSeq, PeerPtr peer)
     if (ledgerSeq == 0)
     {
         const uint32 nbLedgers = 3;
-        maxSeq = getCurrentLedgerSeq();
-        if (maxSeq >= 2 + nbLedgers)
+        const uint32 minLedger = 2;
+
+        // include the most recent slot
+        maxSeq = getCurrentLedgerSeq() + 1;
+
+        if (maxSeq >= minLedger + nbLedgers)
         {
             minSeq = maxSeq - nbLedgers;
         }
         else
         {
-            minSeq = 2;
+            minSeq = minLedger;
         }
     }
     else
@@ -1070,17 +1074,18 @@ HerderImpl::sendSCPStateToPeer(uint32 ledgerSeq, PeerPtr peer)
     {
         auto const& envelopes = mSCP.getCurrentState(seq);
 
-        CLOG(DEBUG, "Herder") << "Send state " << envelopes.size()
-                              << " for ledger " << seq;
-
-        for (auto const& e : envelopes)
+        if (envelopes.size() != 0)
         {
-            StellarMessage m;
-            m.type(SCP_MESSAGE);
-            m.envelope() = e;
+            CLOG(DEBUG, "Herder") << "Send state " << envelopes.size()
+                                  << " for ledger " << seq;
 
-            mSCPMetrics.mEnvelopeEmit.Mark();
-            peer->sendMessage(m);
+            for (auto const& e : envelopes)
+            {
+                StellarMessage m;
+                m.type(SCP_MESSAGE);
+                m.envelope() = e;
+                peer->sendMessage(m);
+            }
         }
     }
 }
@@ -1403,11 +1408,10 @@ HerderImpl::triggerNextLedger(uint32_t ledgerSeqToTrigger)
             mApp.getConfig().DESIRED_MAX_TX_PER_LEDGER;
     }
 
-    UpgradeType ut; // only used for max size check
     for (auto const& upgrade : upgrades)
     {
         Value v(xdr::xdr_to_opaque(upgrade));
-        if (v.size() >= ut.max_size())
+        if (v.size() >= UpgradeType::max_size())
         {
             CLOG(ERROR, "Herder") << "HerderImpl::triggerNextLedger"
                                   << " exceeded size for upgrade step (got "

From ab7ea0678f4035f4d84588c9d29cd056be90137a Mon Sep 17 00:00:00 2001
From: MonsieurNicolas <nicolas@stellar.org>
Date: Tue, 27 Oct 2015 16:15:59 -0700
Subject: [PATCH 15/16] pass const Hash references in a few places

---
 src/herder/Herder.h             |  7 ++++---
 src/herder/HerderImpl.cpp       | 10 +++++-----
 src/herder/HerderImpl.h         |  8 ++++----
 src/herder/PendingEnvelopes.cpp |  4 ++--
 src/herder/PendingEnvelopes.h   |  6 ++----
 5 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/src/herder/Herder.h b/src/herder/Herder.h
index bbfe96963b..be6683cb1e 100644
--- a/src/herder/Herder.h
+++ b/src/herder/Herder.h
@@ -81,13 +81,14 @@ class Herder
     // restores SCP state based on the last messages saved on disk
     virtual void restoreSCPState() = 0;
 
-    virtual void recvSCPQuorumSet(Hash hash, SCPQuorumSet const& qset) = 0;
-    virtual void recvTxSet(Hash hash, TxSetFrame const& txset) = 0;
+    virtual void recvSCPQuorumSet(Hash const& hash,
+                                  SCPQuorumSet const& qset) = 0;
+    virtual void recvTxSet(Hash const& hash, TxSetFrame const& txset) = 0;
     // We are learning about a new transaction.
     virtual TransactionSubmitStatus recvTransaction(TransactionFramePtr tx) = 0;
     virtual void peerDoesntHave(stellar::MessageType type,
                                 uint256 const& itemID, PeerPtr peer) = 0;
-    virtual TxSetFramePtr getTxSet(Hash hash) = 0;
+    virtual TxSetFramePtr getTxSet(Hash const& hash) = 0;
     virtual SCPQuorumSetPtr getQSet(Hash const& qSetHash) = 0;
 
     // We are learning about a new envelope.
diff --git a/src/herder/HerderImpl.cpp b/src/herder/HerderImpl.cpp
index b799170ebb..356a1bdc39 100644
--- a/src/herder/HerderImpl.cpp
+++ b/src/herder/HerderImpl.cpp
@@ -247,7 +247,7 @@ HerderImpl::validateValueHelper(uint64 slotIndex, StellarValue const& b)
         return SCPDriver::kMaybeValidValue;
     }
 
-    Hash txSetHash = b.txSetHash;
+    Hash const& txSetHash = b.txSetHash;
 
     // we are fully synced up
 
@@ -1251,13 +1251,13 @@ HerderImpl::removeReceivedTxs(std::vector<TransactionFramePtr> const& dropTxs)
 }
 
 void
-HerderImpl::recvSCPQuorumSet(Hash hash, const SCPQuorumSet& qset)
+HerderImpl::recvSCPQuorumSet(Hash const& hash, const SCPQuorumSet& qset)
 {
     mPendingEnvelopes.recvSCPQuorumSet(hash, qset);
 }
 
 void
-HerderImpl::recvTxSet(Hash hash, const TxSetFrame& t)
+HerderImpl::recvTxSet(Hash const& hash, const TxSetFrame& t)
 {
     TxSetFramePtr txset(new TxSetFrame(t));
     mPendingEnvelopes.recvTxSet(hash, txset);
@@ -1271,13 +1271,13 @@ HerderImpl::peerDoesntHave(MessageType type, uint256 const& itemID,
 }
 
 TxSetFramePtr
-HerderImpl::getTxSet(Hash hash)
+HerderImpl::getTxSet(Hash const& hash)
 {
     return mPendingEnvelopes.getTxSet(hash);
 }
 
 SCPQuorumSetPtr
-HerderImpl::getQSet(const Hash& qSetHash)
+HerderImpl::getQSet(Hash const& qSetHash)
 {
     return mPendingEnvelopes.getQSet(qSetHash);
 }
diff --git a/src/herder/HerderImpl.h b/src/herder/HerderImpl.h
index 13e21aa730..b84e5d3f92 100644
--- a/src/herder/HerderImpl.h
+++ b/src/herder/HerderImpl.h
@@ -101,12 +101,12 @@ class HerderImpl : public Herder, public SCPDriver
 
     void sendSCPStateToPeer(uint32 ledgerSeq, PeerPtr peer) override;
 
-    void recvSCPQuorumSet(Hash hash, const SCPQuorumSet& qset) override;
-    void recvTxSet(Hash hash, const TxSetFrame& txset) override;
+    void recvSCPQuorumSet(Hash const& hash, const SCPQuorumSet& qset) override;
+    void recvTxSet(Hash const& hash, const TxSetFrame& txset) override;
     void peerDoesntHave(MessageType type, uint256 const& itemID,
                         PeerPtr peer) override;
-    TxSetFramePtr getTxSet(Hash hash) override;
-    SCPQuorumSetPtr getQSet(const Hash& qSetHash) override;
+    TxSetFramePtr getTxSet(Hash const& hash) override;
+    SCPQuorumSetPtr getQSet(Hash const& qSetHash) override;
 
     void processSCPQueue();
 
diff --git a/src/herder/PendingEnvelopes.cpp b/src/herder/PendingEnvelopes.cpp
index c3f41a4846..e27af589da 100755
--- a/src/herder/PendingEnvelopes.cpp
+++ b/src/herder/PendingEnvelopes.cpp
@@ -264,7 +264,7 @@ PendingEnvelopes::slotClosed(uint64 slotIndex)
 }
 
 TxSetFramePtr
-PendingEnvelopes::getTxSet(Hash hash)
+PendingEnvelopes::getTxSet(Hash const& hash)
 {
     if (mTxSetCache.exists(hash))
     {
@@ -275,7 +275,7 @@ PendingEnvelopes::getTxSet(Hash hash)
 }
 
 SCPQuorumSetPtr
-PendingEnvelopes::getQSet(Hash hash)
+PendingEnvelopes::getQSet(Hash const& hash)
 {
     if (mQsetCache.exists(hash))
     {
diff --git a/src/herder/PendingEnvelopes.h b/src/herder/PendingEnvelopes.h
index bf390c96bc..bbabbbabe1 100755
--- a/src/herder/PendingEnvelopes.h
+++ b/src/herder/PendingEnvelopes.h
@@ -70,11 +70,9 @@ class PendingEnvelopes
 
     std::vector<uint64> readySlots();
 
-    bool isFutureCommitted(uint64 slotIndex);
-
     void dumpInfo(Json::Value& ret);
 
-    TxSetFramePtr getTxSet(Hash hash);
-    SCPQuorumSetPtr getQSet(Hash hash);
+    TxSetFramePtr getTxSet(Hash const& hash);
+    SCPQuorumSetPtr getQSet(Hash const& hash);
 };
 }

From da228f162b0b006e165d47ef7ca8bd56260857d3 Mon Sep 17 00:00:00 2001
From: MonsieurNicolas <nicolas@stellar.org>
Date: Tue, 27 Oct 2015 16:57:04 -0700
Subject: [PATCH 16/16] don't mutate txSet cache

---
 src/herder/HerderImpl.cpp | 30 ++++++++++++++++++------------
 src/herder/TxSetFrame.h   |  3 +++
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/src/herder/HerderImpl.cpp b/src/herder/HerderImpl.cpp
index 356a1bdc39..2d536f6645 100644
--- a/src/herder/HerderImpl.cpp
+++ b/src/herder/HerderImpl.cpp
@@ -764,24 +764,30 @@ HerderImpl::combineCandidates(uint64 slotIndex,
 
     // take the txSet with the highest number of transactions,
     // highest xored hash that we have
-    Hash highest;
     TxSetFramePtr bestTxSet;
-    for (auto const& sv : candidateValues)
     {
-        TxSetFramePtr cTxSet = getTxSet(sv.txSetHash);
-
-        if (cTxSet && cTxSet->previousLedgerHash() == lcl.hash)
+        Hash highest;
+        TxSetFramePtr highestTxSet;
+        for (auto const& sv : candidateValues)
         {
-            if (!bestTxSet || (cTxSet->mTransactions.size() >
-                               bestTxSet->mTransactions.size()) ||
-                ((cTxSet->mTransactions.size() ==
-                  bestTxSet->mTransactions.size()) &&
-                 lessThanXored(highest, sv.txSetHash, candidatesHash)))
+            TxSetFramePtr cTxSet = getTxSet(sv.txSetHash);
+
+            if (cTxSet && cTxSet->previousLedgerHash() == lcl.hash)
             {
-                bestTxSet = cTxSet;
-                highest = sv.txSetHash;
+                if (!highestTxSet || (cTxSet->mTransactions.size() >
+                                      highestTxSet->mTransactions.size()) ||
+                    ((cTxSet->mTransactions.size() ==
+                      highestTxSet->mTransactions.size()) &&
+                     lessThanXored(highest, sv.txSetHash, candidatesHash)))
+                {
+                    highestTxSet = cTxSet;
+                    highest = sv.txSetHash;
+                }
             }
         }
+        // make a copy as we're about to modify it and we don't want to mess
+        // with the txSet cache
+        bestTxSet = std::make_shared<TxSetFrame>(*highestTxSet);
     }
 
     for (auto const& upgrade : upgrades)
diff --git a/src/herder/TxSetFrame.h b/src/herder/TxSetFrame.h
index 5f343a6464..3612b556e0 100644
--- a/src/herder/TxSetFrame.h
+++ b/src/herder/TxSetFrame.h
@@ -25,6 +25,9 @@ class TxSetFrame
     std::vector<TransactionFramePtr> mTransactions;
 
     TxSetFrame(Hash const& previousLedgerHash);
+
+    TxSetFrame(TxSetFrame const& other) = default;
+
     // make it from the wire
     TxSetFrame(Hash const& networkID, TransactionSet const& xdrSet);