Skip to content

merge bitcoin#16787, #17474, #17687, #18165, #18877, #18960, #19010, #19044, #19070: block filters #4355

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
Merged
UdjinM6 merged 9 commits into from
Sep 21, 2021
Merged

merge bitcoin#16787, #17474, #17687, #18165, #18877, #18960, #19010, #19044, #19070: block filters #4355

UdjinM6 merged 9 commits into from
Sep 21, 2021

Conversation

@kwvg
Copy link
Collaborator

@kwvg kwvg commented Aug 22, 2021

No description provided.

@kwvg kwvg changed the title bitcoin#16787, #17474, #17687, #18165, #18877, #18960, #19010, #19044, #19070: block filters merge bitcoin#16787, #17474, #17687, #18165, #18877, #18960, #19010, #19044, #19070: block filters Aug 22, 2021
@PastaPastaPasta
Copy link
Member

PastaPastaPasta commented Aug 22, 2021

linter + both tests are unhappy

@kwvg
Copy link
Collaborator Author

kwvg commented Aug 22, 2021

It's a work in progress

@PastaPastaPasta PastaPastaPasta marked this pull request as draft August 22, 2021 20:17
@PastaPastaPasta
Copy link
Member

PastaPastaPasta commented Aug 22, 2021

Marked as draft

@github-actions
Copy link

github-actions bot commented Sep 14, 2021

This pull request has conflicts, please rebase.

@kwvg kwvg marked this pull request as ready for review September 16, 2021 15:44
@PastaPastaPasta
Copy link
Member

PastaPastaPasta commented Sep 17, 2021

Please rename all commits to begin with "Merge"

PastaPastaPasta
PastaPastaPasta requested changes Sep 17, 2021
View reviewed changes
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

LGTM, besides one thing and a comment

src/net_processing.h Outdated
Copy link
Member

@PastaPastaPasta PastaPastaPasta Sep 17, 2021

Choose a reason for hiding this comment

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

We should consider what all of these defaults should be, like should normal nodes be serving bloom filters by default for example?

UdjinM6
UdjinM6 requested changes Sep 18, 2021
View reviewed changes
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

Few small issues

PastaPastaPasta
PastaPastaPasta approved these changes Sep 19, 2021
View reviewed changes
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK for merging via merge commit

UdjinM6
UdjinM6 approved these changes Sep 21, 2021
View reviewed changes
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

utACK

@UdjinM6 UdjinM6 added this to the 18 milestone Sep 21, 2021
@UdjinM6 UdjinM6 merged commit 4ffd42d into dashpay:develop Sep 21, 2021
@UdjinM6 UdjinM6 added the RPC Some notable changes to RPC params/behaviour/descriptions label Sep 28, 2021
@kwvg kwvg deleted the bfilters branch July 18, 2023 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@UdjinM6 UdjinM6 UdjinM6 approved these changes

@PastaPastaPasta PastaPastaPasta PastaPastaPasta approved these changes

Assignees

No one assigned

Labels

RPC Some notable changes to RPC params/behaviour/descriptions

Projects

None yet

Milestone

18

Development

Successfully merging this pull request may close these issues.

3 participants

@kwvg @PastaPastaPasta @UdjinM6