Use pnode->nLastRecv as sync score directly #4261

pull 4tar wants to merge 1 commits into bitcoin:master from 4tar:syncscore changing 1 files +6 −6

4tar commented at 3:52 PM on May 30, 2014: contributor

NodeSyncScore() should find the node which we recv data most recently, so put a negative sign to pnode->nLastRecv is indeed wrong.

Also change the return value type to int64_t.

Signed-off-by: Huang Le 4tarhl@gmail.com

Use pnode->nLastRecv as sync score directly

NodeSyncScore() should find the node which we recv data most recently, so put a negative sign to pnode->nLastRecv is indeed wrong.

Also change the return value type to int64_t.

Signed-off-by: Huang Le <4tarhl@gmail.com>

09a54a65c0

sipa commented at 3:58 PM on May 30, 2014: member

ACK on the sign change.

Meh on the type change.
4tar commented at 4:11 PM on May 30, 2014: contributor

@sipa Is there any reason (maybe historical?) for the return type to be double but not int64_t as which is the type of CNode::nLastRecv? Do we need keep it still be double? If so I'll go to re-submit a PR.
sipa commented at 4:13 PM on May 30, 2014: member

The reason is (was) that at some point a more complex formula could be used.
4tar commented at 4:22 PM on May 30, 2014: contributor

@sipa Yep, that is what I guess. But since this is a internal utility function but a general interface, anytime when we adjust the scoring algorithm we shall always change the function, so it may not necessary to keep the return value to be still in double, which creates extra, though negligible, type casting cost here...

Do you think I should re-submit with keeping the original double type?
sipa commented at 4:27 PM on May 30, 2014: member

No worries, I have no problem with it. Just explaining the history.
BitcoinPullTester commented at 4:30 PM on May 30, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/09a54a65c0d05fe93c4a31603eca8a9a76ff6526 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.
jgarzik commented at 11:10 PM on May 30, 2014: contributor

ACK. I like patches that clean up type usage.
sipa commented at 10:37 AM on June 1, 2014: member

ACK. I like patches that fix broken peer selection.
laanwj added this to the milestone 0.9.2 on Jun 2, 2014
laanwj merged this on Jun 2, 2014
laanwj closed this on Jun 2, 2014
laanwj referenced this in commit 42d87749eb on Jun 2, 2014
4tar deleted the branch on Jun 5, 2014
MarcoFalke locked this on Sep 8, 2021

Contributors