Skip to content

[Refactor] Reorder mnw processing and validation #2584

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
furszy merged 3 commits into from
Nov 8, 2021
Merged

[Refactor] Reorder mnw processing and validation #2584

furszy merged 3 commits into from
Nov 8, 2021

Conversation

@random-zebra
Copy link

@random-zebra random-zebra commented Oct 4, 2021

Extracted from #2421 (and based on top of it) so #2550 can be merged in between.

@random-zebra random-zebra added this to the 6.0.0 milestone Oct 4, 2021
@random-zebra random-zebra self-assigned this Oct 4, 2021
@random-zebra random-zebra mentioned this pull request Oct 4, 2021
Merged
3 tasks
@random-zebra random-zebra force-pushed the 202110_mnw-processing-order branch 2 times, most recently from e5b8e02 to 943001a Compare October 13, 2021 11:50
@random-zebra
Copy link
Author

random-zebra commented Oct 13, 2021

Rebased on master. Ready to go.

@random-zebra random-zebra modified the milestones: 6.0.0, 5.4.0 Oct 13, 2021
furszy
furszy previously approved these changes Oct 19, 2021
View reviewed changes
Copy link

@furszy furszy left a comment •
edited
Loading

Choose a reason for hiding this comment

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

nice refactor, ACK 943001a74f66b1ba7c9902cbeafcedfa60943131.

Plus, made a test case for the "non-existent" mnwinner voter: furszy@fa043ae

@random-zebra random-zebra force-pushed the 202110_mnw-processing-order branch from 943001a to 26513b9 Compare October 21, 2021 13:51
@random-zebra
Copy link
Author

random-zebra commented Oct 21, 2021

Rebased, renamed Voted to RecordWinnerVote and picked furszy's test case.

furszy
furszy previously approved these changes Oct 21, 2021
View reviewed changes
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

ACK 26513b98423f2e071559a863bc79a0d2f89717bc

with a comment that can be tackled here or in a follow up work.

furszy
furszy previously approved these changes Oct 21, 2021
View reviewed changes
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

good, ACK 70ee3c66c156b30f8900130a0a413d45184d38d0

Fuzzbawls
Fuzzbawls previously approved these changes Oct 27, 2021
View reviewed changes
Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

ACK 70ee3c66c156b30f8900130a0a413d45184d38d0

@random-zebra
Copy link
Author

random-zebra commented Oct 27, 2021

I think #2611 has higher priority than this one and should be merged first (as it touches some of the same lines in CMasternodePayments::ProcessMNWinner).

@furszy
Copy link

furszy commented Oct 27, 2021

yeah, all good, will not merge it yet.
Let's stay focused on testing, merging and back porting #2611. High importance for the coming superblock 👌.

@random-zebra random-zebra dismissed stale reviews from Fuzzbawls and furszy via e8559fc October 30, 2021 10:07
@random-zebra random-zebra force-pushed the 202110_mnw-processing-order branch from 70ee3c6 to e8559fc Compare October 30, 2021 10:07
@random-zebra
Copy link
Author

random-zebra commented Oct 30, 2021

Rebased on master, now that #2611 has been merged.

furszy
furszy approved these changes Nov 1, 2021
View reviewed changes
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

utACK e8559fc after rebase

@furszy furszy merged commit c5ef662 into PIVX-Project:master Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Fuzzbawls Fuzzbawls Fuzzbawls left review comments

+1 more reviewer

@furszy furszy furszy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@random-zebra random-zebra

Labels

Cleanup Masternode Refactoring

Projects

None yet

Milestone

5.4.0

Development

Successfully merging this pull request may close these issues.

3 participants

@random-zebra @furszy @Fuzzbawls