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

Mempool Update #705

Merged
merged 32 commits into from
Sep 28, 2020
Merged

Mempool Update #705

merged 32 commits into from
Sep 28, 2020

Conversation

ec2
Copy link
Member

@ec2 ec2 commented Sep 17, 2020 •
edited
Loading

Summary of changes
Changes introduced in this pull request:

  • Updates existing Mempool methods

  • Remove unused MpoolProvider

  • New Mpool config

  • TODO for next couple PRs: publish messages from mpool over gossipsub, message selection algorithm

Reference issue to close (if applicable)

Closes

Other information and links

austinabell
austinabell reviewed Sep 17, 2020
View reviewed changes
Copy link
Contributor

@austinabell austinabell left a comment

Choose a reason for hiding this comment

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

quick scan, can look more in detail later

blockchain/message_pool/src/config.rs Show resolved Hide resolved
blockchain/message_pool/src/lib.rs Outdated Show resolved Hide resolved
blockchain/message_pool/src/msgpool.rs Outdated Show resolved Hide resolved
blockchain/message_pool/src/msgpool.rs Outdated Show resolved Hide resolved
blockchain/message_pool/src/msgpool.rs Outdated Show resolved Hide resolved
blockchain/message_pool/Cargo.toml Outdated Show resolved Hide resolved
blockchain/message_pool/src/msgpool.rs Outdated Show resolved Hide resolved
blockchain/message_pool/src/msgpool.rs Outdated Show resolved Hide resolved
@ec2 ec2 requested a review from austinabell September 25, 2020 07:07
austinabell
austinabell approved these changes Sep 28, 2020
View reviewed changes
Copy link
Contributor

@austinabell austinabell left a comment

Choose a reason for hiding this comment

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

LGTM, a few more docs would be nice, but should be a good starting point to work off of

blockchain/message_pool/src/msgpool.rs Show resolved Hide resolved
blockchain/message_pool/src/msgpool.rs Show resolved Hide resolved
blockchain/message_pool/src/msgpool.rs Outdated Show resolved Hide resolved
StaticallyTypedAnxiety
StaticallyTypedAnxiety approved these changes Sep 28, 2020
View reviewed changes
Copy link
Contributor

@StaticallyTypedAnxiety StaticallyTypedAnxiety left a comment

Choose a reason for hiding this comment

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

Good to merge on my side I just had a few comments but very unnecessary

blockchain/message_pool/src/msgpool.rs Show resolved Hide resolved
RajarupanSampanthan
RajarupanSampanthan approved these changes Sep 28, 2020
View reviewed changes
Copy link
Contributor

@RajarupanSampanthan RajarupanSampanthan left a comment

Choose a reason for hiding this comment

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

Lgtm

@ec2 ec2 merged commit b073565 into main Sep 28, 2020
@ec2 ec2 deleted the ec2/mpool-update branch September 28, 2020 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StaticallyTypedAnxiety StaticallyTypedAnxiety StaticallyTypedAnxiety approved these changes

@austinabell austinabell austinabell approved these changes

@RajarupanSampanthan RajarupanSampanthan RajarupanSampanthan approved these changes

@ansermino ansermino Awaiting requested review from ansermino

@dutterbutter dutterbutter Awaiting requested review from dutterbutter

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

Successfully merging this pull request may close these issues.
4 participants
@ec2 @RajarupanSampanthan @austinabell @StaticallyTypedAnxiety