Fixes after rebase

Author: SirTyson

src/bucket/BucketUtils.cpp

@@ -147,10 +147,10 @@ EvictionResultCandidates::isValid(uint32_t currLedgerSeq,
     // affect evictions scans.
     if (protocolVersionIsBefore(
             initialLedgerVers,
-            BucketBase::FIRST_PROTOCOL_SUPPORTING_PERSISTENT_EVICTION) &&
+            LiveBucket::FIRST_PROTOCOL_SUPPORTING_PERSISTENT_EVICTION) &&
         protocolVersionStartsFrom(
             currLedgerVers,
-            BucketBase::FIRST_PROTOCOL_SUPPORTING_PERSISTENT_EVICTION))
+            LiveBucket::FIRST_PROTOCOL_SUPPORTING_PERSISTENT_EVICTION))
     {
         return false;
     }

src/history/test/HistoryTests.cpp

@@ -1252,10 +1252,10 @@ TEST_CASE("Catchup with protocol upgrade", "[catchup][history]")
     {
         if (protocolVersionEquals(
                 Config::CURRENT_LEDGER_PROTOCOL_VERSION,
-                BucketBase::FIRST_PROTOCOL_SUPPORTING_PERSISTENT_EVICTION))
+                LiveBucket::FIRST_PROTOCOL_SUPPORTING_PERSISTENT_EVICTION))
         {
             testUpgrade(
-                BucketBase::FIRST_PROTOCOL_SUPPORTING_PERSISTENT_EVICTION);
+                LiveBucket::FIRST_PROTOCOL_SUPPORTING_PERSISTENT_EVICTION);
         }
     }
 }

src/historywork/DownloadBucketsWork.cpp

@@ -11,7 +11,6 @@
 #include "historywork/GetAndUnzipRemoteFileWork.h"
 #include "historywork/VerifyBucketWork.h"
 #include "work/WorkWithCallback.h"
-#include "xdr/Stellar-contract-config-setting.h"
 #include <Tracy.hpp>
 #include <fmt/format.h>
 #include <mutex>
@@ -75,7 +74,7 @@ DownloadBucketsWork::resetIter()
 }
 
 template <typename BucketT>
-bool
+void
 DownloadBucketsWork::onSuccessCb(
     Application& app, FileTransferInfo const& ft, std::string const& hash,
     int currId, std::map<std::string, std::shared_ptr<BucketT>>& buckets,
@@ -99,7 +98,6 @@ DownloadBucketsWork::onSuccessCb(
         /*mergeKey=*/nullptr,
         /*index=*/std::move(indexIter->second));
     buckets[hash] = b;
-    return true;
 }
 
 std::shared_ptr<BasicWork>
@@ -159,11 +157,12 @@ DownloadBucketsWork::yieldMoreWork()
             auto self = weakSelf.lock();
             if (self)
             {
-                return onSuccessCb<HotArchiveBucket>(
-                    app, ft, hash, currId, self->mHotBuckets,
-                    self->mHotIndexMap, self->mHotIndexMapMutex);
+                onSuccessCb<HotArchiveBucket>(app, ft, hash, currId,
+                                                    self->mHotBuckets,
+                                                    self->mHotIndexMap,
+                                                    self->mHotIndexMapMutex);
             }
-            return false;
+            return true;
         };
 
         mNextHotBucketIter++;
@@ -182,11 +181,12 @@ DownloadBucketsWork::yieldMoreWork()
             auto self = weakSelf.lock();
             if (self)
             {
-                return onSuccessCb<LiveBucket>(
-                    app, ft, hash, currId, self->mLiveBuckets,
-                    self->mLiveIndexMap, self->mLiveIndexMapMutex);
+                onSuccessCb<LiveBucket>(app, ft, hash, currId,
+                                               self->mLiveBuckets,
+                                               self->mLiveIndexMap,
+                                               self->mLiveIndexMapMutex);
             }
-            return false;
+            return true;
         };
 
         mNextLiveBucketIter++;
@@ -203,12 +203,12 @@ DownloadBucketsWork::yieldMoreWork()
 }
 
 // Add explicit template instantiations
-template bool DownloadBucketsWork::onSuccessCb<LiveBucket>(
+template void DownloadBucketsWork::onSuccessCb<LiveBucket>(
     Application&, FileTransferInfo const&, std::string const&, int,
     std::map<std::string, std::shared_ptr<LiveBucket>>&,
     std::map<int, std::unique_ptr<LiveBucketIndex const>>&, std::mutex&);
 
-template bool DownloadBucketsWork::onSuccessCb<HotArchiveBucket>(
+template void DownloadBucketsWork::onSuccessCb<HotArchiveBucket>(
     Application&, FileTransferInfo const&, std::string const&, int,
     std::map<std::string, std::shared_ptr<HotArchiveBucket>>&,
     std::map<int, std::unique_ptr<HotArchiveBucketIndex const>>&, std::mutex&);

src/historywork/DownloadBucketsWork.h

@@ -27,7 +27,10 @@ class DownloadBucketsWork : public BatchWork
     TmpDir const& mDownloadDir;
     std::shared_ptr<HistoryArchive> mArchive;
 
-    // Store indexes of downloaded buckets
+    // Store indexes of downloaded buckets. Child processes will actually create
+    // the indexes, but DownloadBucketsWork needs to maintain actual ownership
+    // of the pointers so that the success callback can pass them to the
+    // BucketManager. Must be protected by a mutex to avoid race conditions.
     std::map<int, std::unique_ptr<LiveBucketIndex const>> mLiveIndexMap;
     std::map<int, std::unique_ptr<HotArchiveBucketIndex const>> mHotIndexMap;
 
@@ -38,7 +41,7 @@ class DownloadBucketsWork : public BatchWork
     int mHotIndexId{0};
 
     template <typename BucketT>
-    static bool
+    static void
     onSuccessCb(Application& app, FileTransferInfo const& ft,
                 std::string const& hash, int currId,
                 std::map<std::string, std::shared_ptr<BucketT>>& buckets,