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

Connect to different validators on different leaves #1966

Merged
11 commits merged into from
Nov 18, 2020
Merged

Connect to different validators on different leaves #1966

11 commits merged into from
Nov 18, 2020

Conversation

montekki
Copy link
Contributor

This is needed to connect to different validators under different relay leafs.

@montekki montekki added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Nov 17, 2020
@montekki montekki mentioned this pull request Nov 17, 2020
Merged
coriolinus
coriolinus approved these changes Nov 17, 2020
View reviewed changes
Copy link
Contributor

@coriolinus coriolinus left a comment

Choose a reason for hiding this comment

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

Seems fine. Without a usage example it's hard to comment on the API offered, but at first look this seems reasonable.

node/subsystem-util/src/validator_discovery.rs Outdated Show resolved Hide resolved
@ordian
Copy link
Member

ordian commented Nov 17, 2020

Could you adapt this PR to ConnectionRequests API if you think it's more handy?
#1955 (comment)

ordian
ordian reviewed Nov 17, 2020
View reviewed changes
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.

Some open questions and also would be nice to add tests.

node/subsystem-util/src/validator_discovery.rs Show resolved Hide resolved
node/subsystem-util/src/validator_discovery.rs Outdated Show resolved Hide resolved
node/subsystem-util/src/validator_discovery.rs Outdated Show resolved Hide resolved
bkchr
bkchr reviewed Nov 17, 2020
View reviewed changes
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Tests?

node/subsystem-util/src/validator_discovery.rs Outdated Show resolved Hide resolved
node/subsystem-util/src/validator_discovery.rs Outdated Show resolved Hide resolved
node/subsystem-util/src/validator_discovery.rs Outdated Show resolved Hide resolved
@montekki montekki requested a review from ordian November 18, 2020 09:20
ordian
ordian approved these changes Nov 18, 2020
View reviewed changes
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.

LGTM

ordian
ordian reviewed Nov 18, 2020
View reviewed changes
node/subsystem-util/src/validator_discovery.rs
impl ConnectionRequests {
/// Insert a new connection request.
///
/// If a `ConnectionRequest` under a given `relay_parent` already exists it will
Copy link
Member

Choose a reason for hiding this comment

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

would be nice to test this as well

@montekki
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Nov 18, 2020

Trying merge.

@ghost ghost merged commit e2b1016 into paritytech:master Nov 18, 2020
@montekki montekki deleted the fs-multiple-parent-validator-discovery branch November 18, 2020 10:39
ordian added a commit that referenced this pull request Nov 19, 2020
@ordian
Merge branch 'master' into ao-session-info
c9f4e7c 
* master:
  Bump smallvec from 1.4.2 to 1.5.0 (#1971)
  Companion PR for substrate#7549 (#1967)
  Bump parking_lot from 0.11.0 to 0.11.1 (#1970)
  Connect to different validators on different leaves (#1966)
  patches (#1965)
  Bump dlmalloc from 0.1.4 to 0.2.1 (#1962)
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bkchr bkchr bkchr left review comments

@ordian ordian ordian approved these changes

@coriolinus coriolinus coriolinus approved these changes

Assignees
No one assigned
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@montekki @ordian @bkchr @coriolinus