Merge pull request #944 from MonsieurNicolas/previousMeta

Emit ledger entry in meta before modifying it

Reviewed-by: jedmccaleb

Builds/VisualStudio2015/stellar-core.vcxproj

@@ -175,6 +175,7 @@
     <ClCompile Include="..\..\src\ledger\AccountFrame.cpp" />
     <ClCompile Include="..\..\src\ledger\LedgerDelta.cpp" />
     <ClCompile Include="..\..\src\ledger\EntryFrame.cpp" />
+    <ClCompile Include="..\..\src\ledger\LedgerDeltaTests.cpp" />
     <ClCompile Include="..\..\src\ledger\LedgerEntryTests.cpp" />
     <ClCompile Include="..\..\src\ledger\LedgerHeaderFrame.cpp" />
     <ClCompile Include="..\..\src\ledger\LedgerHeaderTests.cpp" />

Builds/VisualStudio2015/stellar-core.vcxproj.filters

@@ -486,6 +486,9 @@
     <ClCompile Include="..\..\src\overlay\FloodTests.cpp">
       <Filter>overlay\tests</Filter>
     </ClCompile>
+    <ClCompile Include="..\..\src\ledger\LedgerDeltaTests.cpp">
+      <Filter>ledger\tests</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="..\..\src\ledger\LedgerManager.h">

src/history/CatchupStateMachine.cpp

@@ -1236,7 +1236,8 @@ CatchupStateMachine::applyHistoryOfSingleCheckpoint(uint32_t checkpoint)
 void
 CatchupStateMachine::enterEndState()
 {
-    assert(mState == CATCHUP_APPLYING || (mState == CATCHUP_RETRYING && (!!mError)));
+    assert(mState == CATCHUP_APPLYING ||
+           (mState == CATCHUP_RETRYING && (!!mError)));
     mApplyState.reset();
     mState = CATCHUP_END;
     CLOG(DEBUG, "History") << "Completed catchup from '" << mArchive->getName()

src/history/HistoryTests.cpp

@@ -135,8 +135,7 @@ class HistoryTests
     bool catchupApplication(uint32_t initLedger,
                             HistoryManager::CatchupMode resumeMode,
                             Application::pointer app2, bool doStart = true,
-                            uint32_t maxCranks = 0xffffffff,
-                            uint32_t gap=0);
+                            uint32_t maxCranks = 0xffffffff, uint32_t gap = 0);
 
     bool
     flip()
@@ -416,8 +415,7 @@ bool
 HistoryTests::catchupApplication(uint32_t initLedger,
                                  HistoryManager::CatchupMode resumeMode,
                                  Application::pointer app2, bool doStart,
-                                 uint32_t maxCranks,
-                                 uint32_t gap)
+                                 uint32_t maxCranks, uint32_t gap)
 {
 
     auto& lm = app2->getLedgerManager();
@@ -949,7 +947,6 @@ TEST_CASE("persist publish queue", "[history]")
     }
 }
 
-
 // The idea with this test is that we join a network and somehow get a gap
 // in the SCP voting sequence while we're trying to catchup.  This should
 // cause catchup to fail, but that failure should itself just flush the
@@ -978,8 +975,8 @@ TEST_CASE_METHOD(HistoryTests, "too far behind / catchup restart",
 
     // Now start a catchup on that _fails_ due to a gap
     LOG(INFO) << "Starting BROKEN catchup (with gap) from " << init;
-    caughtup = catchupApplication(init, HistoryManager::CATCHUP_COMPLETE,
-                                  app2, true, 10000, init + 10);
+    caughtup = catchupApplication(init, HistoryManager::CATCHUP_COMPLETE, app2,
+                                  true, 10000, init + 10);
 
     assert(!caughtup);
 

src/ledger/AccountFrame.cpp

@@ -195,6 +195,18 @@ AccountFrame::getLowThreshold() const
     return mAccountEntry.thresholds[THRESHOLD_LOW];
 }
 
+AccountFrame::pointer
+AccountFrame::loadAccount(LedgerDelta& delta, AccountID const& accountID,
+                          Database& db)
+{
+    auto a = loadAccount(accountID, db);
+    if (a)
+    {
+        delta.recordEntry(*a);
+    }
+    return a;
+}
+
 AccountFrame::pointer
 AccountFrame::loadAccount(AccountID const& accountID, Database& db)
 {

src/ledger/AccountFrame.h

@@ -121,6 +121,8 @@ class AccountFrame : public EntryFrame
     static uint64_t countObjects(soci::session& sess);
 
     // database utilities
+    static AccountFrame::pointer
+    loadAccount(LedgerDelta& delta, AccountID const& accountID, Database& db);
     static AccountFrame::pointer loadAccount(AccountID const& accountID,
                                              Database& db);
 

src/ledger/LedgerDelta.cpp

@@ -89,6 +89,12 @@ LedgerDelta::modEntry(EntryFrame const& entry)
     modEntry(entry.copy());
 }
 
+void
+LedgerDelta::recordEntry(EntryFrame const& entry)
+{
+    recordEntry(entry.copy());
+}
+
 void
 LedgerDelta::addEntry(EntryFrame::pointer entry)
 {
@@ -162,13 +168,28 @@ LedgerDelta::modEntry(EntryFrame::pointer entry)
     }
 }
 
+void
+LedgerDelta::recordEntry(EntryFrame::pointer entry)
+{
+    checkState();
+    // keeps the old one around
+    mPrevious.insert(std::make_pair(entry->getKey(), entry));
+}
+
 void
 LedgerDelta::mergeEntries(LedgerDelta& other)
 {
     checkState();
+
+    // propagates mPrevious for deleted & modified entries
     for (auto& d : other.mDelete)
     {
         deleteEntry(d);
+        auto it = other.mPrevious.find(d);
+        if (it != other.mPrevious.end())
+        {
+            recordEntry(*it->second);
+        }
     }
     for (auto& n : other.mNew)
     {
@@ -177,6 +198,11 @@ LedgerDelta::mergeEntries(LedgerDelta& other)
     for (auto& m : other.mMod)
     {
         modEntry(m.second);
+        auto it = other.mPrevious.find(m.first);
+        if (it != other.mPrevious.end())
+        {
+            recordEntry(*it->second);
+        }
     }
 }
 
@@ -220,6 +246,24 @@ LedgerDelta::rollback()
     }
 }
 
+void
+LedgerDelta::addCurrentMeta(LedgerEntryChanges& changes,
+                            LedgerKey const& key) const
+{
+    auto it = mPrevious.find(key);
+    if (it != mPrevious.end())
+    {
+        // if the old value is from a previous ledger
+        // we emit it
+        auto const& e = it->second->mEntry;
+        if (e.lastModifiedLedgerSeq != mCurrentHeader.mHeader.ledgerSeq)
+        {
+            changes.emplace_back(LEDGER_ENTRY_STATE);
+            changes.back().state() = e;
+        }
+    }
+}
+
 LedgerEntryChanges
 LedgerDelta::getChanges() const
 {
@@ -232,12 +276,14 @@ LedgerDelta::getChanges() const
     }
     for (auto const& k : mMod)
     {
+        addCurrentMeta(changes, k.first);
         changes.emplace_back(LEDGER_ENTRY_UPDATED);
         changes.back().updated() = k.second->mEntry;
     }
 
     for (auto const& k : mDelete)
     {
+        addCurrentMeta(changes, k);
         changes.emplace_back(LEDGER_ENTRY_REMOVED);
         changes.back().removed() = k;
     }

src/ledger/LedgerDelta.h

@@ -33,6 +33,7 @@ class LedgerDelta
     KeyEntryMap mNew;
     KeyEntryMap mMod;
     std::set<LedgerKey, LedgerEntryIdCmp> mDelete;
+    KeyEntryMap mPrevious;
 
     Database& mDb; // Used strictly for rollback of db entry cache.
 
@@ -42,10 +43,16 @@ class LedgerDelta
     void addEntry(EntryFrame::pointer entry);
     void deleteEntry(EntryFrame::pointer entry);
     void modEntry(EntryFrame::pointer entry);
+    void recordEntry(EntryFrame::pointer entry);
 
     // merge "other" into current ledgerDelta
     void mergeEntries(LedgerDelta& other);
 
+    // helper method that adds a meta entry to "changes"
+    // with the previous value of an entry if needed
+    void addCurrentMeta(LedgerEntryChanges& changes,
+                        LedgerKey const& key) const;
+
   public:
     // keeps an internal reference to the outerDelta,
     // will apply changes to the outer scope on commit
@@ -69,6 +76,7 @@ class LedgerDelta
     void deleteEntry(EntryFrame const& entry);
     void deleteEntry(LedgerKey const& key);
     void modEntry(EntryFrame const& entry);
+    void recordEntry(EntryFrame const& entry);
 
     // commits this delta into outer delta
     void commit();