eth: implement eth66

[holiman]

This PR implements support for eth66(https://eips.ethereum.org/EIPS/eip-2481) , in the sense that we advertise it and handle it on the protocol level. This PR does not actually make use of it in any way, internally.

The way it's implemented, is that instead of a giant switch, the eth handler splits it up into a map of handlers.

var eth66 = map[uint64]msgHandler{
	// eth64 announcement messages (no id)
	NewBlockHashesMsg: handleNewBlockhashes,
	NewBlockMsg:       handleNewBlock,
	TransactionsMsg:   handleTransactions,
	// eth65 announcement messages (no id)
	NewPooledTransactionHashesMsg: handleNewPooledTransactionHashes,
	// eth66 messages with request-id
	GetBlockHeadersMsg:       handleGetBlockHeaders66,

So, e.g the eth65 handler map has BlockHeadersMsg: handleBlockHeaders,, and eth66 maps the same message type to handleGetBlockHeaders66 instead.

The two methods are very simliar, and the actual query-resolving has been moved to a function they both share

func handleGetBlockHeaders(backend Backend, msg Decoder, peer *Peer) error {
	// Decode the complex header query
	var query GetBlockHeadersPacket
	if err := msg.Decode(&query); err != nil {
		return fmt.Errorf("%w: message %v: %v", errDecode, msg, err)
	}
	response := answerGetBlockHeadersQuery(backend, query, peer)
	return peer.SendBlockHeaders(response)
}

// handleGetBlockHeaders66 is the ETH-66 version of handleGetBlockHeaders
func handleGetBlockHeaders66(backend Backend, msg Decoder, peer *Peer) error {
	// Decode the complex header query
	var query GetBlockHeadersPacket66
	if err := msg.Decode(&query); err != nil {
		return fmt.Errorf("%w: message %v: %v", errDecode, msg, err)
	}
	response := answerGetBlockHeadersQuery(backend, query.GetBlockHeadersPacket, peer)
	return peer.ReplyBlockHeaders(query.RequestId, response)
}

The big difference, aside from parsing as GetBlockHeadersPacket66 instead of GetBlockHeadersPacket, is that the eth66 handler calls peer.ReplyBlockHeaders instead of peer.SendBlockHeaders(response).

The ReplyXXX( id,...) wraps the message with the given request id.
All new messages are wrapped from the eth65/64 message types, e.g.

type GetBlockHeadersPacket66 struct {
	RequestId uint64
	GetBlockHeadersPacket
}

In this implementation, the Backend is not aware of request ids. For exampe, when dealing with the response containing receipts -- the eth65 handler decodes it, and shoves to the backend:

func handleReceipts(backend Backend, msg Decoder, peer *Peer) error {
	// A batch of receipts arrived to one of our previous requests
	res := new(ReceiptsPacket)
	if err := msg.Decode(res); err != nil {
		return fmt.Errorf("%w: message %v: %v", errDecode, msg, err)
	}
	return backend.Handle(peer, res)
}

And the new handler does the same thing -- decodes into the wrapped type, and then passes the 'original' unwrapped message to the backend:

func handleReceipts66(backend Backend, msg Decoder, peer *Peer) error {
	// A batch of receipts arrived to one of our previous requests
	res := new(ReceiptsPacket66)
	if err := msg.Decode(res); err != nil {
		return fmt.Errorf("%w: message %v: %v", errDecode, msg, err)
	}
	return backend.Handle(peer, &res.ReceiptsPacket)
}

When we deal with outgoing requests, however, we need to handle the distinction in the Peer itself. Example when we request bodies:

func (p *Peer) RequestBodies(hashes []common.Hash) error {
	p.Log().Debug("Fetching batch of block bodies", "count", len(hashes))
	if p.Version() >= ETH66 {
		return p2p.Send(p.rw, GetBlockBodiesMsg, &GetBlockBodiesPacket66{rand.Uint64(), hashes})
	}
	return p2p.Send(p.rw, GetBlockBodiesMsg, GetBlockBodiesPacket(hashes))
}

[holiman]

Alternatively, if we handle it 65/66 separation in the layer above, it would look something like this:

// SendBlockHeaders sends a batch of block headers to the remote peer.
func (p *Peer) SendBlockHeaders(headers []*types.Header) error {
	return p2p.Send(p.rw, BlockHeadersMsg, BlockHeadersPacket(headers))
}

// SendBlockHeaders66 sends a batch of block headers to the remote peer.
func (p *Peer) SendBlockHeaders66(requestId uint64, headers []*types.Header) error {
	return p2p.Send(p.rw, BlockHeadersMsg, requestId,
		BlockHeadersPacket66{id, BlockHeadersPacket(headers)})
}

That's maybe the most straight-forward implementation of them all, but it's not very subtle

[holiman]

I think this way of doing it is the 'cleanest': d6b1989

[holiman]

This PR is more or less 'done'. As in, as far as I know, everything is implemented (as opposed to "this is working perfectly and is safe to merge"). As such, it would be good with a review, to check if the general approach is ok, or if we should approach it some other way.

I'm going to try syncing some local instances over eth66 with eachother, and see how it goes. @carver you don't happen to have any eth66 implementations ready, do you?

[holiman]

One geth goerli node is now serving another locally on my laptop, speaking eth66. The basic protocol messages appear to work, but the transaction pingpong is not yet tested.

[holiman]

Tested now with two peers which are both in sync. One connected to the 'live' network, the other with --netrestrict so it only speaks to the first one (eth66). Block announcement works, blocks are imported fine. Neither of the nodes showed anything in their respective txpools, however. Not sure why, perhaps goerli txs get included pretty quickly, and nothing much is sloshing around in the pools? Or, I borked tx the tx handling.

[rjl493456442]

This PR looks really clean and nice!

[rjl493456442]

It's a bit confusing to me. The eth65 only adds the transaction retrievals, it did nothing at the syncing level. Shouldn't we also test the lightSync on eth64?

[holiman]

Yeah, probably. I didn't add any new-old tests, just new-new tests

[holiman]

With the latest fix, I am now able to

• Have one goerli-node A connected to "the world"
• Have a second goerli-node B connected to A only, speaking eth66, and staying up to date with the chain tip and getting stuff into the transaction pool. (Though blocks are imported a bit haphazardly, but that's because it only get the announcements every once in a while, right?)

So now it appears to function correctly.

[holiman]

Note for reviewer: This explicit error has been removed, and now this case would just fall through and error as an unhandled message

[karalabe]

(Though blocks are imported a bit haphazardly, but that's because it only get the announcements every once in a while, right?

Erm, no, it should be stable independent if it's broadcast or announced blocks. All peers receive either one or the other, but they do receive enough information to never hiccup.

[karalabe]

Either move this up right under BlockBodiesPacket to keep the sequence. Or alternatively pls flip this with the RLP version so that we have the same ordering as the legacy versions,

[karalabe]

Nit throughout, it's not ETH-66, it's eth/66. Please fix it.

[holiman]

Erm, no, it should be stable independent if it's broadcast or announced blocks. All peers receive either one or the other, but they do receive enough information to never hiccup.

Indeed, while adding testcases, I found that the GetBlockHeadersPacket66 was erroneously implemented, so that should be fixed now. I'll try out the relaying again.

[karalabe]

Please expand the struct fields throughout. It's bad style to use unnamed field initialization, it's even forbidden on AppEngine for example.

[karalabe]

Please use *GetBlockBodiesPacket as a paramter, there's no need to copy the struct. Same for the return.

[karalabe]

Ah apologies, the output is a slice, so there's no copy.

[karalabe]

Hmm, the input is a slice too?

[holiman]

Yup, []common.Hash in, []rlp.RawValue out

[karalabe]

Please use *GetNodeDataPacket as input, *NodeDataPacket as output

[karalabe]

Ah apologies, the output is a slice, so there's no copy.

[karalabe]

Hmm, the input is a slice too?

[holiman]

Fixed, rebased

[karalabe]

LGTM!