Merge pull request #1270 from psgreco/master-trim-headers-v2

Restore full functionality to trim_headers (untrim)

src/bench/rpc_blockchain.cpp

@@ -15,7 +15,7 @@
 namespace {
 
 struct TestBlockAndIndex {
-    const std::unique_ptr<const TestingSetup> testing_setup{MakeNoLogFileContext<const TestingSetup>(CBaseChainParams::MAIN)};
+    std::unique_ptr<TestingSetup> testing_setup{MakeNoLogFileContext<TestingSetup>(CBaseChainParams::MAIN)};
     CBlock block{};
     uint256 blockHash{};
     CBlockIndex blockindex{};
@@ -28,6 +28,7 @@ struct TestBlockAndIndex {
 
         stream >> block;
 
+        CBlockIndex::SetNodeContext(&(testing_setup->m_node));
         blockHash = block.GetHash();
         blockindex.phashBlock = &blockHash;
         blockindex.nBits = 403014710;

src/chain.cpp

@@ -5,6 +5,11 @@
 
 #include <chain.h>
 #include <util/time.h>
+#include <validation.h>
+#include <node/context.h>
+
+
+node::NodeContext *CBlockIndex::m_pcontext;
 
 std::string CBlockFileInfo::ToString() const
 {
@@ -51,6 +56,27 @@ CBlockLocator CChain::GetLocator(const CBlockIndex *pindex) const {
     return CBlockLocator(vHave);
 }
 
+void CBlockIndex::untrim() EXCLUSIVE_LOCKS_REQUIRED(::cs_main){
+    AssertLockHeld(::cs_main);
+    if (!trimmed())
+        return;
+    CBlockIndex tmp;
+    const CBlockIndex *pindexfull = untrim_to(&tmp);
+    assert(pindexfull!=this);
+    m_trimmed = false;
+    set_stored();
+    proof = pindexfull->proof;
+    m_dynafed_params = pindexfull->m_dynafed_params;
+    m_signblock_witness = pindexfull->m_signblock_witness;
+    m_pcontext->chainman->m_blockman.m_dirty_blockindex.insert(this);
+}
+
+const CBlockIndex *CBlockIndex::untrim_to(CBlockIndex *pindexNew) const EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
+{
+    AssertLockHeld(::cs_main);
+    return m_pcontext->chainman->m_blockman.m_block_tree_db->RegenerateFullIndex(this, pindexNew);
+}
+
 const CBlockIndex *CChain::FindFork(const CBlockIndex *pindex) const {
     if (pindex == nullptr) {
         return nullptr;

src/chain.h

@@ -16,6 +16,9 @@
 
 #include <vector>
 
+namespace node {
+struct NodeContext;
+}
 /**
  * Maximum amount of time that a block timestamp is allowed to exceed the
  * current network-adjusted time before the block will be accepted.
@@ -215,26 +218,41 @@ protected:
 
     bool m_trimmed{false};
     bool m_trimmed_dynafed_block{false};
+    bool m_stored_lvl{false};
 
     friend class CBlockTreeDB;
 
+    static node::NodeContext *m_pcontext;
+
 public:
+    static void SetNodeContext(node::NodeContext *context) {m_pcontext = context;};
 
     // Irrevocably remove blocksigning and dynafed-related stuff from this
     // in-memory copy of the block header.
-    void trim() {
+    bool trim() {
         assert_untrimmed();
+        if (!m_stored_lvl) {
+            // We can't trim in-memory data if it's not on disk yet, but we can if it's already been recovered once
+            return false;
+        }
         m_trimmed = true;
         m_trimmed_dynafed_block = !m_dynafed_params.value().IsNull();
         proof = std::nullopt;
         m_dynafed_params = std::nullopt;
         m_signblock_witness = std::nullopt;
+        return true;
     }
 
+    void untrim();
+    const CBlockIndex * untrim_to(CBlockIndex *pindexNew) const;
+
     inline bool trimmed() const {
         return m_trimmed;
     }
 
+    inline void set_stored() {
+        m_stored_lvl = true;
+    }
     inline void assert_untrimmed() const {
         assert(!m_trimmed);
     }
@@ -501,6 +519,9 @@ public:
 
         // For compatibility with elements 0.14 based chains
         if (g_signed_blocks) {
+            if (!ser_action.ForRead()) {
+                obj.assert_untrimmed();
+            }
             if (is_dyna) {
                 READWRITE(obj.m_dynafed_params.value());
                 READWRITE(obj.m_signblock_witness.value().stack);

src/dynafed.cpp

@@ -1,6 +1,8 @@
 
 #include <dynafed.h>
 #include <hash.h>
+#include <validation.h>
+#include <node/context.h>
 
 bool NextBlockIsParameterTransition(const CBlockIndex* pindexPrev, const Consensus::Params& consensus, DynaFedParamEntry& winning_entry)
 {
@@ -15,6 +17,10 @@ bool NextBlockIsParameterTransition(const CBlockIndex* pindexPrev, const Consens
     for (int32_t height = next_height - 1; height >= (int32_t)(next_height - consensus.dynamic_epoch_length); --height) {
         const CBlockIndex* p_epoch_walk = pindexPrev->GetAncestor(height);
         assert(p_epoch_walk);
+        if (node::fTrimHeaders) {
+            LOCK(cs_main);
+            ForceUntrimHeader(p_epoch_walk);
+        }
         const DynaFedParamEntry& proposal = p_epoch_walk->dynafed_params().m_proposed;
         const uint256 proposal_root = proposal.CalculateRoot();
         vote_tally[proposal_root]++;
@@ -60,6 +66,10 @@ DynaFedParamEntry ComputeNextBlockFullCurrentParameters(const CBlockIndex* pinde
     // may be pre-dynafed params
     const CBlockIndex* p_epoch_start = pindexPrev->GetAncestor(epoch_start_height);
     assert(p_epoch_start);
+    if (node::fTrimHeaders) {
+        LOCK(cs_main);
+        ForceUntrimHeader(p_epoch_start);
+    }
     if (p_epoch_start->dynafed_params().IsNull()) {
         // We need to construct the "full" current parameters of pre-dynafed
         // consensus
@@ -93,6 +103,10 @@ DynaFedParamEntry ComputeNextBlockCurrentParameters(const CBlockIndex* pindexPre
 {
     assert(pindexPrev);
 
+    if (node::fTrimHeaders) {
+        LOCK(cs_main);
+        ForceUntrimHeader(pindexPrev);
+    }
     DynaFedParamEntry entry = ComputeNextBlockFullCurrentParameters(pindexPrev, consensus);
 
     uint32_t next_height = pindexPrev->nHeight+1;

src/init.cpp

@@ -1011,13 +1011,13 @@ bool AppInitParameterInteraction(const ArgsManager& args)
     }
 
     if (args.GetBoolArg("-trim_headers", false)) {
-        LogPrintf("Configured for header-trimming mode. This will reduce memory usage substantially, but we will be unable to serve as a full P2P peer, and certain header fields may be missing from JSON RPC output.\n");
+        LogPrintf("Configured for header-trimming mode. This will reduce memory usage substantially, but will increase IO usage when the headers need to be temporarily untrimmed.\n");
         node::fTrimHeaders = true;
         // This calculation is driven by GetValidFedpegScripts in pegins.cpp, which walks the chain
         //   back to current epoch start, and then an additional total_valid_epochs on top of that.
         //   We add one epoch here for the current partial epoch, and then another one for good luck.
 
-        node::nMustKeepFullHeaders = (chainparams.GetConsensus().total_valid_epochs + 2) * epoch_length;
+        node::nMustKeepFullHeaders = chainparams.GetConsensus().total_valid_epochs * epoch_length;
         // This is the number of headers we can have in flight downloading at a time, beyond the
         //   set of blocks we've already validated. Capping this is necessary to keep memory usage
         //   bounded during IBD.
@@ -1243,6 +1243,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     const ArgsManager& args = *Assert(node.args);
     const CChainParams& chainparams = Params();
 
+    CBlockIndex::SetNodeContext(&node);
     auto opt_max_upload = ParseByteUnits(args.GetArg("-maxuploadtarget", DEFAULT_MAX_UPLOAD_TARGET), ByteUnit::M);
     if (!opt_max_upload) {
         return InitError(strprintf(_("Unable to parse -maxuploadtarget: '%s'"), args.GetArg("-maxuploadtarget", "")));
@@ -1712,7 +1713,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 
     // if pruning, unset the service bit and perform the initial blockstore prune
     // after any wallet rescanning has taken place.
-    if (fPruneMode || node::fTrimHeaders) {
+    if (fPruneMode) {
         LogPrintf("Unsetting NODE_NETWORK on prune mode\n");
         nLocalServices = ServiceFlags(nLocalServices & ~NODE_NETWORK);
         if (!fReindex) {
@@ -1724,11 +1725,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         }
     }
 
-    if (node::fTrimHeaders) {
-        LogPrintf("Unsetting NODE_NETWORK_LIMITED on header trim mode\n");
-        nLocalServices = ServiceFlags(nLocalServices & ~NODE_NETWORK_LIMITED);
-    }
-
     // ********************************************************* Step 11: import blocks
 
     if (!CheckDiskSpace(gArgs.GetDataDirNet())) {

src/net_processing.cpp

@@ -3305,12 +3305,13 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
         for (; pindex; pindex = m_chainman.ActiveChain().Next(pindex))
         {
             if (pindex->trimmed()) {
-                // For simplicity, if any of the headers they're asking for are trimmed,
+                // Header is trimmed, reload from disk before sending
-                //   just drop the request.
+                CBlockIndex tmpBlockIndexFull;
-                LogPrint(BCLog::NET, "%s: ignoring getheaders from peer=%i which would return at least one trimmed header\n", __func__, pfrom.GetId());
+                const CBlockIndex* pindexfull = pindex->untrim_to(&tmpBlockIndexFull);
-                return;
+                vHeaders.push_back(pindexfull->GetBlockHeader());
+            } else {
+                vHeaders.push_back(pindex->GetBlockHeader());
             }
-            vHeaders.push_back(pindex->GetBlockHeader());
             if (--nLimit <= 0 || pindex->GetBlockHash() == hashStop)
                 break;
         }

src/node/blockstorage.cpp

@@ -223,15 +223,7 @@ bool BlockManager::LoadBlockIndex(
 {
     int trim_below_height = 0;
     if (fTrimHeaders) {
-        int max_height = 0;
+        trim_below_height = std::numeric_limits<int>::max();
-        if (!m_block_tree_db->WalkBlockIndexGutsForMaxHeight(&max_height)) {
-            LogPrintf("LoadBlockIndex: Failed to WalkBlockIndexGutsForMaxHeight.\n");
-            return false;
-        }
-
-        int must_keep_headers = (consensus_params.total_valid_epochs + 2) * consensus_params.dynamic_epoch_length;
-        int extra_headers_buffer = consensus_params.dynamic_epoch_length * 2; // XXX arbitrary
-        trim_below_height = max_height - must_keep_headers - extra_headers_buffer;
     }
     if (!m_block_tree_db->LoadBlockIndexGuts(consensus_params, [this](const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return this->InsertBlockIndex(hash); }, trim_below_height)) {
         return false;
@@ -337,6 +329,9 @@ bool BlockManager::LoadBlockIndex(
         }
     }
 
+    if (pindexBestHeader) {
+        ForceUntrimHeader(pindexBestHeader);
+    }
     return true;
 }
 
@@ -803,23 +798,6 @@ bool ReadBlockFromDisk(CBlock& block, const CBlockIndex* pindex, const Consensus
     return true;
 }
 
-bool ReadBlockHeaderFromDisk(CBlockHeader& header, const CBlockIndex* pindex, const Consensus::Params& consensusParams)
-{
-    // Not very efficient: read a block and throw away all but the header.
-    CBlock tmp;
-    if (!ReadBlockFromDisk(tmp, pindex, consensusParams)) {
-        return false;
-    }
-    const FlatFilePos block_pos{WITH_LOCK(cs_main, return pindex->GetBlockPos())};
-
-    header = tmp.GetBlockHeader();
-    if (tmp.GetHash() != pindex->GetBlockHash()) {
-        return error("ReadBlockheaderFromDisk(CBlockHeader&, CBlockIndex*): GetHash() doesn't match index for %s at %s",
-                     pindex->ToString(), block_pos.ToString());
-    }
-    return true;
-}
-
 bool ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos, const CMessageHeader::MessageStartChars& message_start)
 {
     FlatFilePos hpos = pos;

src/node/blockstorage.h

@@ -76,6 +76,7 @@ class BlockManager
 {
     friend CChainState;
     friend ChainstateManager;
+    friend CBlockIndex;
 
 private:
     void FlushBlockFile(bool fFinalize = false, bool finalize_undo = false);
@@ -193,7 +194,6 @@ bool ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos, const Consensus::P
 bool ReadBlockFromDisk(CBlock& block, const CBlockIndex* pindex, const Consensus::Params& consensusParams);
 bool ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos, const CMessageHeader::MessageStartChars& message_start);
 // ELEMENTS:
-bool ReadBlockHeaderFromDisk(class CBlockHeader& header, const CBlockIndex* pindex, const Consensus::Params& consensusParams);
 
 bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex* pindex);
 

src/pegins.cpp

@@ -26,6 +26,8 @@
 // ELEMENTS
 //
 
+#include <validation.h>
+
 namespace {
 static secp256k1_context* secp256k1_ctx_validation;
 
@@ -487,6 +489,10 @@ std::vector<std::pair<CScript, CScript>> GetValidFedpegScripts(const CBlockIndex
             break;
         }
 
+        if (node::fTrimHeaders) {
+            LOCK(cs_main);
+            ForceUntrimHeader(p_epoch_start);
+        }
         if (!p_epoch_start->dynafed_params().IsNull()) {
             fedpegscripts.push_back(std::make_pair(p_epoch_start->dynafed_params().m_current.m_fedpeg_program, p_epoch_start->dynafed_params().m_current.m_fedpegscript));
         } else {

src/qt/test/rpcnestedtests.cpp

@@ -48,6 +48,7 @@ void RPCNestedTests::rpcNestedTests()
 
     TestingSetup test;
     m_node.setContext(&test.m_node);
+    CBlockIndex::SetNodeContext(&test.m_node);
 
     if (RPCIsInWarmup(nullptr)) SetRPCWarmupFinished();
 

src/rest.cpp

@@ -232,13 +232,10 @@ static bool rest_headers(const std::any& context,
     case RetFormat::BINARY: {
         CDataStream ssHeader(SER_NETWORK, PROTOCOL_VERSION);
         for (const CBlockIndex *pindex : headers) {
-            if (pindex->trimmed()) {
+            LOCK(cs_main);
-                CBlockHeader tmp;
+            CBlockIndex tmpBlockIndexFull;
-                node::ReadBlockHeaderFromDisk(tmp, pindex, Params().GetConsensus());
+            const CBlockIndex* pindexfull=pindex->untrim_to(&tmpBlockIndexFull);
-                ssHeader << tmp;
+            ssHeader << pindexfull->GetBlockHeader();
-            } else {
-                ssHeader << pindex->GetBlockHeader();
-            }
         }
 
         std::string binaryHeader = ssHeader.str();
@@ -250,14 +247,10 @@ static bool rest_headers(const std::any& context,
     case RetFormat::HEX: {
         CDataStream ssHeader(SER_NETWORK, PROTOCOL_VERSION);
         for (const CBlockIndex *pindex : headers) {
-            if (pindex->trimmed()) {
+            LOCK(cs_main);
-                CBlockHeader tmp;
+            CBlockIndex tmpBlockIndexFull;
-                node::ReadBlockHeaderFromDisk(tmp, pindex, Params().GetConsensus());
+            const CBlockIndex* pindexfull=pindex->untrim_to(&tmpBlockIndexFull);
-                ssHeader << tmp;
+            ssHeader << pindexfull->GetBlockHeader();
-
-            } else {
-                ssHeader << pindex->GetBlockHeader();
-            }
         }
 
         std::string strHex = HexStr(ssHeader) + "\n";

src/rpc/blockchain.cpp

@@ -193,15 +193,22 @@ CBlockIndex* ParseHashOrHeight(const UniValue& param, ChainstateManager& chainma
     }
 }
 
-UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex)
+UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex_)
 {
     // Serialize passed information without accessing chain state of the active chain!
     AssertLockNotHeld(cs_main); // For performance reasons
 
+    CBlockIndex tmpBlockIndexFull;
+    const CBlockIndex* blockindex;
+    {
+        LOCK(cs_main);
+        blockindex = blockindex_->untrim_to(&tmpBlockIndexFull);
+    }
+
     UniValue result(UniValue::VOBJ);
     result.pushKV("hash", blockindex->GetBlockHash().GetHex());
     const CBlockIndex* pnext;
-    int confirmations = ComputeNextBlockAndDepth(tip, blockindex, pnext);
+    int confirmations = ComputeNextBlockAndDepth(tip, blockindex_, pnext);
     result.pushKV("confirmations", confirmations);
     result.pushKV("height", blockindex->nHeight);
     result.pushKV("version", blockindex->nVersion);
@@ -238,7 +245,7 @@ UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex
         }
     }
     result.pushKV("nTx", (uint64_t)blockindex->nTx);
-    if (blockindex->pprev)
+    if (blockindex_->pprev)
         result.pushKV("previousblockhash", blockindex->pprev->GetBlockHash().GetHex());
     if (pnext)
         result.pushKV("nextblockhash", pnext->GetBlockHash().GetHex());
@@ -1002,7 +1009,7 @@ static RPCHelpMan getblockheader()
     if (!request.params[1].isNull())
         fVerbose = request.params[1].get_bool();
 
-    const CBlockIndex* pblockindex;
+    CBlockIndex* pblockindex;
     const CBlockIndex* tip;
     {
         ChainstateManager& chainman = EnsureAnyChainman(request.context);
@@ -1017,14 +1024,11 @@ static RPCHelpMan getblockheader()
 
     if (!fVerbose)
     {
+        LOCK(cs_main);
         CDataStream ssBlock(SER_NETWORK, PROTOCOL_VERSION);
-        if (pblockindex->trimmed()) {
+        CBlockIndex tmpBlockIndexFull;
-            CBlockHeader tmp;
+        const CBlockIndex* pblockindexfull=pblockindex->untrim_to(&tmpBlockIndexFull);
-            node::ReadBlockHeaderFromDisk(tmp, pblockindex, Params().GetConsensus());
+        ssBlock << pblockindexfull->GetBlockHeader();
-            ssBlock << tmp;
-        } else {
-            ssBlock << pblockindex->GetBlockHeader();
-        }
         std::string strHex = HexStr(ssBlock);
         return strHex;
     }
@@ -1737,6 +1741,7 @@ RPCHelpMan getblockchaininfo()
     }
     obj.pushKV("size_on_disk", chainman.m_blockman.CalculateCurrentUsage());
     obj.pushKV("pruned",                node::fPruneMode);
+    obj.pushKV("trim_headers",          node::fTrimHeaders); // ELEMENTS
     if (g_signed_blocks) {
         if (!DeploymentActiveAfter(tip, chainparams.GetConsensus(), Consensus::DEPLOYMENT_DYNA_FED)) {
             CScript sign_block_script = chainparams.GetConsensus().signblockscript;

src/txdb.cpp

@@ -19,6 +19,7 @@
 
 // ELEMENTS
 #include <block_proof.h> // CheckProof
+#include <chainparams.h> // Params()
 
 static constexpr uint8_t DB_COIN{'C'};
 static constexpr uint8_t DB_COINS{'c'};
@@ -330,39 +331,48 @@ bool CBlockTreeDB::WritePAKList(const std::vector<std::vector<unsigned char> >&
         return Write(std::make_pair(DB_PAK, uint256S("1")), offline_list) && Write(std::make_pair(DB_PAK, uint256S("2")), online_list) && Write(std::make_pair(DB_PAK, uint256S("3")), reject);
 }
 
-/** Note that we only get a conservative (lower) estimate of the max header height here,
+const CBlockIndex *CBlockTreeDB::RegenerateFullIndex(const CBlockIndex *pindexTrimmed, CBlockIndex *pindexNew) const
- * obtained by sampling the first 10,000 headers on disk (which are in random order) and
+{
- * taking the highest block we see. */
+    LOCK(cs_main);
-bool CBlockTreeDB::WalkBlockIndexGutsForMaxHeight(int* nHeight) {
+
-    std::unique_ptr<CDBIterator> pcursor(NewIterator());
+    if(!pindexTrimmed->trimmed()) {
-    *nHeight = 0;
+        return pindexTrimmed;
-    int i = 0;
-    pcursor->Seek(std::make_pair(DB_BLOCK_INDEX, uint256()));
-    while (pcursor->Valid()) {
-        if (ShutdownRequested()) return false;
-        std::pair<uint8_t, uint256> key;
-        if (pcursor->GetKey(key) && key.first == DB_BLOCK_INDEX) {
-            i++;
-            if (i > 10'000) {
-                // Under the (accurate) assumption that the headers on disk are effectively in random height order,
-                //   we have a good-enough (conservative) estimate of the max height very quickly, and don't need to
-                //   waste more time. Shortcutting like this will cause us to keep a few extra headers, which is fine.
-                break;
-            }
-            CDiskBlockIndex diskindex;
-            if (pcursor->GetValue(diskindex)) {
-                if (diskindex.nHeight > *nHeight) {
-                    *nHeight = diskindex.nHeight;
-                }
-                pcursor->Next();
-            } else {
-                return error("%s: failed to read value", __func__);
-            }
-        } else {
-            break;
-        }
     }
-    return true;
+    CBlockHeader tmp;
+    bool BlockRead = false;
+    {
+        // In unpruned nodes, same data could be read from blocks using ReadBlockFromDisk, but that turned out to
+        // be about 6x slower than reading from the index
+        std::pair<uint8_t, uint256> key(DB_BLOCK_INDEX, pindexTrimmed->GetBlockHash());
+        CDiskBlockIndex diskindex;
+        BlockRead = this->Read(key, diskindex);
+        tmp = diskindex.GetBlockHeader();
+    }
+    assert(BlockRead);
+    // Clone the needed data from the original trimmed block
+    pindexNew->pprev          = pindexTrimmed->pprev;
+    pindexNew->phashBlock     = pindexTrimmed->phashBlock;
+    // Construct block index object
+    pindexNew->nHeight        = pindexTrimmed->nHeight;
+    pindexNew->nFile          = pindexTrimmed->nFile;
+    pindexNew->nDataPos       = pindexTrimmed->nDataPos;
+    pindexNew->nUndoPos       = pindexTrimmed->nUndoPos;
+    pindexNew->nVersion       = pindexTrimmed->nVersion;
+    pindexNew->hashMerkleRoot = pindexTrimmed->hashMerkleRoot;
+    pindexNew->nTime          = pindexTrimmed->nTime;
+    pindexNew->nBits          = pindexTrimmed->nBits;
+    pindexNew->nNonce         = pindexTrimmed->nNonce;
+    pindexNew->nStatus        = pindexTrimmed->nStatus;
+    pindexNew->nTx            = pindexTrimmed->nTx;
+
+    pindexNew->proof               = tmp.proof;
+    pindexNew->m_dynafed_params    = tmp.m_dynafed_params;
+    pindexNew->m_signblock_witness = tmp.m_signblock_witness;
+
+    if (pindexTrimmed->nHeight && pindexTrimmed->nHeight % 1000 == 0) {
+        assert(CheckProof(pindexNew->GetBlockHeader(), Params().GetConsensus()));
+    }
+    return pindexNew;
 }
 
 bool CBlockTreeDB::LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function<CBlockIndex*(const uint256&)> insertBlockIndex, int trimBelowHeight)
@@ -396,23 +406,26 @@ bool CBlockTreeDB::LoadBlockIndexGuts(const Consensus::Params& consensusParams,
                 pindexNew->nStatus        = diskindex.nStatus;
                 pindexNew->nTx            = diskindex.nTx;
 
+                pindexNew->proof               = diskindex.proof;
+                pindexNew->m_dynafed_params    = diskindex.m_dynafed_params;
+                pindexNew->m_signblock_witness = diskindex.m_signblock_witness;
+
+                assert(!(g_signed_blocks && diskindex.m_dynafed_params.value().IsNull() && diskindex.proof.value().IsNull()));
+
+                pindexNew->set_stored();
                 n_total++;
+
+                const uint256 block_hash = pindexNew->GetBlockHash();
+                // Only validate one of every 1000 block header for sanity check
+                if (pindexNew->nHeight % 1000 == 0 &&
+                        block_hash != consensusParams.hashGenesisBlock &&
+                        !CheckProof(pindexNew->GetBlockHeader(), consensusParams)) {
+                    return error("%s: CheckProof: %s, %s", __func__, block_hash.ToString(), pindexNew->ToString());
+                }
                 if (diskindex.nHeight >= trimBelowHeight) {
                     n_untrimmed++;
-                    pindexNew->proof               = diskindex.proof;
-                    pindexNew->m_dynafed_params    = diskindex.m_dynafed_params;
-                    pindexNew->m_signblock_witness = diskindex.m_signblock_witness;
-
-                    const uint256 block_hash = pindexNew->GetBlockHash();
-                    // Only validate one of every 1000 block header for sanity check
-                    if (pindexNew->nHeight % 1000 == 0 &&
-                            block_hash != consensusParams.hashGenesisBlock &&
-                            !CheckProof(pindexNew->GetBlockHeader(), consensusParams)) {
-                        return error("%s: CheckProof: %s, %s", __func__, block_hash.ToString(), pindexNew->ToString());
-                    }
                 } else {
-                    pindexNew->m_trimmed = true;
+                    pindexNew->trim();
-                    pindexNew->m_trimmed_dynafed_block = !diskindex.m_dynafed_params.value().IsNull();
                 }
 
                 pcursor->Next();

src/txdb.h

@@ -91,7 +91,7 @@ public:
     bool LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function<CBlockIndex*(const uint256&)> insertBlockIndex, int trimBelowHeight)
         EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
     // ELEMENTS:
-    bool WalkBlockIndexGutsForMaxHeight(int* nHeight);
+    const CBlockIndex *RegenerateFullIndex(const CBlockIndex *pindexTrimmed, CBlockIndex *pindexNew) const;
     bool ReadPAKList(std::vector<std::vector<unsigned char> >& offline_list, std::vector<std::vector<unsigned char> >& online_list, bool& reject);
     bool WritePAKList(const std::vector<std::vector<unsigned char> >& offline_list, const std::vector<std::vector<unsigned char> >& online_list, bool reject);
 };

src/validation.cpp

@@ -2568,38 +2568,53 @@ bool CChainState::FlushStateToDisk(
                 m_blockman.FlushBlockFile();
             }
 
+            std::set<CBlockIndex*> setTrimmableBlockIndex(m_blockman.m_dirty_blockindex);
             // Then update all block file information (which may refer to block and undo files).
             {
                 LOG_TIME_MILLIS_WITH_CATEGORY("write block index to disk", BCLog::BENCH);
 
+                if (node::fTrimHeaders) {
+                    for (std::set<CBlockIndex*>::iterator it = setTrimmableBlockIndex.begin(); it != setTrimmableBlockIndex.end(); it++) {
+                        (*it)->untrim();
+                    }
+                }
                 if (!m_blockman.WriteBlockIndexDB()) {
                     return AbortNode(state, "Failed to write to block index database");
                 }
 
-                if (node::fTrimHeaders) {
+                // This should be done inside WriteBatchSync, but CBlockIndex is const there
-                    std::set<CBlockIndex*> setTrimmableBlockIndex(m_blockman.m_dirty_blockindex);
+                for (std::set<CBlockIndex*>::iterator it = setTrimmableBlockIndex.begin(); it != setTrimmableBlockIndex.end(); it++) {
+                    (*it)->set_stored();
+                }
+
+                int trim_height = 0;
+                if (pindexBestHeader && (uint64_t)pindexBestHeader->nHeight > node::nMustKeepFullHeaders) { // check first, to prevent underflow
+                    trim_height = pindexBestHeader->nHeight - node::nMustKeepFullHeaders;
+                }
+                if (node::fTrimHeaders && trim_height > 0 && !ShutdownRequested()) {
+                    static int nMinTrimHeight{0};
                     LogPrintf("Flushing block index, trimming headers, setTrimmableBlockIndex.size(): %d\n", setTrimmableBlockIndex.size());
-                    int trim_height = m_chain.Height() - node::nMustKeepFullHeaders;
-                    int min_height = std::numeric_limits<int>::max();
-                    CBlockIndex* min_index = nullptr;
                     for (std::set<CBlockIndex*>::iterator it = setTrimmableBlockIndex.begin(); it != setTrimmableBlockIndex.end(); it++) {
                         (*it)->assert_untrimmed();
                         if ((*it)->nHeight < trim_height) {
                             (*it)->trim();
-                            if ((*it)->nHeight < min_height) {
-                                min_height = (*it)->nHeight;
-                                min_index = *it;
-                            }
                         }
                     }
-
+                    CBlockIndex* min_index = pindexBestHeader->GetAncestor(trim_height-1);
                     // Handle any remaining untrimmed blocks that were too recent for trimming last time we flushed.
                     if (min_index) {
-                        min_index = min_index->pprev;
+                        int nMaxTrimHeightRound = std::max(nMinTrimHeight, min_index->nHeight + 1);
-                        while (min_index && !min_index->trimmed()) {
+                        while (min_index && min_index->nHeight >= nMinTrimHeight) {
-                            min_index->trim();
+                            if (!min_index->trimmed()) {
+                                // there may be gaps due to untrimmed blocks, we need to check them all
+                                if (!min_index->trim()) {
+                                    // Header could not be trimmed, we'll need to try again next round
+                                    nMaxTrimHeightRound = min_index->nHeight;
+                                }
+                            }
                             min_index = min_index->pprev;
                         }
+                        nMinTrimHeight = nMaxTrimHeightRound;
                     }
                 }
             }
@@ -2702,6 +2717,18 @@ static void UpdateTipLog(
         !warning_messages.empty() ? strprintf(" warning='%s'", warning_messages) : "");
 }
 
+void ForceUntrimHeader(const CBlockIndex *pindex_) EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
+{
+    AssertLockHeld(cs_main);
+
+    assert(pindex_);
+    if (!pindex_->trimmed()) {
+        return;
+    }
+    CBlockIndex *pindex=const_cast<CBlockIndex*>(pindex_);
+    pindex->untrim();
+}
+
 void CChainState::UpdateTip(const CBlockIndex* pindexNew)
 {
     AssertLockHeld(::cs_main);
@@ -2747,11 +2774,13 @@ void CChainState::UpdateTip(const CBlockIndex* pindexNew)
     }
     UpdateTipLog(coins_tip, pindexNew, m_params, __func__, "", warning_messages.original);
 
+    ForceUntrimHeader(pindexNew);
     // Do some logging if dynafed parameters changed.
     if (pindexNew->pprev && !pindexNew->dynafed_params().IsNull()) {
         int height = pindexNew->nHeight;
         uint256 hash = pindexNew->GetBlockHash();
         uint256 root = pindexNew->dynafed_params().m_current.CalculateRoot();
+        ForceUntrimHeader(pindexNew->pprev);
         if (pindexNew->pprev->dynafed_params().IsNull()) {
             LogPrintf("Dynafed activated in block %d:%s: %s\n", height, hash.GetHex(), root.GetHex());
         } else if (root != pindexNew->pprev->dynafed_params().m_current.CalculateRoot()) {

src/validation.h

@@ -1023,4 +1023,5 @@ bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mocka
  */
 const AssumeutxoData* ExpectedAssumeutxo(const int height, const CChainParams& params);
 
+void ForceUntrimHeader(const CBlockIndex *pindex_);
 #endif // BITCOIN_VALIDATION_H

test/functional/feature_trim_headers.py

@@ -0,0 +1,264 @@
+#!/usr/bin/env python3
+
+import codecs
+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import assert_equal
+from test_framework import (
+    address,
+    key,
+)
+from test_framework.messages import (
+    CBlock,
+    from_hex,
+)
+from test_framework.script import (
+    OP_NOP,
+    OP_RETURN,
+    CScript
+)
+
+# Generate wallet import format from private key.
+def wif(pk):
+    # Base58Check version for regtest WIF keys is 0xef = 239
+    pk_compressed = pk + bytes([0x1])
+    return address.byte_to_base58(pk_compressed, 239)
+
+# The signblockscript is a Bitcoin Script k-of-n multisig script.
+def make_signblockscript(num_nodes, required_signers, keys):
+    assert num_nodes >= required_signers
+    script = "{}".format(50 + required_signers)
+    for i in range(num_nodes):
+        k = keys[i]
+        script += "21"
+        script += codecs.encode(k.get_pubkey().get_bytes(), 'hex_codec').decode("utf-8")
+    script += "{}".format(50 + num_nodes)  # num keys
+    script += "ae"  # OP_CHECKMULTISIG
+    return script
+
+class TrimHeadersTest(BitcoinTestFramework):
+    def skip_test_if_missing_module(self):
+        self.skip_if_no_wallet()
+
+    # Dynamically generate N keys to be used for block signing.
+    def init_keys(self, num_keys):
+        self.keys = []
+        self.wifs = []
+        for i in range(num_keys):
+            k = key.ECKey()
+            k.generate()
+            w = wif(k.get_bytes())
+            self.keys.append(k)
+            self.wifs.append(w)
+
+    def set_test_params(self):
+        self.num_nodes = 3
+        self.num_keys = 1
+        self.required_signers = 1
+        self.setup_clean_chain = True
+        self.init_keys(self.num_keys)
+        signblockscript = make_signblockscript(self.num_keys, self.required_signers, self.keys)
+        self.witnessScript = signblockscript  # post-dynafed this becomes witnessScript
+        args = [
+            "-signblockscript={}".format(signblockscript),
+            "-con_max_block_sig_size={}".format(self.required_signers * 74 + self.num_nodes * 33),
+            "-anyonecanspendaremine=1",
+            "-evbparams=dynafed:0:::",
+            "-con_dyna_deploy_signal=1",
+        ]
+        self.trim_args = args + ["-trim_headers=1"]
+        self.prune_args = self.trim_args + ["-prune=1"]
+        self.extra_args = [
+            args,
+            self.trim_args,
+            self.prune_args,
+        ]
+
+    def setup_network(self):
+        self.setup_nodes()
+        self.connect_nodes(0, 1)
+        self.connect_nodes(0, 2)
+
+    def check_height(self, expected_height, all=False, verbose=True):
+        if verbose:
+            self.log.info(f"Check height {expected_height}")
+        if all:
+            for n in self.nodes:
+                assert_equal(n.getblockcount(), expected_height)
+        else:
+            assert_equal(self.nodes[0].getblockcount(), expected_height)
+
+    def mine_block(self, make_transactions):
+        # alternate mining between the signing nodes
+        mineridx = self.nodes[0].getblockcount() % self.required_signers  # assuming in sync
+        mineridx_next = (self.nodes[0].getblockcount() + 1) % self.required_signers
+        miner = self.nodes[mineridx]
+        miner_next = self.nodes[mineridx_next]
+
+        # If dynafed is enabled, this means signblockscript has been WSH-wrapped
+        blockchain_info = self.nodes[0].getblockchaininfo()
+        deployment_info = self.nodes[0].getdeploymentinfo()
+        dynafed_active = deployment_info['deployments']['dynafed']['bip9']['status'] == "active"
+        if dynafed_active:
+            wsh_wrap = self.nodes[0].decodescript(self.witnessScript)['segwit']['hex']
+            assert_equal(wsh_wrap, blockchain_info['current_signblock_hex'])
+
+        # Make a few transactions to make non-empty blocks for compact transmission
+        if make_transactions:
+            for i in range(10):
+                miner.sendtoaddress(miner_next.getnewaddress(), 1, "", "", True)
+        # miner makes a block
+        block = miner.getnewblockhex()
+        block_struct = from_hex(CBlock(), block)
+
+        # make another block with the commitment field filled out
+        dummy_block = miner.getnewblockhex(commit_data="deadbeef")
+        dummy_struct = from_hex(CBlock(), dummy_block)
+        assert_equal(len(dummy_struct.vtx[0].vout), len(block_struct.vtx[0].vout) + 1)
+        # OP_RETURN deadbeef
+        assert_equal(CScript(dummy_struct.vtx[0].vout[0].scriptPubKey).hex(), '6a04deadbeef')
+
+        # All nodes get compact blocks, first node may get complete
+        # block in 0.5 RTT even with transactions thanks to p2p connection
+        # with non-signing node being miner
+        for i in range(self.num_keys):
+            sketch = miner.getcompactsketch(block)
+            compact_response = self.nodes[i].consumecompactsketch(sketch)
+            if "block_tx_req" in compact_response:
+                block_txn = self.nodes[i].consumegetblocktxn(block, compact_response["block_tx_req"])
+                final_block = self.nodes[i].finalizecompactblock(sketch, block_txn, compact_response["found_transactions"])
+            else:
+                # If there's only coinbase, it should succeed immediately
+                final_block = compact_response["blockhex"]
+            # Block should be complete, sans signatures
+            self.nodes[i].testproposedblock(final_block)
+
+        # collect num_keys signatures from signers, reduce to required_signers sigs during combine
+        sigs = []
+        for i in range(self.num_keys):
+            result = miner.combineblocksigs(block, sigs, self.witnessScript)
+            sigs = sigs + self.nodes[i].signblock(block, self.witnessScript)
+            assert_equal(result["complete"], i >= self.required_signers)
+            # submitting should have no effect pre-threshhold
+            if i < self.required_signers:
+                miner.submitblock(result["hex"])
+
+        result = miner.combineblocksigs(block, sigs, self.witnessScript)
+        assert_equal(result["complete"], True)
+
+        self.nodes[0].submitblock(result["hex"])
+
+    def mine_blocks(self, num_blocks, transactions):
+        for _ in range(num_blocks):
+            self.mine_block(transactions)
+
+    def mine_large_blocks(self, n):
+        big_script = CScript([OP_RETURN] + [OP_NOP] * 950000)
+        node = self.nodes[0]
+
+        for _ in range(n):
+            hex = node.getnewblockhex()
+            block = from_hex(CBlock(), hex)
+            tx = block.vtx[0]
+            tx.vout[0].scriptPubKey = big_script
+            tx.rehash()
+            block.vtx[0] = tx
+            block.hashMerkleRoot = block.calc_merkle_root()
+            block.solve()
+            h = block.serialize().hex()
+
+            sigs = node.signblock(h, self.witnessScript)
+
+            result = node.combineblocksigs(h, sigs, self.witnessScript)
+            assert_equal(result["complete"], True)
+
+            node.submitblock(result["hex"])
+
+
+    def run_test(self):
+        for i in range(self.num_keys):
+            self.nodes[i].importprivkey(self.wifs[i])
+
+        expected_height = 0
+        self.check_height(expected_height, all=True)
+
+        self.log.info("Mining and signing 101 blocks to unlock funds")
+        expected_height += 101
+        self.mine_blocks(101, False)
+        self.sync_all()
+        self.check_height(expected_height, all=True)
+        # check the new field in getblockchaininfo
+        assert not self.nodes[0].getblockchaininfo()["trim_headers"]
+        assert self.nodes[1].getblockchaininfo()["trim_headers"]
+        assert self.nodes[2].getblockchaininfo()["trim_headers"]
+
+        self.log.info("Shut down trimmed nodes")
+        self.stop_node(1)
+        self.stop_node(2)
+
+        self.log.info("Mining and signing non-empty blocks")
+        expected_height += 10
+        self.mine_blocks(10, True)
+        self.check_height(expected_height)
+
+        # signblock rpc field stuff
+        tip = self.nodes[0].getblockhash(self.nodes[0].getblockcount())
+        header = self.nodes[0].getblockheader(tip)
+        block = self.nodes[0].getblock(tip)
+
+        assert 'signblock_witness_asm' in header
+        assert 'signblock_witness_hex' in header
+        assert 'signblock_witness_asm' in block
+        assert 'signblock_witness_hex' in block
+
+        assert_equal(self.nodes[0].getdeploymentinfo()['deployments']['dynafed']['bip9']['status'], "defined")
+
+        # activate dynafed
+        blocks_til_dynafed = 431 - self.nodes[0].getblockcount()
+        self.log.info("Activating dynafed")
+        self.mine_blocks(blocks_til_dynafed, False)
+        expected_height += blocks_til_dynafed
+        self.check_height(expected_height)
+
+        assert_equal(self.nodes[0].getdeploymentinfo()['deployments']['dynafed']['bip9']['status'], "locked_in")
+
+        num = 3000
+        self.log.info(f"Mine {num} dynamic federation blocks without txns")
+        self.mine_blocks(num, False)
+        expected_height += num
+        self.check_height(expected_height)
+
+        num = 10
+        self.log.info(f"Mine {num} dynamic federation blocks with txns")
+        self.mine_blocks(num, True)
+        expected_height += num
+        self.check_height(expected_height)
+
+        num = 777
+        self.log.info(f"Mine {num} large blocks")
+        expected_height += num
+        self.mine_large_blocks(num)
+
+        self.log.info("Restart the trimmed nodes")
+        self.start_node(1, extra_args=self.trim_args)
+        self.start_node(2, extra_args=self.prune_args)
+        self.connect_nodes(0, 1)
+        self.connect_nodes(0, 2)
+
+        self.sync_all()
+        self.check_height(expected_height, all=True)
+
+        self.log.info("Prune the pruned node")
+        self.nodes[2].pruneblockchain(4000)
+
+        hash = self.nodes[0].getblockhash(expected_height)
+        block = self.nodes[0].getblock(hash)
+        for i in range(1, self.num_nodes):
+            assert_equal(hash, self.nodes[i].getblockhash(expected_height))
+            assert_equal(block, self.nodes[i].getblock(hash))
+
+        self.log.info(f"All nodes at height {expected_height} with block hash {hash}")
+
+
+if __name__ == '__main__':
+    TrimHeadersTest().main()

test/functional/rpc_blockchain.py

@@ -123,6 +123,7 @@ class BlockchainTest(BitcoinTestFramework):
             'pruned',
             'size_on_disk',
             'time',
+            'trim_headers',
             'verificationprogress',
             'warnings',
         ]

test/functional/test_runner.py

@@ -112,6 +112,7 @@ BASE_SCRIPTS = [
     'rpc_getnewblockhex.py',
     'wallet_elements_regression_1172.py --legacy-wallet',
     'wallet_elements_regression_1259.py --legacy-wallet',
+    'feature_trim_headers.py',
     # Longest test should go first, to favor running tests in parallel
     'wallet_hd.py --legacy-wallet',
     'wallet_hd.py --descriptors',

test/lint/lint-circular-dependencies.sh

@@ -22,6 +22,29 @@ EXPECTED_CIRCULAR_DEPENDENCIES=(
     "wallet/fees -> wallet/wallet -> wallet/fees"
     "wallet/wallet -> wallet/walletdb -> wallet/wallet"
     "node/coinstats -> validation -> node/coinstats"
+    # ELEMENTs: introduced by https://github.com/ElementsProject/elements/pull/1270
+    "chain -> validation -> chain"
+    "chain -> validation -> consensus/tx_verify -> chain"
+    "dynafed -> validation -> dynafed"
+    "pegins -> validation -> pegins"
+    "chain -> node/context -> txmempool -> chain"
+    "chain -> validation -> deploymentstatus -> chain"
+    "chain -> validation -> index/blockfilterindex -> chain"
+    "chain -> validation -> primitives/pak -> chain"
+    "chain -> validation -> txdb -> chain"
+    "chain -> validation -> validationinterface -> chain"
+    "chain -> validation -> txdb -> pow -> chain"
+    "chain -> validation -> deploymentstatus -> versionbits -> chain"
+    "confidential_validation -> pegins -> validation -> confidential_validation"
+    "consensus/tx_verify -> pegins -> validation -> consensus/tx_verify"
+    "dynafed -> validation -> primitives/pak -> dynafed"
+    "pegins -> validation -> txmempool -> pegins"
+    "block_proof -> chain -> validation -> block_proof"
+    "block_proof -> chain -> validation -> txdb -> block_proof"
+    "chain -> node/context -> net_processing -> node/blockstorage -> chain"
+    "consensus/tx_verify -> pegins -> validation -> txmempool -> consensus/tx_verify"
+    "block_proof -> chain -> node/context -> net_processing -> node/blockstorage -> block_proof"
+    "core_io -> script/sign -> pegins -> validation -> signet -> core_io"
     # ELEMENTS: will be fixed by blinding cleanup
     "blindpsbt -> psbt -> blindpsbt"
     # ELEMENTS: not so easy to fix, caused by us doing asset ID lookups in the