Support for persisting prune locks in blocks/index db

src/interfaces/chain.h

@@ -150,8 +150,8 @@ public:
     virtual bool haveBlockOnDisk(int height) = 0;
 
     virtual bool pruneLockExists(const std::string& name) const = 0;
-    virtual void updatePruneLock(const std::string& name, const node::PruneLockInfo& lock_info) = 0;
+    virtual bool updatePruneLock(const std::string& name, const node::PruneLockInfo& lock_info, bool sync=false) = 0;
-    virtual void deletePruneLock(const std::string& name) = 0;
+    virtual bool deletePruneLock(const std::string& name) = 0;
 
     //! Get locator for the current chain tip.
     virtual CBlockLocator getTipLocator() = 0;

src/node/blockstorage.cpp

@@ -47,6 +47,7 @@ static constexpr uint8_t DB_BLOCK_INDEX{'b'};
 static constexpr uint8_t DB_FLAG{'F'};
 static constexpr uint8_t DB_REINDEX_FLAG{'R'};
 static constexpr uint8_t DB_LAST_BLOCK{'l'};
+static constexpr uint8_t DB_PRUNE_LOCK{'L'};
 // Keys used in previous version that might still be found in the DB:
 // BlockTreeDB::DB_TXINDEX_BLOCK{'T'};
 // BlockTreeDB::DB_TXINDEX{'t'}
@@ -76,7 +77,7 @@ bool BlockTreeDB::ReadLastBlockFile(int& nFile)
     return Read(DB_LAST_BLOCK, nFile);
 }
 
-bool BlockTreeDB::WriteBatchSync(const std::vector<std::pair<int, const CBlockFileInfo*>>& fileInfo, int nLastFile, const std::vector<const CBlockIndex*>& blockinfo)
+bool BlockTreeDB::WriteBatchSync(const std::vector<std::pair<int, const CBlockFileInfo*>>& fileInfo, int nLastFile, const std::vector<const CBlockIndex*>& blockinfo, const std::unordered_map<std::string, node::PruneLockInfo>& prune_locks)
 {
     CDBBatch batch(*this);
     for (const auto& [file, info] : fileInfo) {
@@ -86,9 +87,41 @@ bool BlockTreeDB::WriteBatchSync(const std::vector<std::pair<int, const CBlockFi
     for (const CBlockIndex* bi : blockinfo) {
         batch.Write(std::make_pair(DB_BLOCK_INDEX, bi->GetBlockHash()), CDiskBlockIndex{bi});
     }
+    for (const auto& prune_lock : prune_locks) {
+        if (prune_lock.second.temporary) continue;
+        batch.Write(std::make_pair(DB_PRUNE_LOCK, prune_lock.first), prune_lock.second);
+    }
     return WriteBatch(batch, true);
 }
 
+bool BlockTreeDB::WritePruneLock(const std::string& name, const node::PruneLockInfo& lock_info) {
+    if (lock_info.temporary) return true;
+    return Write(std::make_pair(DB_PRUNE_LOCK, name), lock_info);
+}
+
+bool BlockTreeDB::DeletePruneLock(const std::string& name) {
+    return Erase(std::make_pair(DB_PRUNE_LOCK, name));
+}
+
+bool BlockTreeDB::LoadPruneLocks(std::unordered_map<std::string, node::PruneLockInfo>& prune_locks, const util::SignalInterrupt& interrupt) {
+    std::unique_ptr<CDBIterator> pcursor(NewIterator());
+    for (pcursor->Seek(DB_PRUNE_LOCK); pcursor->Valid(); pcursor->Next()) {
+        if (interrupt) return false;
+
+        std::pair<uint8_t, std::string> key;
+        if ((!pcursor->GetKey(key)) || key.first != DB_PRUNE_LOCK) break;
+
+        node::PruneLockInfo& lock_info = prune_locks[key.second];
+        if (!pcursor->GetValue(lock_info)) {
+            LogError("%s: failed to %s prune lock '%s'\n", __func__, "read", key.second);
+            return false;
+        }
+        lock_info.temporary = false;
+    }
+
+    return true;
+}
+
 bool BlockTreeDB::WriteFlag(const std::string& name, bool fValue)
 {
     return Write(std::make_pair(DB_FLAG, name), fValue ? uint8_t{'1'} : uint8_t{'0'});
@@ -281,10 +314,10 @@ bool BlockManager::DoPruneLocksForbidPruning(const CBlockFileInfo& block_file_in
 {
     AssertLockHeld(cs_main);
     for (const auto& prune_lock : m_prune_locks) {
-        if (prune_lock.second.height_first == std::numeric_limits<int>::max()) continue;
+        if (prune_lock.second.height_first == std::numeric_limits<uint64_t>::max()) continue;
         // Remove the buffer and one additional block here to get actual height that is outside of the buffer
-        const unsigned int lock_height{(unsigned)std::max(1, prune_lock.second.height_first - PRUNE_LOCK_BUFFER - 1)};
+        const uint64_t lock_height{(prune_lock.second.height_first <= PRUNE_LOCK_BUFFER + 1) ? 1 : (prune_lock.second.height_first - PRUNE_LOCK_BUFFER - 1)};
-        const unsigned int lock_height_last{(unsigned)std::max(1, SaturatingAdd(prune_lock.second.height_last, PRUNE_LOCK_BUFFER))};
+        const uint64_t lock_height_last{SaturatingAdd(prune_lock.second.height_last, (uint64_t)PRUNE_LOCK_BUFFER)};
         if (block_file_info.nHeightFirst > lock_height_last) continue;
         if (block_file_info.nHeightLast <= lock_height) continue;
         // TODO: Check each block within the file against the prune_lock range
@@ -407,15 +440,37 @@ bool BlockManager::PruneLockExists(const std::string& name) const {
     return m_prune_locks.count(name);
 }
 
-void BlockManager::UpdatePruneLock(const std::string& name, const PruneLockInfo& lock_info) {
+bool BlockManager::UpdatePruneLock(const std::string& name, const PruneLockInfo& lock_info, const bool sync) {
     AssertLockHeld(::cs_main);
-    m_prune_locks[name] = lock_info;
+    if (sync) {
+        if (!m_block_tree_db->WritePruneLock(name, lock_info)) {
+            LogError("%s: failed to %s prune lock '%s'\n", __func__, "write", name);
+            return false;
+        }
+    }
+    PruneLockInfo& stored_lock_info = m_prune_locks[name];
+    if (lock_info.temporary && !stored_lock_info.temporary) {
+        // Erase non-temporary lock from disk
+        if (!m_block_tree_db->DeletePruneLock(name)) {
+            LogError("%s: failed to %s prune lock '%s'\n", __func__, "erase", name);
+            return false;
+        }
+    }
+    stored_lock_info = lock_info;
+    return true;
 }
 
-void BlockManager::DeletePruneLock(const std::string& name)
+bool BlockManager::DeletePruneLock(const std::string& name)
 {
     AssertLockHeld(::cs_main);
     m_prune_locks.erase(name);
+
+    // Since there is no reasonable expectation for any follow-up to this prune lock, actually ensure it gets committed to disk immediately
+    if (!m_block_tree_db->DeletePruneLock(name)) {
+        LogError("%s: failed to %s prune lock '%s'\n", __func__, "erase", name);
+        return false;
+    }
+    return true;
 }
 
 CBlockIndex* BlockManager::InsertBlockIndex(const uint256& hash)
@@ -441,6 +496,8 @@ bool BlockManager::LoadBlockIndex(const std::optional<uint256>& snapshot_blockha
         return false;
     }
 
+    if (!m_block_tree_db->LoadPruneLocks(m_prune_locks, m_interrupt)) return false;
+
     if (snapshot_blockhash) {
         const std::optional<AssumeutxoData> maybe_au_data = GetParams().AssumeutxoForBlockhash(*snapshot_blockhash);
         if (!maybe_au_data) {
@@ -529,7 +586,7 @@ bool BlockManager::WriteBlockIndexDB()
         m_dirty_blockindex.erase(it++);
     }
     int max_blockfile = WITH_LOCK(cs_LastBlockFile, return this->MaxBlockfileNum());
-    if (!m_block_tree_db->WriteBatchSync(vFiles, max_blockfile, vBlocks)) {
+    if (!m_block_tree_db->WriteBatchSync(vFiles, max_blockfile, vBlocks, m_prune_locks)) {
         return false;
     }
     return true;

src/node/blockstorage.h

@@ -41,6 +41,9 @@ class ChainstateManager;
 namespace Consensus {
 struct Params;
 }
+namespace node {
+struct PruneLockInfo;
+};
 namespace util {
 class SignalInterrupt;
 } // namespace util
@@ -51,15 +54,18 @@ class BlockTreeDB : public CDBWrapper
 {
 public:
     using CDBWrapper::CDBWrapper;
-    bool WriteBatchSync(const std::vector<std::pair<int, const CBlockFileInfo*>>& fileInfo, int nLastFile, const std::vector<const CBlockIndex*>& blockinfo);
+    bool WriteBatchSync(const std::vector<std::pair<int, const CBlockFileInfo*>>& fileInfo, int nLastFile, const std::vector<const CBlockIndex*>& blockinfo, const std::unordered_map<std::string, node::PruneLockInfo>& prune_locks);
     bool ReadBlockFileInfo(int nFile, CBlockFileInfo& info);
     bool ReadLastBlockFile(int& nFile);
     bool WriteReindexing(bool fReindexing);
     void ReadReindexing(bool& fReindexing);
+    bool WritePruneLock(const std::string& name, const node::PruneLockInfo&);
+    bool DeletePruneLock(const std::string& name);
     bool WriteFlag(const std::string& name, bool fValue);
     bool ReadFlag(const std::string& name, bool& fValue);
     bool LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function<CBlockIndex*(const uint256&)> insertBlockIndex, const util::SignalInterrupt& interrupt)
         EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+    bool LoadPruneLocks(std::unordered_map<std::string, node::PruneLockInfo>& prune_locks, const util::SignalInterrupt& interrupt);
 };
 } // namespace kernel
 
@@ -93,8 +99,16 @@ struct CBlockIndexHeightOnlyComparator {
 
 struct PruneLockInfo {
     std::string desc; //! Arbitrary human-readable description of the lock purpose
-    int height_first{std::numeric_limits<int>::max()}; //! Height of earliest block that should be kept and not pruned
+    uint64_t height_first{std::numeric_limits<uint64_t>::max()}; //! Height of earliest block that should be kept and not pruned
-    int height_last{std::numeric_limits<int>::max()}; //! Height of latest block that should be kept and not pruned
+    uint64_t height_last{std::numeric_limits<uint64_t>::max()}; //! Height of latest block that should be kept and not pruned
+    bool temporary{true};
+
+    SERIALIZE_METHODS(PruneLockInfo, obj)
+    {
+        READWRITE(obj.desc);
+        READWRITE(VARINT(obj.height_first));
+        READWRITE(VARINT(obj.height_last));
+    }
 };
 
 enum BlockfileType {
@@ -412,8 +426,8 @@ public:
 
     bool PruneLockExists(const std::string& name) const SHARED_LOCKS_REQUIRED(::cs_main);
     //! Create or update a prune lock identified by its name
-    void UpdatePruneLock(const std::string& name, const PruneLockInfo& lock_info) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+    bool UpdatePruneLock(const std::string& name, const PruneLockInfo& lock_info, bool sync=false) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
-    void DeletePruneLock(const std::string& name) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
+    bool DeletePruneLock(const std::string& name) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
     /** Open a block file (blk?????.dat) */
     AutoFile OpenBlockFile(const FlatFilePos& pos, bool fReadOnly = false) const;

src/node/interfaces.cpp

@@ -560,17 +560,17 @@ public:
         auto& blockman = m_node.chainman->m_blockman;
         return blockman.PruneLockExists(name);
     }
-    void updatePruneLock(const std::string& name, const node::PruneLockInfo& lock_info) override
+    bool updatePruneLock(const std::string& name, const node::PruneLockInfo& lock_info, bool sync) override
     {
         LOCK(cs_main);
         auto& blockman = m_node.chainman->m_blockman;
-        blockman.UpdatePruneLock(name, lock_info);
+        return blockman.UpdatePruneLock(name, lock_info, sync);
     }
-    void deletePruneLock(const std::string& name) override
+    bool deletePruneLock(const std::string& name) override
     {
         LOCK(cs_main);
         auto& blockman = m_node.chainman->m_blockman;
-        blockman.DeletePruneLock(name);
+        return blockman.DeletePruneLock(name);
     }
     CBlockLocator getTipLocator() override
     {

src/test/fuzz/block_index.cpp

@@ -89,7 +89,8 @@ FUZZ_TARGET(block_index, .init = init_block_index)
     }
 
     // Store these files and blocks in the block index. It should not fail.
-    assert(block_index.WriteBatchSync(files_info, files_count - 1, blocks_info));
+    const std::unordered_map<std::string, node::PruneLockInfo> prune_locks;
+    assert(block_index.WriteBatchSync(files_info, files_count - 1, blocks_info, prune_locks));
 
     // We should be able to read every block file info we stored. Its value should correspond to
     // what we stored above.

src/validation.cpp

@@ -3034,12 +3034,13 @@ bool Chainstate::DisconnectTip(BlockValidationState& state, DisconnectedBlockTra
 
     {
         // Prune locks that began around the tip should be moved backward so they get a chance to reorg
-        const int max_height_first{pindexDelete->nHeight - 1};
+        const uint64_t max_height_first{static_cast<uint64_t>(pindexDelete->nHeight - 1)};
         for (auto& prune_lock : m_blockman.m_prune_locks) {
             if (prune_lock.second.height_first < max_height_first) continue;
 
             --prune_lock.second.height_first;
             LogPrint(BCLog::PRUNE, "%s prune lock moved back to %d\n", prune_lock.first, prune_lock.second.height_first);
+            // NOTE: Don't need to write to db here, since it will get synced with the rest of the chainstate
         }
     }