From f1a0cac8bfc89a33dfa7dd08bc743bc6c5e63424 Mon Sep 17 00:00:00 2001
From: MonsieurNicolas <nicolas@stellar.org>
Date: Sat, 8 Dec 2018 12:47:00 -0800
Subject: [PATCH 1/2] added getLastClosedLedgerHAS to LedgerManager

---
 src/ledger/LedgerManager.h       |  4 ++++
 src/ledger/LedgerManagerImpl.cpp | 15 +++++++++++----
 src/ledger/LedgerManagerImpl.h   |  2 ++
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/src/ledger/LedgerManager.h b/src/ledger/LedgerManager.h
index 3746f774f3..072e8d9715 100644
--- a/src/ledger/LedgerManager.h
+++ b/src/ledger/LedgerManager.h
@@ -104,6 +104,10 @@ class LedgerManager
     virtual LedgerHeaderHistoryEntry const&
     getLastClosedLedgerHeader() const = 0;
 
+    // return the HAS that corresponds to the last closed ledger as persisted in
+    // the database
+    virtual HistoryArchiveState getLastClosedLedgerHAS() = 0;
+
     // Return the sequence number of the LCL.
     virtual uint32_t getLastClosedLedgerNum() const = 0;
 
diff --git a/src/ledger/LedgerManagerImpl.cpp b/src/ledger/LedgerManagerImpl.cpp
index 1df7ef1406..4d85028d18 100644
--- a/src/ledger/LedgerManagerImpl.cpp
+++ b/src/ledger/LedgerManagerImpl.cpp
@@ -261,10 +261,7 @@ LedgerManagerImpl::loadLastKnownLedger(
 
         if (handler)
         {
-            string hasString = mApp.getPersistentState().getState(
-                PersistentState::kHistoryArchiveState);
-            HistoryArchiveState has;
-            has.fromString(hasString);
+            HistoryArchiveState has = getLastClosedLedgerHAS();
 
             auto continuation = [this, handler,
                                  has](asio::error_code const& ec) {
@@ -354,6 +351,16 @@ LedgerManagerImpl::getLastClosedLedgerHeader() const
     return mLastClosedLedger;
 }
 
+HistoryArchiveState
+LedgerManagerImpl::getLastClosedLedgerHAS()
+{
+    string hasString = mApp.getPersistentState().getState(
+        PersistentState::kHistoryArchiveState);
+    HistoryArchiveState has;
+    has.fromString(hasString);
+    return has;
+}
+
 uint32_t
 LedgerManagerImpl::getLastClosedLedgerNum() const
 {
diff --git a/src/ledger/LedgerManagerImpl.h b/src/ledger/LedgerManagerImpl.h
index b007146c44..873bbde767 100644
--- a/src/ledger/LedgerManagerImpl.h
+++ b/src/ledger/LedgerManagerImpl.h
@@ -113,6 +113,8 @@ class LedgerManagerImpl : public LedgerManager
 
     LedgerHeaderHistoryEntry const& getLastClosedLedgerHeader() const override;
 
+    HistoryArchiveState getLastClosedLedgerHAS() override;
+
     Database& getDatabase() override;
 
     void startCatchup(CatchupConfiguration configuration,

From f4b6572f6cc063df58fd4c145e12589abd01be8c Mon Sep 17 00:00:00 2001
From: MonsieurNicolas <nicolas@stellar.org>
Date: Sat, 8 Dec 2018 15:46:03 -0800
Subject: [PATCH 2/2] always retain buckets referenced by last closed ledger

the in memory state (tracked by BucketManager) may change as merges happen,
this may cause buckets referenced by the LCL HAS to not be the same
---
 src/bucket/BucketManagerImpl.cpp | 48 +++++++++++++++++++++++++-------
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/src/bucket/BucketManagerImpl.cpp b/src/bucket/BucketManagerImpl.cpp
index 21baf73ba8..36547608e5 100644
--- a/src/bucket/BucketManagerImpl.cpp
+++ b/src/bucket/BucketManagerImpl.cpp
@@ -6,6 +6,7 @@
 #include "bucket/BucketList.h"
 #include "crypto/Hex.h"
 #include "history/HistoryManager.h"
+#include "ledger/LedgerManager.h"
 #include "main/Application.h"
 #include "main/Config.h"
 #include "overlay/StellarXDR.h"
@@ -237,28 +238,55 @@ BucketManagerImpl::getBucketByHash(uint256 const& hash)
 std::set<Hash>
 BucketManagerImpl::getReferencedBuckets() const
 {
-    auto referenced = std::set<Hash>{};
+    std::set<Hash> referenced;
+    // retain current bucket list
     for (uint32_t i = 0; i < BucketList::kNumLevels; ++i)
     {
         auto const& level = mBucketList.getLevel(i);
-        referenced.insert(level.getCurr()->getHash());
-        referenced.insert(level.getSnap()->getHash());
+        auto rit = referenced.insert(level.getCurr()->getHash());
+        if (rit.second)
+        {
+            CLOG(TRACE, "Bucket")
+                << binToHex(*rit.first) << " referenced by bucket list";
+        }
+        rit = referenced.insert(level.getSnap()->getHash());
+        if (rit.second)
+        {
+            CLOG(TRACE, "Bucket")
+                << binToHex(*rit.first) << " referenced by bucket list";
+        }
         for (auto const& h : level.getNext().getHashes())
         {
-            referenced.insert(hexToBin256(h));
+            rit = referenced.insert(hexToBin256(h));
+            if (rit.second)
+            {
+                CLOG(TRACE, "Bucket") << h << " referenced by bucket list";
+            }
+        }
+    }
+    // retain any bucket referenced by the last closed ledger as recorded in the
+    // database (as merge complete, the bucket list drifts from that state)
+    auto lclHas = mApp.getLedgerManager().getLastClosedLedgerHAS();
+    auto lclBuckets = lclHas.allBuckets();
+    for (auto const& h : lclBuckets)
+    {
+        auto rit = referenced.insert(hexToBin256(h));
+        if (rit.second)
+        {
+            CLOG(TRACE, "Bucket") << h << " referenced by LCL";
         }
     }
 
-    // Implicitly retain any buckets that are referenced by a state in
-    // the publish queue.
+    // retain buckets that are referenced by a state in the publish queue.
     auto pub = mApp.getHistoryManager().getBucketsReferencedByPublishQueue();
     {
         for (auto const& h : pub)
         {
-            CLOG(DEBUG, "Bucket")
-                << "BucketManager::forgetUnreferencedBuckets: " << h
-                << " referenced by publish queue";
-            referenced.insert(hexToBin256(h));
+            auto rit = referenced.insert(hexToBin256(h));
+            if (rit.second)
+            {
+                CLOG(TRACE, "Bucket") << h << " referenced by publish queue";
+            }
         }
     }