validation: remove redundant check on pindex #20868

pull jarolrod wants to merge 1 commits into bitcoin:master from jarolrod:redundant-check-validation changing 1 files +2 −4

jarolrod commented at 8:06 PM on January 6, 2021: member

This removes a redundant check on pindex being a nullptr. By the time we get to this step pindex is always a nullptr as the branch where it has been set would have already returned.

Closes #19223
DrahtBot added the label Validation on Jan 6, 2021
mjdietzx commented at 9:40 PM on January 6, 2021: contributor

Concept ACK
Zero-1729 commented at 10:09 PM on January 6, 2021: contributor

ACK

ajtowns commented at 5:22 AM on January 7, 2021: member

Perhaps make it:

     BlockMap::iterator miSelf = m_block_index.find(hash);
-    CBlockIndex *pindex = nullptr;
     if (hash != chainparams.GetConsensus().hashGenesisBlock) {
         if (miSelf != m_block_index.end()) {
             // Block header is already known.
-            pindex = miSelf->second;
+            CBlockIndex *pindex = miSelf->second;
             if (ppindex)
                 *ppindex = pindex;
             if (pindex->nStatus & BLOCK_FAILED_MASK) {
@@ -3621,7 +3620,7 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationS
             }
         }
     }
-    pindex = AddToBlockIndex(block);
+    CBlockIndex *pindex = AddToBlockIndex(block);

so it's clear that the earlier pindex and the later pindex are independent?

jarolrod force-pushed on Jan 8, 2021
jarolrod commented at 8:58 PM on January 8, 2021: member

updated 1574879a7b06e570c9943de02e6efcc74817d44a -> 8be1a34f56ab8cb76b2d29b081c77c052bbad191

addressed @ajtowns comment. Thanks for the tip!

in src/validation.cpp:3623 in 8be1a34f56 outdated

3619 | @@ -3621,8 +3620,7 @@ bool BlockManager::AcceptBlockHeader(const CBlockHeader& block, BlockValidationS
3620 |              }
3621 |          }
3622 |      }
3623 | -    if (pindex == nullptr)
3624 | -        pindex = AddToBlockIndex(block);
3625 | +    CBlockIndex *pindex = AddToBlockIndex(block);

promag commented at 2:06 PM on January 10, 2021:

nit, CBlockIndex* pindex = ..., same in L3554.

promag commented at 2:07 PM on January 10, 2021: member

Code review ACK 8be1a34f56ab8cb76b2d29b081c77c052bbad191.
theStack approved
theStack commented at 2:52 PM on January 10, 2021: member

Code review ACK 8be1a34f56ab8cb76b2d29b081c77c052bbad191 🥃
DrahtBot commented at 11:45 AM on January 13, 2021: member



🕵️ @sipa has been requested to review this pull request as specified in the REVIEWERS file.

validation: remove redundant check on pindex

This removes a conditional that checks if pindex is equal to nullptr.
This check is redundant because the branch where pindex is set returns at an earlier time. Additionaly, The independence of the earlier and later pindex is made clearer.

c943282b5e

jarolrod force-pushed on Jan 13, 2021
theStack approved
theStack commented at 4:26 PM on January 13, 2021: member

re-ACK c943282b5e6312537f885c811d43120ce2f5b766
jarolrod commented at 4:28 PM on January 13, 2021: member

updated 8be1a34f56ab8cb76b2d29b081c77c052bbad191 -> c943282b5e6312537f885c811d43120ce2f5b766

Moved placement of pointer from name to type based on @promag suggestion
Zero-1729 commented at 6:05 AM on January 14, 2021: contributor

re-ACK c943282
ajtowns commented at 6:52 AM on January 14, 2021: member

ACK c943282b5e6312537f885c811d43120ce2f5b766 - code review only
decryp2kanon commented at 6:00 PM on January 19, 2021: contributor

Concept ACK