Document assumptions made in PeerLogicValidation::SendMessages(…) and rescanblockchain(…) #13548

1. 
   practicalswift commented at 4:46 pm on June 27, 2018: contributor

   Document assumptions made in PeerLogicValidation::SendMessages(...) and rescanblockchain(...) via assertions.

   The fact that pBestIndex != nullptr and stopBlock != nullptr in these contexts is not immediately obvious. Explicit is better than implicit.

2. Document assumption made about pBestIndex in PeerLogicValidation::SendMessages(...) e30ef3e521
3. Document assumption made about stopBlock in rescanblockchain(...) a7946bc261
4. fanquake added the label Refactoring on Jun 27, 2018
5. MarcoFalke closed this on Aug 17, 2018

   ---

6. MarcoFalke reopened this on Aug 17, 2018

   ---

7. 
   promag commented at 4:00 pm on August 17, 2018: member

   is not immediately obvious. Explicit is better than implicit.

   I don’t agree, it’s obvious because they are used right after the assertions.

8. 
   practicalswift commented at 4:36 pm on August 17, 2018: contributor

   @promag Yes, but the assertion documents that the dereference without a prior null check is intentional and not just an oversight. That helps static analyzers and human reviewers, don’t you agree? :-)

   FWIW I’ve seen numerous static analyzers independently flag these specific cases (and the cases covered by the other similar assertion PR:s). I’m not just adding asserts randomly for fun :-)

9. 
   DrahtBot commented at 4:15 pm on September 17, 2018: member
10. practicalswift closed this on Oct 18, 2018

    ---

11. practicalswift deleted the branch on Apr 10, 2021
12. DrahtBot locked this on Aug 16, 2022

Contributors
practicalswift promag DrahtBot

Labels
Refactoring