refactor: untangle assumevalid decision branches

Flatten nested conditionals into a linear gating sequence for readability and precise logging. No functional change, TODOs are addressed in next commit

src/validation.cpp

@@ -2424,33 +2424,42 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
     }
 
     bool fScriptChecks = true;
-    if (!m_chainman.AssumedValidBlock().IsNull()) {
+    if (m_chainman.AssumedValidBlock().IsNull()) {
+        // TODO
+    } else {
+        constexpr int64_t TWO_WEEKS_IN_SECONDS{60 * 60 * 24 * 7 * 2};
         // We've been configured with the hash of a block which has been externally verified to have a valid history.
         // A suitable default value is included with the software and updated from time to time.  Because validity
         //  relative to a piece of software is an objective fact these defaults can be easily reviewed.
         // This setting doesn't force the selection of any particular chain but makes validating some faster by
         //  effectively caching the result of part of the verification.
         BlockMap::const_iterator it{m_blockman.m_block_index.find(m_chainman.AssumedValidBlock())};
-        if (it != m_blockman.m_block_index.end()) {
+        if (it == m_blockman.m_block_index.end()) {
-            if (it->second.GetAncestor(pindex->nHeight) == pindex &&
+            // TODO
-                m_chainman.m_best_header->GetAncestor(pindex->nHeight) == pindex &&
+        } else if (it->second.GetAncestor(pindex->nHeight) != pindex) {
-                m_chainman.m_best_header->nChainWork >= m_chainman.MinimumChainWork()) {
+            // TODO
-                // This block is a member of the assumed verified chain and an ancestor of the best header.
+        } else if (m_chainman.m_best_header->GetAncestor(pindex->nHeight) != pindex) {
-                // Script verification is skipped when connecting blocks under the
+            // TODO
-                // assumevalid block. Assuming the assumevalid block is valid this
+        } else if (m_chainman.m_best_header->nChainWork < m_chainman.MinimumChainWork()) {
-                // is safe because block merkle hashes are still computed and checked,
+            // TODO
-                // Of course, if an assumed valid block is invalid due to false scriptSigs
+        } else if (GetBlockProofEquivalentTime(*m_chainman.m_best_header, *pindex, *m_chainman.m_best_header, params.GetConsensus()) <= TWO_WEEKS_IN_SECONDS) {
-                // this optimization would allow an invalid chain to be accepted.
+            // TODO
-                // The equivalent time check discourages hash power from extorting the network via DOS attack
+        } else {
-                //  into accepting an invalid block through telling users they must manually set assumevalid.
+            // This block is a member of the assumed verified chain and an ancestor of the best header.
-                //  Requiring a software change or burying the invalid block, regardless of the setting, makes
+            // Script verification is skipped when connecting blocks under the
-                //  it hard to hide the implication of the demand.  This also avoids having release candidates
+            //  assumevalid block. Assuming the assumevalid block is valid this
-                //  that are hardly doing any signature verification at all in testing without having to
+            //  is safe because block merkle hashes are still computed and checked,
-                //  artificially set the default assumed verified block further back.
+            // Of course, if an assumed valid block is invalid due to false scriptSigs
-                // The test against the minimum chain work prevents the skipping when denied access to any chain at
+            //  this optimization would allow an invalid chain to be accepted.
-                //  least as good as the expected chain.
+            // The equivalent time check discourages hash power from extorting the network via DOS attack
-                fScriptChecks = (GetBlockProofEquivalentTime(*m_chainman.m_best_header, *pindex, *m_chainman.m_best_header, params.GetConsensus()) <= 60 * 60 * 24 * 7 * 2);
+            //  into accepting an invalid block through telling users they must manually set assumevalid.
-            }
+            //  Requiring a software change or burying the invalid block, regardless of the setting, makes
+            //  it hard to hide the implication of the demand. This also avoids having release candidates
+            //  that are hardly doing any signature verification at all in testing without having to
+            //  artificially set the default assumed verified block further back.
+            // The test against the minimum chain work prevents the skipping when denied access to any chain at
+            //  least as good as the expected chain.
+            fScriptChecks = false;
         }
     }