Refactor computation of Ledger hash

src/ripple/app/ledger/Ledger.cpp

@@ -200,8 +200,7 @@ Ledger::Ledger (Ledger const& ledger,
 }
 
 // Create a new open ledger that follows this one
-Ledger::Ledger (bool /* dummy */,
+Ledger::Ledger (open_ledger_t, Ledger const& prevLedger)
-                Ledger& prevLedger)
     : mImmutable (false)
     , txMap_ (std::make_shared <SHAMap> (SHAMapType::TRANSACTION,
         getApp().family(), deprecatedLogs().journal("SHAMap")))
@@ -216,28 +215,17 @@ Ledger::Ledger (bool /* dummy */,
     info_.hash = prevLedger.info().hash + uint256(1);
     info_.drops = prevLedger.info().drops;
     info_.closeTimeResolution = prevLedger.info_.closeTimeResolution;
-
-    prevLedger.updateHash ();
-
-    // VFALCO TODO Require callers to update the hash
     info_.parentHash = prevLedger.getHash ();
-
-    assert (info_.parentHash.isNonZero ());
-
     info_.closeTimeResolution = getNextLedgerTimeResolution (
         prevLedger.info_.closeTimeResolution,
         getCloseAgree(prevLedger.info()), info_.seq);
-
+    // VFALCO Remove this call to getApp
     if (prevLedger.info_.closeTime == 0)
-    {
         info_.closeTime = roundCloseTime (
             getApp().getOPs ().getCloseTimeNC (), info_.closeTimeResolution);
-    }
     else
-    {
         info_.closeTime =
             prevLedger.info_.closeTime + info_.closeTimeResolution;
-    }
 }
 
 Ledger::Ledger (void const* data,
@@ -381,14 +369,6 @@ bool Ledger::addSLE (SLE const& sle)
     return stateMap_->addItem(item, false, false);
 }
 
-uint256 const&
-Ledger::getHash()
-{
-    if (! mValidHash)
-        updateHash();
-    return info_.hash;
-}
-
 bool Ledger::saveValidatedLedger (bool current)
 {
     // TODO(tom): Fix this hard-coded SQL!
@@ -1010,9 +990,6 @@ Ledger::rawTxInsert (uint256 const& key,
                 std::move(item), true, false))
             LogicError("duplicate_tx: " + to_string(key));
     }
-
-    // VFALCO TODO We could touch only the txMap
-    touch();
 }
 
 std::shared_ptr<SLE>

src/ripple/app/ledger/Ledger.h

@@ -110,9 +110,13 @@ public:
     Ledger (void const* data,
         std::size_t size, bool hasPrefix);
 
-    // Create a new ledger that follows this one
+    /** Create a new open ledger
-    // VFALCO `previous` should be const
+
-    Ledger (bool dummy, Ledger& previous);
+        The ledger will have the sequence number that
+        follows previous, and have
+        parentCloseTime == previous.closeTime.
+    */
+    Ledger (open_ledger_t, Ledger const& previous);
 
     // Create a new ledger that's a snapshot of this one
     Ledger (Ledger const& target, bool isMutable);
@@ -198,18 +202,6 @@ public:
 
     //--------------------------------------------------------------------------
 
-    /** Hint that the contents have changed.
-        Thread Safety:
-            Not thread safe
-        Effects:
-            The next call to getHash will return updated hashes
-    */
-    void
-    touch()
-    {
-        mValidHash = false;
-    }
-
     void setClosed()
     {
         info_.open = false;
@@ -245,11 +237,13 @@ public:
     void addRaw (Serializer& s) const;
     void setRaw (SerialIter& sit, bool hasPrefix);
 
-    /** Return the hash of the ledger.
+    // DEPRECATED
-        This will recalculate the hash if necessary.
+    // Remove contract.h include
-    */
     uint256 const&
-    getHash();
+    getHash() const
+    {
+        return info_.hash;
+    }
 
     void setTotalDrops (std::uint64_t totDrops)
     {

src/ripple/app/ledger/impl/LedgerConsensusImp.cpp

@@ -979,7 +979,8 @@ void LedgerConsensusImp::accept (std::shared_ptr<SHAMap> set)
     CanonicalTXSet retriableTransactions (set->getHash ());
 
     // Build the new last closed ledger
-    auto newLCL = std::make_shared<Ledger> (false, *mPreviousLedger);
+    auto newLCL = std::make_shared<Ledger>(
+        open_ledger, *mPreviousLedger);
     newLCL->setClosed (); // so applyTransactions sees a closed ledger
 
     // Set up to write SHAMap changes to our database,
@@ -1075,7 +1076,8 @@ void LedgerConsensusImp::accept (std::shared_ptr<SHAMap> set)
     ledgerMaster_.consensusBuilt (newLCL);
 
     // Build new open ledger
-    auto newOL = std::make_shared<Ledger> (true, *newLCL);
+    auto newOL = std::make_shared<Ledger>(
+        open_ledger, *newLCL);
     OpenView accum(&*newOL);
     assert(accum.open());
 

src/ripple/app/main/Application.cpp

@@ -1045,17 +1045,18 @@ private:
 
 void ApplicationImp::startGenesisLedger ()
 {
-    auto const genesis = std::make_shared<Ledger>(
+    std::shared_ptr<Ledger const> const genesis =
+        std::make_shared<Ledger>(
             create_genesis, getConfig());
-    auto const next =
+    auto const next = std::make_shared<Ledger>(
-        std::make_shared<Ledger>(true, *genesis);
+        open_ledger, *genesis);
     next->setClosed ();
     next->setAccepted ();
     m_networkOPs->setLastCloseTime (next->info().closeTime);
     openLedger_.emplace(next, getConfig(),
         cachedSLEs_, deprecatedLogs().journal("OpenLedger"));
     m_ledgerMaster->pushLedger (next,
-        std::make_shared<Ledger>(true, *next));
+        std::make_shared<Ledger>(open_ledger, *next));
 }
 
 Ledger::pointer
@@ -1301,7 +1302,8 @@ bool ApplicationImp::loadOldLedger (
 
         m_ledgerMaster->setLedgerRangePresent (loadLedger->info().seq, loadLedger->info().seq);
 
-        Ledger::pointer openLedger = std::make_shared<Ledger> (false, std::ref (*loadLedger));
+        auto const openLedger =
+            std::make_shared<Ledger>(open_ledger, *loadLedger);
         m_ledgerMaster->switchLedgers (loadLedger, openLedger);
         m_ledgerMaster->forceValid(loadLedger);
         m_networkOPs->setLastCloseTime (loadLedger->info().closeTime);

src/ripple/app/misc/NetworkOPs.cpp

@@ -1365,7 +1365,7 @@ void NetworkOPsImp::switchLastClosedLedger (
     clearNeedNetworkLedger ();
     newLCL->setClosed ();
     auto const newOL = std::make_shared<
-        Ledger>(false, std::ref (*newLCL));
+        Ledger>(open_ledger, std::ref (*newLCL));
     // Caller must own master lock
     {
         auto const oldOL =

src/ripple/ledger/tests/View_test.cpp

@@ -148,9 +148,12 @@ class View_test
     {
         Config const config;
         using namespace jtx;
-        auto const ledger =
+        std::shared_ptr<Ledger const> const genesis =
             std::make_shared<Ledger>(
                 create_genesis, config);
+        auto const ledger =
+            std::make_shared<Ledger>(
+                open_ledger, *genesis);
         wipe(*ledger);
         ReadView& v = *ledger;
         succ(v, 0, boost::none);
@@ -392,9 +395,12 @@ class View_test
         // erase the item, apply.
         {
             Config const config;
-            auto const ledger =
+            std::shared_ptr<Ledger const> const genesis =
                 std::make_shared<Ledger>(
                     create_genesis, config);
+            auto const ledger =
+                std::make_shared<Ledger>(
+                    open_ledger, *genesis);
             wipe(*ledger);
             ledger->rawInsert(sle(1));
             ReadView& v0 = *ledger;

src/ripple/test/jtx/impl/Env.cpp

@@ -84,7 +84,7 @@ Env::close(
     clock.set(closeTime);
     // VFALCO TODO Fix the Ledger constructor
     auto next = std::make_shared<Ledger>(
-        false, const_cast<Ledger&>(*closed_));
+        open_ledger, *closed_);
     next->setClosed();
 #if 0
     // Build a SHAMap, put all the transactions