Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ethereum 2.0 Networking Specification #1328

Merged
merged 10 commits into from Aug 5, 2019
Merged

Conversation

@arnetheduck
Copy link
Contributor

@arnetheduck arnetheduck commented Aug 1, 2019

This PR consolidates the work done by various teams and community towards a unified Ethereum 2.0 networking specification. In particular, this it consolidates the RPC-Interface.md and the libp2p-standardization.md documents into a single unified libp2p-based network specification.

Highlights of the PR include:

  • Concrete libp2p protocol selection for interop and mainnet, including transports, protocol negotiation and payload encoding
  • Broadcast for beacon blocks, attestations as well as a simplified request/response protocol for block recovery
  • Design rationale section summarising past discussions

This PR is the result of a collaborative effort between the Lighthouse, Nimbus and Protocol Labs teams.

It is a continuation of, and replaces #1281.

Copy link
Contributor

@Mikerah Mikerah left a comment

Some Questions/Comments:

  • Since we are already taking into account Noise, we should also take into account onion routing.
  • If we move to QUIC, would we still need two separate protocols for peer discovery (UDP-based Discv5) and peer communication (TCP)?

specs/networking/p2p-interface.md Show resolved Hide resolved
specs/networking/p2p-interface.md Outdated Show resolved Hide resolved
specs/networking/p2p-interface.md Outdated Show resolved Hide resolved
specs/networking/p2p-interface.md Show resolved Hide resolved
specs/networking/p2p-interface.md Outdated Show resolved Hide resolved
specs/networking/p2p-interface.md Show resolved Hide resolved
specs/networking/p2p-interface.md Outdated Show resolved Hide resolved
specs/networking/p2p-interface.md Show resolved Hide resolved
specs/networking/p2p-interface.md Show resolved Hide resolved
specs/networking/p2p-interface.md Show resolved Hide resolved
specs/networking/p2p-interface.md Show resolved Hide resolved
specs/networking/p2p-interface.md Outdated Show resolved Hide resolved
specs/networking/p2p-interface.md Outdated Show resolved Hide resolved
specs/networking/p2p-interface.md Outdated Show resolved Hide resolved
specs/networking/p2p-interface.md Outdated Show resolved Hide resolved
specs/networking/p2p-interface.md Show resolved Hide resolved
specs/networking/p2p-interface.md Show resolved Hide resolved
specs/networking/p2p-interface.md Outdated Show resolved Hide resolved
specs/networking/p2p-interface.md Outdated Show resolved Hide resolved
specs/networking/p2p-interface.md Show resolved Hide resolved
Requests are segregated by protocol ID to:

1. Leverage protocol routing in libp2p, such that the libp2p stack will route the incoming stream to the appropriate handler. This allows each the handler function for each request type to be self-contained. For an analogy, think about how you attach HTTP handlers to a REST API server.
2. Version requests independently. In a coarser-grained umbrella protocol, the entire protocol would have to be versioned even if just one field in a single message changed.
Copy link

@dryajov dryajov Aug 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

counterpoint, how to reliably handle interdependent endpoints (are there any?). If endpoint A also needs endpoint B upgraded, there is no mechanism to do that reliably with this approach.

Copy link
Contributor

@raulk raulk Aug 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plan is to group A and B in the discovery layer under protocol "families/suites/manifests" -- which represent aggregations of finer-grained protocols under one logical unit. that way we get the best of both worlds: upgraded peers can advertise/negotiate newer versions of a given operation, while continuing to serve older peers by supporting the former versions too.

while we could consider a mechanism to expose machine-readable metadata that specifies that A depends on B, I believe that's beyond scope. in practice, it should be enough to:

  1. specify that in the RFC/EIP that introduces A.
  2. potentially grouping A and B under a protocol family that can be atomically advertised.
  3. allowing peers to incur in a runtime error when they encounter a peer that supports A but not B.

Copy link
Contributor

@raulk raulk Aug 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cross-referenced from #1335.


## Req/Resp

### Why segregate requests into dedicated protocol IDs?
Copy link

@dryajov dryajov Aug 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory, I like the elegance of REST like endpoints, but in practice they might be as ambiguous as semver.

For example, how does the client know which protocol the other peer speaks? This isn't easy to do if every endpoint is versioned independently and the client relies on libp2p internals such as identify and multistream/multiselect to signal which versions the other end supports, which right now (AFAIR), isn't possible without probing each endpoint explicitly.

Copy link
Contributor

@raulk raulk Aug 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example, how does the client know which protocol the other peer speaks?

We have planned to address that in the discovery layer, but we're deferring defining the exact mechanism because it's irrelevant until mainnet. There's an idea here though: https://github.com/ethereum/eth2.0-specs/pull/1328/files#diff-9e07d955515655364c9a5f28c94f5627R236

Copy link
Contributor

@raulk raulk Aug 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened an issue to track this: #1335, and added a checklist item to the Open Points comment below: #1328 (comment).

specs/networking/p2p-interface.md Outdated Show resolved Hide resolved
specs/networking/p2p-interface.md Outdated Show resolved Hide resolved
specs/networking/p2p-interface.md Outdated Show resolved Hide resolved
specs/networking/p2p-interface.md Show resolved Hide resolved
specs/networking/p2p-interface.md Outdated Show resolved Hide resolved
specs/networking/p2p-interface.md Outdated Show resolved Hide resolved

#### Mainnet

- `ssz_snappy` - All objects are ssz-encoded and then compressed with snappy. Example: The beacon attestation topic string is: `/beacon_attestation/ssz_snappy` and the data field of a gossipsub message is an `Attestation` that has been ssz-encoded then compressed with snappy.
Copy link
Contributor

@hwwhww hwwhww Aug 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since topics are plain UTF-8 strings, does it make sense to have a map of code -> encoding, so that we can save some space in these topics? e.g., ssz is 1 and ssz_snappy is 2, so the topics are /beacon_attestation/1 and /beacon_attestation/2 .

Copy link
Contributor Author

@arnetheduck arnetheduck Aug 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we do this, we have a higher-value target in beacon_attestation - ie one could imagine having a fourcc here where the first three chars are topic and the last is encoding or something like this. that said, if we get stream compression, that should take care of it.

Also, we're sending SSZ - it's not the most bandwidth-friendly format out there already.. not sure this will make a practical difference.

all that said, if there's strong support, I'm fine with indices too.

@raulk
Copy link
Contributor

@raulk raulk commented Aug 2, 2019

Open points 💬

To keep things tidy, this comment will canonically track open points that (1) emerge from the discussion AND (2) are not immediately solvable.

We specify a follow-up action for each, in order to make them actionable.

  • ETH1 and ETH2 cross-compatible encoding of ENRs (RLP vs SSZ): ethereum/devp2p#107
  • Networking: protocol advertisement in discv5: #1335

Co-Authored-By: Hsiao-Wei Wang <hwwang156@gmail.com>
Co-Authored-By: Preston Van Loon <preston@prysmaticlabs.com>
specs/networking/p2p-interface.md Outdated Show resolved Hide resolved
specs/networking/p2p-interface.md Outdated Show resolved Hide resolved
Co-Authored-By: Hsiao-Wei Wang <hwwang156@gmail.com>
specs/networking/p2p-interface.md Outdated Show resolved Hide resolved
@arnetheduck
Copy link
Contributor Author

@arnetheduck arnetheduck commented Aug 2, 2019

  • Since we are already taking into account Noise, we should also take into account onion routing.

onion routing generally adds significant overhead and latency. this would need consideration before adding - numbers?

  • If we move to QUIC, would we still need two separate protocols for peer discovery (UDP-based Discv5) and peer communication (TCP)?

clients will likely implement alternative peer discovery mechanisms regardless as discv5 is explicitly limited to UDP - a no-go in many networks (and browsers).

that said, for core validating nodes in the network, it is reasonable to expect a stable networking setup with UDP access and there discv5 will do the job.

It's going to take a while for QUIC to be deployed and generally accepted / implemented - in this spec and for this reason, we explicitly chose to not stray into QUIC territory (the door remains open for clients to experiment with it)

* constants -> configurations
* constant name updates
* initial validation requirement for attestations
* allow aggregated attestations to be published
* move discv5 down a bit
* additional rationale
djrtwo
djrtwo approved these changes Aug 5, 2019
Copy link
Contributor

@djrtwo djrtwo left a comment

Great work everyone!
Merging this to serve as stable target for libp2p interop experiments going on. Fully expect this to iterate as we get our hands dirty but this is an excellent starting point.

@djrtwo djrtwo merged commit 11db5f1 into ethereum:dev Aug 5, 2019
6 checks passed
zah added a commit to status-im/nimbus-eth2 that referenced this issue Aug 7, 2019
@hwwhww hwwhww mentioned this pull request Aug 9, 2019
mratsim added a commit to status-im/nimbus-eth2 that referenced this issue Aug 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment