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.

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

[Refactor] Reorder mnw processing and validation #2584

Merged

Conversation

random-zebra
Copy link

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 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

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
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.

nice refactor, ACK 943001a.

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

src/masternode-payments.cpp Outdated Show resolved Hide resolved
@random-zebra
Copy link
Author

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

furszy
furszy previously approved these changes Oct 21, 2021
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 26513b9

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

src/masternode-payments.cpp Outdated Show resolved Hide resolved
furszy
furszy previously approved these changes Oct 21, 2021
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 70ee3c6

Fuzzbawls
Fuzzbawls previously approved these changes Oct 27, 2021
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 70ee3c6

@random-zebra
Copy link
Author

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
Copy link
Author

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

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants