sipa / bitcoin

Bitcoin integration/staging tree
http://www.bitcoin.org
MIT License
88 stars 21 forks source link

Sipa headersfirst8 patches #4

Closed rebroad closed 9 years ago

rebroad commented 10 years ago

Various changes/fixes.

sipa commented 10 years ago

Thanks! Having these suggested changes as small individual patches is nice.

A few comments in general:

sipa commented 10 years ago

Regarding the fix logic for nSyncStarted: got it; you want to keep sending out initial header syncs as long as no node has responded with any header. That makes sense, but on a high-latency connection I expect that to result in multiple full header syncs being started from multiple nodes, wasting bandwidth.

sipa commented 10 years ago

I've cherry picked 3 of your logging improvements already into the bitcoin/bitcoin#4468 PR.

rebroad commented 10 years ago

Ok, yes, I now see that the pprev does make sense as otherwise we can't tell if they are ignoring the getheaders request of if they're up to date, or if they're just slow...

rebroad commented 10 years ago

I've added some commits to this that fix the stalled logic, hopefully.

rebroad commented 10 years ago

doh.. I removed the commit that fixed the getheaders from getting loads of headers, thinking you'd patched that, but now it's gone back to downloading tens of thousands of headers again, so I guess you didn't.

sipa commented 10 years ago

Can you rebase and test head? I've made a small change that could affect overzealous stall disconnection.

rebroad commented 9 years ago

@sipa I've made some changes. Please could you have a look at this? It seems to run pretty well so far, but there are still some improvements I'd like to add, such as a fix to make it stop requesting blocks from really slow nodes, and perhaps limiting how many concurrent downloads are allowed.