Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Implement past-session validator discovery APIs for Network Bridge #1461

Closed
rphmeier opened this issue Jul 23, 2020 · 13 comments · Fixed by #2009
Closed

Implement past-session validator discovery APIs for Network Bridge #1461

rphmeier opened this issue Jul 23, 2020 · 13 comments · Fixed by #2009
Assignees
Labels
S1-implement PR/Issue is in the implementation stage
Milestone

Comments

@rphmeier
Copy link
Contributor

rphmeier commented Jul 23, 2020

#1452 introduced some APIs for the network bridge to enable validator discovery. Those APIs are open for discussion, but the information provided by them should stay pretty much the same: a way to signal to connect to particular validator IDs and learn the PeerIds associated with that set so as to be aware of when we are connected to those validators.

@rphmeier rphmeier added the S1-implement PR/Issue is in the implementation stage label Jul 23, 2020
@rphmeier rphmeier added this to the White Flint milestone Jul 23, 2020
@rphmeier
Copy link
Contributor Author

rphmeier commented Jul 28, 2020

Related: paritytech/substrate#6595 (comment) but this is probably not a blocker.

Note that the network-bridge API will accept ValidatorIds, which we need to transform into the authority-discovery session keys somehow. A runtime API might be the best way to do that.

@tomaka
Copy link
Contributor

tomaka commented Aug 7, 2020

Note that the network-bridge API will accept ValidatorIds, which we need to transform into the authority-discovery session keys somehow. A runtime API might be the best way to do that.

Having no knowledge of the runtime nor about the various types of keys, I'd appreciate some help from someone here.

@rphmeier
Copy link
Contributor Author

rphmeier commented Aug 7, 2020

The ValidatorId->AuthorityDiscoveryKey we can handle on parachains-core side; what we need is a networking API accepting AuthorityDiscoveryKeys so that we can implement ConnectToValidators in the NetworkBridge.

@tomaka
Copy link
Contributor

tomaka commented Aug 7, 2020

The way things are laid out in my mind is:

  • The NetworkBridge would own some sort of AuthorityDiscoveryService (see this PR) in addition to the sc_network::NetworkService it already has.
  • Whenever a peer connects, the NetworkBridge can call a method on that AuthorityDiscoveryService to look up the AuthorityDiscoveryKey potentially associated to the PeerId we have just connected to.
  • At the start of each validator rotation N, the NetworkBridge would call a method from the AuthorityDiscoveryService to look up the addresses and PeerIds of the parachain-assigned validators of rotation N and N+1, then call NetworkService::set_priority_group("collation", that_list).

The authority-discovery module ensures, at any time, that it holds the PeerIds and addresses of all the validators of the current session/epoch and of the next one.
(at the moment, it only works with the current session, see paritytech/substrate#6772)

The idea is that in the future you would open only the collation notifications protocol with peers that have been looked up (as opposed to opening all notifications protocols with all peers right now), but this isn't possible yet without some refactoring of the peerset (paritytech/substrate#6087).

@rphmeier
Copy link
Contributor Author

rphmeier commented Aug 7, 2020

I'd rather not have the network bridge be directly responsible for dealing with rotations but rather just receive requests to connect to validators based on validator IDs.

This feature is not only for collator networking, but it is our first major use-case of it. We also need this to make PoV Distribution production-ready and for recovery of availability pieces for approval

@mxinden
Copy link
Contributor

mxinden commented Aug 7, 2020

Like Pierre above, I am in favor of the NetworkBridge to first query the authority discovery Service for the Multiaddrs of the authorities and then pass these on via the NetworkService to the peerset.

With the above we would separate concerns, as in the authority discovery only being concerned with discovering authorities.

Within NetworkBridge's event handling matching events from other subsystems:

ConnectToValidators(validators: Vec<AuthorityId>) => {
  let multiaddress = validators.into_iter()
    // `get_addresses_for_authority_id` accesses the cached addresses, thus the `await` does not take long.
    .map(|id| self.authority_discovery.get_addresses_for_authority_id(id).await)
    .filter(|addresses| addresses.is_some())
    .collect::<HashSet<_>>();

  self.network.set_priority_group(COLLATOR_PRIORITY_GROUP_NAME, multiaddresses)?;
}

Within NetworkBridge's event handling matching events from the underlying network:

sc_network::event::Event::NotificationStreamOpened { remote, .. } => {
  let authority_id: Option<AuthorityId> = self.authority_discovery.get_authority_id_for_peer_id(remote).await;
  return NetworkBridgeEvent::PeerConnected {
    authority_id,
    peer_id: remote,
  };
}

What do you think @rphmeier?

@rphmeier
Copy link
Contributor Author

rphmeier commented Aug 7, 2020

That proposal seems fine as long as there is no implied validator rotation-tracking logic in the network bridge subsystem.

The network bridge shouldn't need to track validator rotations (which the subsystems would have to do anyway) or the peer-sets that would be interested in any particular validator connection - this is also the responsibility of the subsystem.

The first snippet is fine, because the actual API takes a PeerSet parameter. We should note that across sessions, the second snippet may lose its validity.

We can have the network bridge maintain a ValidatorId -> AuthorityId mapping.

@mxinden
Copy link
Contributor

mxinden commented Aug 17, 2020

paritytech/substrate#6760 is merged and thus functionality suggested in #1461 (comment) can be used within NetworkBridge.

@rphmeier rphmeier assigned rphmeier and unassigned tomaka and mxinden Aug 17, 2020
@rphmeier
Copy link
Contributor Author

rphmeier commented Aug 18, 2020

We have the following networking subsystems which would heavily benefit from point-to-point networking between validators:

  • Collator Protocol
  • Availability Distribution
  • Availability Recovery
  • PoV Distribution

All of these, except Availability Recovery, are what I'd call "immediate" because they only have to deal with validators of the current session. For these, the priority-group approach should work, but maybe with some modification to avoid them clobbering each other.

However, Availability Recovery may need to perform recovery of data from prior sessions. If we allow disputes for up to a day after inclusion, then that would be 4 full sessions. At the session boundary, validators can both change their keys or even leave the set.

We will need some kind of discovery mechanism for associating old AuthorityIds with current Peer IDs. We will also want authorities to publish their addresses under not only their most recent AuthorityId but also their AuthorityIds for the last N sessions. There should be some glue in the authority discovery logic that we can fill out with blockchain-specific logic for determining our set of authority IDs. i.e. the authority discovery could be dependent on a Box<Fn() -> Vec<AuthorityId>> which yields all the authority IDs it should attempt to see if it has control over and sign messages with. Basically an abstraction around what is "current" so that Polkadot can fill it out with the authority sets of the last N sessions.

@mxinden
Copy link
Contributor

mxinden commented Aug 20, 2020

Implementation for address lookups of past sessions as described above is tracked in paritytech/substrate#6910.

@rphmeier
Copy link
Contributor Author

@ordian is this addressed by #1699?

@ordian
Copy link
Member

ordian commented Oct 26, 2020

@rphmeier right, but only for the validators in the current session

if we'll find a way to circumvent as you mentioned in #1699 (comment), we can close the issue

@rphmeier rphmeier changed the title Implement validator discovery APIs for Network Bridge Implement past-session validator discovery APIs for Network Bridge Nov 18, 2020
@rphmeier
Copy link
Contributor Author

Triage: session info makes this possible for other sessions. current session already implemented and functioning. Shifting milestone.

@rphmeier rphmeier removed this from the Grosvenor milestone Nov 18, 2020
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
S1-implement PR/Issue is in the implementation stage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants