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

client/authority-discovery: Remove sentry node logic #7368

Merged
5 commits merged into from
Oct 26, 2020

Conversation

mxinden
Copy link
Contributor

@mxinden mxinden commented Oct 21, 2020

The notion of sentry nodes has been deprecated (see [1] for details).
This commit removes support for sentry nodes in the
client/authority-discovery module.

While removing Role::Sentry this commit also introduces
Role::Discover, allowing a node to discover addresses of authorities
without publishing ones own addresses. This will be needed in Polkadot
for collator nodes (//CC @ordian).

polkadot companion: paritytech/polkadot#1835

[1] #6845

Release note suggestion:

Remove support for sentry nodes in sc-authority-discovery. Validator nodes running the authority discovery module will publish their own addresses on the DHT, not the addresses of their sentry nodes as done before.

The notion of sentry nodes has been deprecated (see [1] for details).
This commit removes support for sentry nodes in the
`client/authority-discovery` module.

While removing `Role::Sentry` this commit also introduces
`Role::Discover`, allowing a node to discover addresses of authorities
without publishing ones own addresses. This will be needed in Polkadot
for collator nodes.

[1] paritytech#6845
@mxinden mxinden added A0-please_review Pull request needs code review. B3-apinoteworthy C1-low PR touches the given topic and has a low impact on builders. labels Oct 21, 2020
Copy link
Member

@ordian ordian left a comment

Choose a reason for hiding this comment

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

The existing change looks good.
But I don't know if any other code needs to be touched as well.

client/authority-discovery/src/service.rs Show resolved Hide resolved
@mxinden
Copy link
Contributor Author

mxinden commented Oct 26, 2020

bot merge

@ghost
Copy link

ghost commented Oct 26, 2020

Missing process info; check that the PR belongs to a project column.

Merge can be attempted if:

  • The PR has approval from two core-devs (or one if the PR is labelled insubstantial).
  • The PR has approval from a member of substrateteamleads.
  • The PR is attached to a project column and has approval from the project owner.

See https://github.com/paritytech/parity-processbot#faq

@mxinden
Copy link
Contributor Author

mxinden commented Oct 26, 2020

The PR has approval from a member of substrateteamleads.

@tomaka could you give this pull request a review as well?

@tomaka
Copy link
Contributor

tomaka commented Oct 26, 2020

bot merge

@ghost
Copy link

ghost commented Oct 26, 2020

Trying merge.

@ghost ghost merged commit 668390a into paritytech:master Oct 26, 2020
mxinden added a commit to mxinden/substrate that referenced this pull request Oct 27, 2020
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants