Make sure to clean up mapBlockSource if we've already seen the block #11012

1. 
   theuni commented at 1:59 AM on August 9, 2017: member

   Otherwise we may leave them dangling.

   Credit TheBlueMatt.

2. Make sure to clean up mapBlockSource if we've already seen the block

   Credit TheBlueMatt

   3f8fa7f58b
3. fanquake added the label P2P on Aug 9, 2017
4. sipa added this to the milestone 0.15.0 on Aug 9, 2017
5. in src/net_processing.cpp:2140 in 3f8fa7f58b

   2133 | @@ -2134,9 +2134,12 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
   2134 |              }
   2135 |              bool fNewBlock = false;
   2136 |              ProcessNewBlock(chainparams, pblock, true, &fNewBlock);
   2137 | -            if (fNewBlock)
   2138 | +            if (fNewBlock) {
   2139 |                  pfrom->nLastBlockTime = GetTime();
   2140 | -
   2141 | +            } else {
   2142 | +                LOCK(cs_main);
   

   ---

   ---

   

   promag commented at 7:20 AM on August 9, 2017:

   Nit, move the lock up and remove the one below?

6. 
   promag commented at 7:24 AM on August 9, 2017: member

   Nit, reword Remove existing blocks from mapBlockSource?

7. 
   laanwj commented at 8:59 AM on August 10, 2017: member

   utACK https://github.com/bitcoin/bitcoin/pull/11012/commits/3f8fa7f58b68a8ed596c62d1edb85a3455a5a724

8. in src/net_processing.cpp:2400 in 3f8fa7f58b

   2396 | @@ -2390,8 +2397,12 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
   2397 |          }
   2398 |          bool fNewBlock = false;
   2399 |          ProcessNewBlock(chainparams, pblock, forceProcessing, &fNewBlock);
   2400 | -        if (fNewBlock)
   2401 | +        if (fNewBlock) {
   

   ---

   ---

   

   laanwj commented at 9:03 AM on August 10, 2017:

   Would be nice to factor this out to a function, as we're repeating exactly the same code three times.

   ---

   

   theuni commented at 4:04 PM on August 10, 2017:

   Would you like me to do that here? Either way works for me.

   ---

   

   laanwj commented at 5:42 PM on August 10, 2017:

   I don't care strongly, I'm fine with having this as quick fix only. But it feels a tad brittle.

   ---

   

   theuni commented at 8:31 PM on August 10, 2017:

   I believe that @TheBlueMatt had plans to clean this up further (or remove the need) in the future, so if it's all the same, I'd prefer to leave it as-is for 0.15.

9. 
   jnewbery commented at 9:40 PM on August 11, 2017: member

   utACK 3f8fa7f58b68a8ed596c62d1edb85a3455a5a724

10. 
    promag commented at 1:58 PM on August 12, 2017: member

    utACK 3f8fa7f. Should we open an issue to clean this in the future as @theuni suggests?

11. laanwj merged this on Aug 14, 2017
12. laanwj closed this on Aug 14, 2017

    ---

13. laanwj referenced this in commit 0e5cff6f2b on Aug 14, 2017
14. PastaPastaPasta referenced this in commit b3e33518a4 on Aug 6, 2019
15. PastaPastaPasta referenced this in commit aa8f2e7660 on Aug 6, 2019
16. PastaPastaPasta referenced this in commit 5fd7da81d0 on Aug 6, 2019
17. PastaPastaPasta referenced this in commit b4e54e997d on Aug 7, 2019
18. PastaPastaPasta referenced this in commit cd8334661c on Aug 8, 2019
19. PastaPastaPasta referenced this in commit ac80c90123 on Aug 12, 2019
20. barrystyle referenced this in commit 79da4b8887 on Jan 22, 2020
21. DrahtBot locked this on Sep 8, 2021

Contributors

theunipromaglaanwjjnewbery

Milestone
0.15.0