p2p: good will assumption for the DAO fork #313
Conversation
Discussion at #309. After sending a request to check for a particular hash of a hard fork block, allow the peer to claim that it does not know about this block, and then assume the peer is valid, unless it shows it's not so.
|
It looks like the CI is showing for all tests -- seems strange. I'm not at my computer right now but will check it out as soon as I get back home this evening. |
solution: HeaderCheck check for new ErrHashEmpty error, where that error is ignored by eth/handler.go. This is presented as a possible alternative/+addition to @sorpaas changed in ethereumproject#313 Rel ethereumproject#309
solution: HeaderCheck check for new ErrHashEmpty error, where that error is ignored by eth/handler.go. This is presented as a possible alternative/+addition to @sorpaas changed in ethereumproject#313 Rel ethereumproject#309
len(headers) > 0 || !isForkCheck is equivalent to len(headers) > 0 || !(len(headers) == 1), and this is true for len == 0 (!(len(headers) == 1) is true), len == 1 (len(headers > 0) is true, and so on.
|
@whilei the test should pass now. Have a look. Turns out that this line |
|
I think your changes look good -- thanks for digging into this :) You may need to push an empty commit to trigger the CI tests to run again -- it looks like AppVeyor/branch hit one of the irksome downloader tests (see #245), and I can't restart the AppVeyor tests manually. |
|
Okay I'll merge this then. Let's get into #314 later. |
|
Below are just some comments on the current good-will implementation between ETH's and ETC's version: ETH's go-ethereum uses a similar technique for the good-will assumption -- to check whether BlockHeaders is of size zero. But it has some additional checks before it decides not to drop the peer connection -- It checks the peer's current best hash, and think the peer is misbehaving if the best hash's block number passes the fork check header number. I don't think this is a bad idea. However, I also don't think that this prevents any network attacks -- a malicious client can just return the correct hash for the fork check block, and return blocks in a different blockchain for all other blocks. So unless the client actually checks all blocks, it would not know whether the peer is misbehaving. In addition, some well-behaved client might be dropped for ETH's case. Consider a really light peer that deletes block headers as it validates along the way, and only keep the last 256 block headers (just enough to keep the EVM running). In this case, according to the protocol specification, that peer would return an empty array for the fork check BlockHeaders request, although its best hash block number has already passed the fork check block number. |
solution: HeaderCheck check for new ErrHashEmpty error, where that error is ignored by eth/handler.go. This is presented as a possible alternative/+addition to @sorpaas changed in ethereumproject#313 Rel ethereumproject#309
…not necessarily always true See ethereum/go-ethereum#14848 for an explanation of why the line `len(headers) > 0 || !filter` is added back. I still don't think we should check total difficulty of the peer with the fork header like ETH does. It might break light clients that only keep the last 256 blocks and does not prevent network attacks. See #313 for an explanation.
Discussion at #309.
After sending a request to check for a particular hash of a hard fork block, allow the peer to claim that it does not know about this block, and then assume the peer is valid, unless it shows it's not so.