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.

Sign up for GitHub

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

Jump to bottom

Feat/pubsub validators #58

Merged
merged 4 commits into from
Dec 17, 2019
Merged

Feat/pubsub validators #58

merged 4 commits into from
Dec 17, 2019

Conversation

dryajov
Copy link
Contributor

@dryajov dryajov commented Dec 14, 2019

No description provided.

@dryajov dryajov requested a review from zah December 14, 2019 18:06
tersec
tersec reviewed Dec 16, 2019
View reviewed changes
libp2p/protocols/pubsub/pubsub.nim
# TODO: add timeout to validator
pending = pending.concat(p.validators[topic].mapIt(it(topic, message)))

await allFutures(pending) # await all futures
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems reasonable.

tersec
tersec approved these changes Dec 16, 2019
View reviewed changes
Copy link
Contributor

@tersec tersec left a comment

Choose a reason for hiding this comment

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

Seems reasonable. As with any code, there are possible improvements, et cetera, but it unblocks continued attestation aggregation extended validation.

zah
zah reviewed Dec 16, 2019
View reviewed changes
libp2p/protocols/pubsub/pubsub.nim Outdated Show resolved Hide resolved
libp2p/protocols/pubsub/pubsub.nim Outdated Show resolved Hide resolved
@dryajov dryajov merged commit 68cc576 into master Dec 17, 2019
@dryajov dryajov deleted the feat/pubsub-validators branch December 17, 2019 05:36
@tersec tersec mentioned this pull request Dec 18, 2019
Closed
@dryajov dryajov mentioned this pull request Jan 23, 2020
Closed
@dryajov dryajov mentioned this pull request Feb 5, 2020
Closed
@etan-status etan-status mentioned this pull request May 22, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zah zah zah left review comments

@tersec tersec tersec approved these changes

@arnetheduck arnetheduck Awaiting requested review from arnetheduck

@cheatfate cheatfate Awaiting requested review from cheatfate

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dryajov @zah @tersec