Backports and changes to fix CI

.cirrus.yml

@@ -156,7 +156,7 @@ task:
 
 task:
   name: '[no depends, sanitizers: fuzzer,address,undefined,integer] [focal]'
-  only_if: $CIRRUS_BRANCH == $CIRRUS_DEFAULT_BRANCH || $CIRRUS_BASE_BRANCH == $CIRRUS_DEFAULT_BRANCH
+  # only_if: $CIRRUS_BRANCH == $CIRRUS_DEFAULT_BRANCH || $CIRRUS_BASE_BRANCH == $CIRRUS_DEFAULT_BRANCH
   << : *GLOBAL_TASK_TEMPLATE
   container:
     image: ubuntu:focal

src/chain.cpp

@@ -5,6 +5,8 @@
 
 #include <chain.h>
 
+#include <validation.h> // pblocktree
+
 /**
  * CChain implementation
  */
@@ -48,6 +50,24 @@ CBlockLocator CChain::GetLocator(const CBlockIndex *pindex) const {
     return CBlockLocator(vHave);
 }
 
+void CBlockIndex::untrim() {
+    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;
+}
+
+const CBlockIndex *CBlockIndex::untrim_to(CBlockIndex *pindexNew) const
+{
+    return pblocktree->RegenerateFullIndex(this, pindexNew);
+}
+
 const CBlockIndex *CChain::FindFork(const CBlockIndex *pindex) const {
     if (pindex == nullptr) {
         return nullptr;

src/chain.h

@@ -201,26 +201,38 @@ class CBlockIndex
 
     bool m_trimmed{false};
     bool m_trimmed_dynafed_block{false};
+    bool m_stored_lvl{false};
 
     friend class CBlockTreeDB;
 
 public:
 
     // 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);
     }
@@ -463,6 +475,9 @@ class CDiskBlockIndex : public CBlockIndex
 
         // 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,8 +1,12 @@
 
 #include <dynafed.h>
 #include <hash.h>
+#include <validation.h>
 
-bool NextBlockIsParameterTransition(const CBlockIndex* pindexPrev, const Consensus::Params& consensus, DynaFedParamEntry& winning_entry)
+
+/* Returns true if the next block would be the first block of an epoch with new
+ * parameters. It also returns the parameter set that is being transitioned to. */
+static bool NextBlockIsParameterTransition(const CBlockIndex* pindexPrev, const Consensus::Params& consensus, DynaFedParamEntry& winning_entry)  EXCLUSIVE_LOCKS_REQUIRED(cs_main)
 {
     uint32_t next_height = pindexPrev->nHeight + 1;
     assert(consensus.dynamic_epoch_length != 0);
@@ -15,6 +19,7 @@ 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);
+        ForceUntrimHeader(p_epoch_walk);
         const DynaFedParamEntry& proposal = p_epoch_walk->dynafed_params().m_proposed;
         const uint256 proposal_root = proposal.CalculateRoot();
         vote_tally[proposal_root]++;
@@ -37,6 +42,7 @@ DynaFedParamEntry ComputeNextBlockFullCurrentParameters(const CBlockIndex* pinde
     const uint32_t epoch_length = consensus.dynamic_epoch_length;
     uint32_t epoch_age = next_height % epoch_length;
 
+    LOCK(cs_main);
     DynaFedParamEntry winning_proposal;
     // Early return when there is a winning proposal
     if (NextBlockIsParameterTransition(pindexPrev, consensus, winning_proposal)) {
@@ -60,6 +66,7 @@ DynaFedParamEntry ComputeNextBlockFullCurrentParameters(const CBlockIndex* pinde
     // may be pre-dynafed params
     const CBlockIndex* p_epoch_start = pindexPrev->GetAncestor(epoch_start_height);
     assert(p_epoch_start);
+    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 +100,8 @@ DynaFedParamEntry ComputeNextBlockCurrentParameters(const CBlockIndex* pindexPre
 {
     assert(pindexPrev);
 
+    LOCK(cs_main);
+    ForceUntrimHeader(pindexPrev);
     DynaFedParamEntry entry = ComputeNextBlockFullCurrentParameters(pindexPrev, consensus);
 
     uint32_t next_height = pindexPrev->nHeight+1;

src/dynafed.h

@@ -6,10 +6,6 @@
 #include <chainparams.h>
 #include <primitives/block.h>
 
-/* Returns true if the next block would be the first block of an epoch with new
- * parameters. It also returns the parameter set that is being transitioned to. */
-bool NextBlockIsParameterTransition(const CBlockIndex* pindexPrev, const Consensus::Params& consensus, DynaFedParamEntry& winning_entry);
-
 /* Compute the next block's enforced parameters */
 DynaFedParamEntry ComputeNextBlockFullCurrentParameters(const CBlockIndex* pindexPrev, const Consensus::Params& consensus);
 /* Compute the next block's expected published parameters. Blocks at "epoch_age" of non-0 only

src/init.cpp

@@ -987,13 +987,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");
         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.
 
-        nMustKeepFullHeaders = (chainparams.GetConsensus().total_valid_epochs + 2) * epoch_length;
+        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.
@@ -1711,7 +1711,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 || fTrimHeaders) {
+    if (fPruneMode) {
         LogPrintf("Unsetting NODE_NETWORK on prune mode\n");
         nLocalServices = ServiceFlags(nLocalServices & ~NODE_NETWORK);
         if (!fReindex) {
@@ -1723,11 +1723,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         }
     }
 
-    if (fTrimHeaders) {
-        LogPrintf("Unsetting NODE_NETWORK_LIMITED on header trim mode\n");
-        nLocalServices = ServiceFlags(nLocalServices & ~NODE_NETWORK_LIMITED);
-    }
-
     if (DeploymentEnabled(chainparams.GetConsensus(), Consensus::DEPLOYMENT_SEGWIT)) {
         // Advertise witness capabilities.
         // The option to not set NODE_WITNESS is only used in the tests and should be removed.

src/net.cpp

@@ -1361,6 +1361,9 @@ bool CConnman::GenerateSelectSet(std::set<SOCKET> &recv_set, std::set<SOCKET> &s
             //   write buffer in this case before receiving more. This avoids
             //   needlessly queueing received data, if the remote peer is not themselves
             //   receiving data. This means properly utilizing TCP flow control signalling.
+            //   This logic can put both nodes in deadlock if they are both "not receiving",
+            //   so there is a special case where we only stop receiving new messages, but
+            //   keep processing the in-progress ones.
             // * Otherwise, if there is space left in the receive buffer, select() for
             //   receiving data.
             // * Hand off all complete messages to the processor, to be handled without
@@ -1380,7 +1383,9 @@ bool CConnman::GenerateSelectSet(std::set<SOCKET> &recv_set, std::set<SOCKET> &s
             error_set.insert(pnode->hSocket);
             if (select_send) {
                 send_set.insert(pnode->hSocket);
-                continue;
+                // Only stop receiving new messages, but keep processing incomplete ones
+                if (!pnode->m_deserializer->IsEmpty())
+                    continue;
             }
             if (select_recv) {
                 recv_set.insert(pnode->hSocket);

src/net.h

@@ -302,6 +302,8 @@ class CNetMessage {
  */
 class TransportDeserializer {
 public:
+    // returns true if the current deserialization is empty
+    virtual bool IsEmpty() const = 0;
     // returns true if the current deserialization is complete
     virtual bool Complete() const = 0;
     // set the serialization context version
@@ -352,6 +354,10 @@ class V1TransportDeserializer final : public TransportDeserializer
         Reset();
     }
 
+    bool IsEmpty() const override
+    {
+        return (nHdrPos == 0);
+    }
     bool Complete() const override
     {
         if (!in_data)

src/net_processing.cpp

@@ -3183,12 +3183,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,
-                //   just drop the request.
-                LogPrint(BCLog::NET, "%s: ignoring getheaders from peer=%i which would return at least one trimmed header\n", __func__, pfrom.GetId());
-                return;
+                // Header is trimmed, reload from disk before sending
+                CBlockIndex tmpBlockIndexFull;
+                const CBlockIndex* pindexfull = pindex->untrim_to(&tmpBlockIndexFull);
+                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

@@ -408,17 +408,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;
-    }
-    header = tmp.GetBlockHeader();
-    return true;
-}
-
 bool ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos, const CMessageHeader::MessageStartChars& message_start)
 {
     FlatFilePos hpos = pos;

src/node/blockstorage.h

@@ -79,7 +79,6 @@ bool ReadBlockFromDisk(CBlock& block, const CBlockIndex* pindex, const Consensus
 bool ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatFilePos& pos, const CMessageHeader::MessageStartChars& message_start);
 bool ReadRawBlockFromDisk(std::vector<uint8_t>& block, const CBlockIndex* pindex, 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);
 bool WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValidationState& state, CBlockIndex* pindex, const CChainParams& chainparams);

src/pegins.cpp

@@ -26,6 +26,8 @@
 // ELEMENTS
 //
 
+#include <validation.h>
+
 namespace {
 static secp256k1_context* secp256k1_ctx_validation;
 
@@ -473,6 +475,7 @@ std::vector<std::pair<CScript, CScript>> GetValidFedpegScripts(const CBlockIndex
         fedpegscripts.push_back(std::make_pair(next_param.m_fedpeg_program, next_param.m_fedpegscript));
     }
 
+    LOCK(cs_main);
     // Next we walk backwards up to M epoch starts
     for (int32_t i = 0; i < (int32_t) params.total_valid_epochs; i++) {
         // We are within total_valid_epochs of the genesis
@@ -487,6 +490,7 @@ std::vector<std::pair<CScript, CScript>> GetValidFedpegScripts(const CBlockIndex
             break;
         }
 
+        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/rest.cpp

@@ -221,13 +221,9 @@ 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()) {
-                CBlockHeader tmp;
-                ReadBlockHeaderFromDisk(tmp, pindex, Params().GetConsensus());
-                ssHeader << tmp;
-            } else {
-                ssHeader << pindex->GetBlockHeader();
-            }
+            CBlockIndex tmpBlockIndexFull;
+            const CBlockIndex* pindexfull = pindex->untrim_to(&tmpBlockIndexFull);
+            ssHeader << pindexfull->GetBlockHeader();
         }
 
         std::string binaryHeader = ssHeader.str();
@@ -239,14 +235,9 @@ 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()) {
-                CBlockHeader tmp;
-                ReadBlockHeaderFromDisk(tmp, pindex, Params().GetConsensus());
-                ssHeader << tmp;
-
-            } else {
-                ssHeader << pindex->GetBlockHeader();
-            }
+            CBlockIndex tmpBlockIndexFull;
+            const CBlockIndex* pindexfull = pindex->untrim_to(&tmpBlockIndexFull);
+            ssHeader << pindexfull->GetBlockHeader();
         }
 
         std::string strHex = HexStr(ssHeader) + "\n";

src/rpc/blockchain.cpp

@@ -226,15 +226,18 @@ 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 = 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);
@@ -271,7 +274,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());
@@ -966,7 +969,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);
@@ -982,13 +985,9 @@ static RPCHelpMan getblockheader()
     if (!fVerbose)
     {
         CDataStream ssBlock(SER_NETWORK, PROTOCOL_VERSION);
-        if (pblockindex->trimmed()) {
-            CBlockHeader tmp;
-            ReadBlockHeaderFromDisk(tmp, pblockindex, Params().GetConsensus());
-            ssBlock << tmp;
-        } else {
-            ssBlock << pblockindex->GetBlockHeader();
-        }
+        CBlockIndex tmpBlockIndexFull;
+        const CBlockIndex* pblockindexfull = pblockindex->untrim_to(&tmpBlockIndexFull);
+        ssBlock << pblockindexfull->GetBlockHeader();
         std::string strHex = HexStr(ssBlock);
         return strHex;
     }
@@ -2261,6 +2260,7 @@ static RPCHelpMan getblockstats()
                     if (out.nValue.IsExplicit() && out.nAsset.IsExplicit() && out.nAsset.GetAsset() == asset) {
                         tx_total_out += out.nValue.GetAmount();
                     }
+                    utxo_size_inc += GetSerializeSize(out, PROTOCOL_VERSION) + PER_UTXO_OVERHEAD;
                 }
             }
         } else {