Skip to content

[network] topic validator for staked channels#1225

Merged
synzhu merged 22 commits intomasterfrom
smnzhu/topic-validators
Sep 7, 2021
Merged

[network] topic validator for staked channels#1225
synzhu merged 22 commits intomasterfrom
smnzhu/topic-validators

Conversation

@synzhu
Copy link
Copy Markdown
Contributor

@synzhu synzhu commented Aug 28, 2021

@synzhu synzhu changed the base branch from master to smnzhu/staked-sync-provider August 28, 2021 23:34
Base automatically changed from smnzhu/staked-sync-provider to smnzhu/identity-provider August 30, 2021 16:06
@huitseeker huitseeker force-pushed the smnzhu/identity-provider branch from cf0cf32 to d937be0 Compare August 30, 2021 17:56
Copy link
Copy Markdown
Contributor

@huitseeker huitseeker left a comment

Choose a reason for hiding this comment

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

Thanks a bunch! this looks good apart from a couple of places

@synzhu synzhu requested a review from thep2p as a code owner August 31, 2021 19:41
@huitseeker huitseeker force-pushed the smnzhu/identity-provider branch 5 times, most recently from 9d8423a to 4ce2800 Compare September 2, 2021 18:07
Base automatically changed from smnzhu/identity-provider to master September 2, 2021 18:53
@huitseeker huitseeker force-pushed the smnzhu/topic-validators branch 3 times, most recently from 331c712 to 6b0fbcd Compare September 3, 2021 00:22
Copy link
Copy Markdown
Contributor

@huitseeker huitseeker left a comment

Choose a reason for hiding this comment

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

A couple of places where this is still breaking, putting it back in your queue for now (but we're not far, i think).

Comment on lines +535 to +540
err := n.pubSub.UnregisterTopicValidator(topic.String())
if err != nil {
return fmt.Errorf("could not unregister topic validator for (%s): %w", topic, err)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

as you'll see from CI, this fails if there's no validator on the topic (e.g. the subscribe call didn't keep any)

@huitseeker huitseeker force-pushed the smnzhu/topic-validators branch from 53dc28d to ff800e9 Compare September 3, 2021 00:56
@huitseeker huitseeker force-pushed the smnzhu/topic-validators branch from d16b62a to 596b11a Compare September 7, 2021 17:52
@synzhu
Copy link
Copy Markdown
Contributor Author

synzhu commented Sep 7, 2021

bors merge

bors bot added a commit that referenced this pull request Sep 7, 2021
1225: [network] topic validator for staked channels r=smnzhu a=smnzhu

closes https://github.com/dapperlabs/flow-go/issues/5802

### TODO:
- [x] write tests

Co-authored-by: Simon Zhu <simon.zsiyan@gmail.com>
Co-authored-by: François Garillot <francois.garillot@dapperlabs.com>
Co-authored-by: Vishal <1117327+vishalchangrani@users.noreply.github.com>
@huitseeker huitseeker mentioned this pull request Sep 7, 2021
@synzhu synzhu merged commit 4f4e980 into master Sep 7, 2021
@synzhu synzhu deleted the smnzhu/topic-validators branch September 7, 2021 18:10
@bors
Copy link
Copy Markdown
Contributor

bors bot commented Sep 7, 2021

This PR was included in a batch that successfully built, but then failed to merge into master (it was a non-fast-forward update). It will be automatically retried.

@Kay-Zee
Copy link
Copy Markdown
Member

Kay-Zee commented Sep 7, 2021

bors cancel

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Sep 7, 2021

Canceled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants