p2p: good will assumption for the DAO fork

[sorpaas]

After initialize the RLPx protocol, when connecting to a server peer running go-ethereum-classic:

1. Client and server both sends a Status message. The client tells the server running go-ethereum-classic that the best hash it has is the genesis block hash.
2. Server sends GetBlockHeaders of block 192000 checking whether it is on the same side of the DAO hard fork.
3. Client doesn't have any hash for 192000 right now, so it returns the reply BlockHeaders with an empty array.
4. Server then decides to drop the peer.

Is this the expected behavior? Since the client doesn't have any hash for the DAO hard fork, I think go-ethereum should assume it is on the same side, and only drop the peer after it has sync-ed after 192000.

In ETH's go-ethereum, on step (4), the server peer would assume that it is on the same side of the hard fork and allows the peer to continue the connection.

[whilei]

This is currently the expected behavior, but [see next comment] I agree with you that we should use "good-will" until 192000 is known.

[whilei]

I noticed that there is a check for the peer's current available height, where if the peer's height is below the required fork's height, it breaks the RequiredHash check:

		if _, height := p.Head(); height.Cmp(fork.Block) < 0 {
			break
		}

https://github.com/ethereumproject/go-ethereum/blob/master/eth/handler.go#L284

However, I wonder if we might improve it by running an additional check if our client's current header height is at (or near) the required fork's block.
Something like:

		if _, height := p.Head(); height.Cmp(fork.Block) < 0 {
			break
		}
		// Is our client's current header beneath fork-with-required-hash block number?
		// Note: adjust fork block number subtracting Downloader.MaxHeaderFetch constant, to give ourselves some wiggle room before
		// requesting the actual header for download
		adjustedForkNumber := new(big.Int).Sub(fork.Block, big.NewInt(downloader.MaxHeaderFetch))
		if pm.blockchain.CurrentHeader().Number.Cmp(adjustedForkNumber) < 0 {
			break
		}

[sorpaas]

@whilei Thanks for getting into this issue.

I think you also have a valid point and we might also consider to do that, but I think the problem might just lie in this line (

go-ethereum/eth/handler.go

Line 408 in 92cfa38

isForkCheck := len(headers) == 1

). Basically, go-ethereum assumes that any fork check would have the BlockHeaders array of length 1, but it can also be 0. p2p library just passes the block information to the consensus layer and doesn't care about whether it is actually 192000 or not.

[whilei]

Closing via #313