Network specification update

[AgeManning]

Provides updates to the networking specification.

Specifically:

• Fixes the REQ_RESP_MAX_SIZE
• Renames the previously similar BeaconBlocks and RecentBeaconBlocks to BeaconBlocksByRange and BeaconBlocksByRoot respectively
• Removes the 1:1 mapping in BeaconBlocksByRoot, a responder may now return less blocks than requested
• The responder to a BeaconBlocksByRange request should also limit their response by the REQ_RESP_MAX_SIZE or SSZ_MAX_LIST_SIZE
• Adds the notion of a response_chunk. Responses that consist of a single SSZ-list (BeaconBlocksByRange, BeaconBlocksByRoot) are now sent back over the stream in individual response_chunk.
• Adds clarification around the SSZ-encoding of the request/response types
• Adds clarification to the RPC requests

This extends on the improvements in #1390

[djrtwo]

Cleaned up the language a bit and clarified a some things.
One minor question

[zah]

If the hello message is now valid at any time, shall we rename it to status? A lot of people will associate the term "hello" with something that is appropriate only at the start of the session.

[prestonvanloon]

Instead of

(
  blocks: []BeaconBlock
)

we could define it as

(
  []BeaconBlock
)

What do you think? Having a field name might imply that this is a container. We missed the note originally when implementing the networking spec and sent block response containers.

[AgeManning]

I have updated this PR to accommodate suggestions.
Specifically, we have changed the REQ_RESP_MAX_SIZE to apply to the encoded payload of the request/responses and the RESP_TIMEOUT now applies per response chunk.

[raulk]

Nice work on the chunking strategy! How about leveraging this to mandate/recommend early/streaming validation, with the ability to Reset a stream, or decrement peer reputation on failure? This would create a more secure network.

[raulk]

This can incur in excessive fragmentation/overhead/underutilisation if list items are small, as you're effectively defining a 1:1 mapping between chunk and list element.

I'd recommend to make the boundaries of chunks full list elements, allowing a chunk to contain multiple complete list elements, and disallowing partial elements or bleeding over between chunks.

It may be worth formally introducing the term chunkable, so you can label message fields that satisfy this property in the schemas below.

[zah]

Please note that adding chunks does not increase the size of the overall payload, because we are replacing the leading 4-byte offsets that previously appeared in the SSZ lists with response codes and varint lengths which will often amount to just 2 bytes per chunk (in the small items case).

[AgeManning]

I'd recommend to make the boundaries of chunks full list elements, allowing a chunk to contain multiple complete list elements, and disallowing partial elements or bleeding over between chunks.

This is the case. "Responses that consist of a single SSZ-list (such as BlocksByRange and BlocksByRoot) send each list item as a response_chunk"
Perhaps this is not clear enough?

A chunk represents a single encoded item

[raulk]

A chunk represents a single encoded item

Yeah, in practice this can incur in excessive chunking. If you have 3 list items available, and they all fit within one chunk, you can coalesce them into the same chunk. This is especially more efficient if you can fit multiple chunks within a single MTU. I'm not so worried about the byte overhead (although... death by a thousand cuts is a thing), but rather about the fragmentation, and making a protocol with a chunking strategy that's future proof.

[AgeManning]

Not entirely sure I follow.
Different requests have different discrete items. Eg a status message has a single ssz-encoded response (which is a container). It's chunk would be the size of that ssz-container. (It likely won't be a multiple of an MTU).

Responses that have lists, we now split into single items and send them as a chunk wrapped in a "error response". If we grouped multiple items under a single error response, we would require the encoding to be an ssz-list (like we had earlier) and the receiver would need to know which bytes are lists and which are single elements.

An SSZ list is encoded like | offset| offset| offset ..... | item | item | item |. Previously we sent this whole thing as a single error response across one stream, it now more like:
| error_response | item | error_response | item | ...
So we have replaced the offsets with error_responses and allow the receiver to take individual elements rather than wait for the entire stream to end.

Let me know if I've misunderstood your point

[raulk]

Beware of a starvation DoS attack where attackers could deliberately trickle their chunks slowly so as to keep that socket/file descriptor busy as long as possible.

[AgeManning]

Each chunk is bounded by RESP_TIMEOUT. So the slowest an attacker can be is 1 chunk per RESP_TIMEOUT. If the chunk is malicious or useless, the application will likely drop the peer

[raulk]

Yes, I'm worried about the case where an attacker has good data but they explicitly starve you off it by trickling it slowly. RESP_TIMEOUT is 10s; imagine you have 1000 chunks, of 1kb each (1mb total). This policy makes it possible to craft an attack where the peer sends you one valid chunk every 9 seconds, overall taking 150 minutes (2.5 hours) to dispatch 1mb of data.

This can be counteracted by a global RPC timeout.

[AgeManning]

Good point. Will discuss :)

[arnetheduck]

I'm not sure this is something the spec necessarily needs to discuss - it seems it could be handled with a peer quality algorithm instead - if the trickling peer is the only one you have, it's also the one you want.

[prestonvanloon]

LGTM. I would like a recommendation for the scenario when a dialing peer does not send a Status immediately.

[arnetheduck]

LGTM. I would like a recommendation for the scenario when a dialing peer does not send a Status immediately.

timeout/disconnect - no point spending resources if they're not following the protocol

[djrtwo]

I'd like to get this merged soon and release to v0.8.4

[ChihChengLiang]

d) More than the maximum number of requested chunks are read.

@AgeManning Should this number be specified?

[AgeManning]

The two types that can request chunks are BeaconBlocksByRange and BeaconBlocksByRoot. Each specify in the request the maximum number of blocks/chunks that should be returned.

For BeaconBlocksByRange this is the count parameter, for BeaconBlocksByRoot this is the number of hashes requested.

Let me know if this doesn't answer your question :)

[ChihChengLiang]

That makes perfect sense. Thank you for the clarification ❤️.