Remove NumBlocksOfPeers #4134

1. 
   laanwj commented at 8:17 AM on May 6, 2014: member

   Generally useless information. Only updates on connect time, not after that. Peers can easily lie and the median filter is not effective in preventing that.

   In the past it was used for progress display in the GUI but CheckPoints::guessVerificationProgress provides a better way that is now used. It was too easy to mislead it. Peers do lie about it in practice, see issue #4065.

   From the RPC, getpeerinfo gives the peer raw values of the initial height, which are more useful. I've also suggested in #4133 to show the peer information in a GUI table.

   (for the discussion on IRC that sparked this, see #4127)

2. 
   gmaxwell commented at 8:46 AM on May 6, 2014: contributor

   ACK. We've had periodic bouts of complaints on IRC (and github issues) from a mix of broken/trouble making nodes and also people just getting confused when the data is days stale due to long lived connections.

3. in src/main.cpp:None in 34e5c60bea outdated

   3477 | @@ -3486,7 +3478,6 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv)
   3478 |          AddTimeData(pfrom->addr, nTime);
   3479 |  
   3480 |          LOCK(cs_main);
   

   ---

   ---

   

   sipa commented at 8:48 AM on May 6, 2014:

   This lock can go.

   ---

   

   laanwj commented at 9:09 AM on May 6, 2014:

   Good catch

4. in src/qt/bitcoingui.cpp:None in 34e5c60bea outdated

    652 | -    }
    653 | -    else
    654 | -    {
    655 | -        tooltip = tr("Processed %1 blocks of transaction history.").arg(count);
    656 | -    }
    657 | +    tooltip = tr("Processed %1 blocks of transaction history.").arg(count);
   

   ---

   ---

   

   sipa commented at 8:49 AM on May 6, 2014:

   Print the progress % instead? Perhaps in a different PR.

   ---

   

   laanwj commented at 9:08 AM on May 6, 2014:

   Could be, though we already have a progress in % in the progress bar itself, so it wouldn't add much.

   ---

   

   sipa commented at 9:09 AM on May 6, 2014:

   Ok.

   ---

   

   laanwj commented at 9:14 AM on May 6, 2014:

   Huh it appears that it doesn't anymore. We used to have a percentage display at some point. Now we only show how far we're behind. Oh well.

5. 
   jgarzik commented at 8:49 AM on May 6, 2014: contributor

   ACK

6. Remove NumBlocksOfPeers

   Generally useless information. Only updates on connect time, not after
   that. Peers can easily lie and the median filter is not effective in
   preventing that.
   
   In the past it was used for progress display in the GUI but
   `CheckPoints::guessVerificationProgress` provides a better way that is now used.
   It was too easy to mislead it. Peers do lie about it in practice, see issue #4065.
   
   From the RPC, `getpeerinfo` gives the peer raw values, which are more
   useful.

   aa250f0453
7. 
   BitcoinPullTester commented at 9:41 AM on May 6, 2014: none

   Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/aa250f0453029afbf3c903899a3770c61e389468 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

8. 
   sipa commented at 10:46 AM on May 6, 2014: member

   Untested ACK

9. laanwj added this to the milestone 0.10.0 on May 6, 2014
10. 
    Diapolo commented at 11:02 AM on May 6, 2014: none

    I'm sure I'm going to miss this sometimes, but I have no strong pro or con feeling...

11. 
    laanwj commented at 11:23 AM on May 6, 2014: member

    @Diapolo Yes, having the information in some form would still be useful, hence my proposal #4133. It would show the reported height per peer, so that people can make their own conclusions and see what the source of the information is.

12. 
    Telepatheic commented at 3:42 PM on May 6, 2014: contributor

    Untested ACK although I'd like to see a better GetTotalBlocksEstimate() function so we can display an estimate for the total number of blocks.

13. laanwj merged this on May 9, 2014
14. laanwj closed this on May 9, 2014

    ---

15. laanwj referenced this in commit 82564e21e7 on May 9, 2014
16. DrahtBot locked this on Sep 8, 2021

Contributors

laanwjgmaxwellsipajgarzikBitcoinPullTesterDiapoloTelepatheic

Milestone
0.10.0