refactor: Pass block reference instead of pointer to PeerManagerImpl::BlockRequested #22221

1. 
   MarcoFalke commented at 8:00 am on June 11, 2021: member

   This allows to remove an assert and at the same time make it more obvious that the block is never nullptr.

   Also, add missing {} while touching the function.

2. refactor: Pass block reference instead of pointer to PeerManagerImpl::BlockRequested fa334b4054
3. 
   MarcoFalke commented at 8:01 am on June 11, 2021: member

   From #22141 (review)
4. 
   jnewbery commented at 8:45 am on June 11, 2021: member

   Code review ACK fa334b405411dc97fbed12b5e9103510eeb2c9f1
5. DrahtBot added the label P2P on Jun 11, 2021
6. DrahtBot added the label Refactoring on Jun 11, 2021
7. 
   mjdietzx commented at 2:17 pm on June 11, 2021: contributor

   crACK fa334b405411dc97fbed12b5e9103510eeb2c9f1
8. 
   practicalswift commented at 6:52 pm on June 11, 2021: contributor

   Concept ACK

   Candidates for similar treatment can be found using the git grep commands in #19062 :)

9. theStack approved
10. 
    theStack commented at 8:14 pm on June 11, 2021: member

    Code review ACK fa334b405411dc97fbed12b5e9103510eeb2c9f1
11. fanquake merged this on Jun 12, 2021
12. fanquake closed this on Jun 12, 2021

    ---

13. MarcoFalke deleted the branch on Jun 13, 2021
14. 
    Sjors commented at 4:34 pm on June 13, 2021: member

    This change may not be entirely without consequence (though I’m not sure if it really matters): #20295 (comment)
15. 
    MarcoFalke commented at 5:40 pm on June 13, 2021: member

    There was an assert(pindex);, so passing nullptr wasn’t possible before this commit either.
16. 
    Sjors commented at 7:33 pm on June 13, 2021: member

    That assert was introduced in 85e058b19145b5068f2f71a90c1182bf2a93c473 as part of #22141.
17. 
    jnewbery commented at 9:37 pm on June 13, 2021: member

    BlockRequested no longer takes a pointer. Which means we can’t call it for blocks for which we don’t have a header.

    (from #20295 (comment))

    I think this is fine. Since headers-first syncing, we’ll never attempt to download a block if we don’t already have its header.

    Adding the assert in #22141 didn’t add any new assumptions. All callers of BlockRequested were already dereferencing the CBlockIndex* that was bring passed in.

18. sidhujag referenced this in commit 887b3d10a0 on Jun 13, 2021
19. gwillen referenced this in commit 68aa09da02 on Jun 1, 2022
20. DrahtBot locked this on Aug 16, 2022

Contributors
MarcoFalke jnewbery mjdietzx practicalswift theStack Sjors

Labels
Refactoring P2P