Skip to content

[Test] Add MN payments test coverage #2550

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 7 commits into from
Oct 12, 2021
Merged

[Test] Add MN payments test coverage #2550

furszy merged 7 commits into from
Oct 12, 2021

Conversation

@furszy
Copy link

@furszy furszy commented Sep 10, 2021

Expanded the unit test suite adding coverage for Masternode payments.

@furszy furszy self-assigned this Sep 10, 2021
random-zebra
random-zebra reviewed Sep 10, 2021
View reviewed changes
@furszy furszy force-pushed the 2021_mnpayments_test_final branch from a40abdb to 4eb8c52 Compare September 10, 2021 17:13
@furszy furszy added the Tests label Sep 10, 2021
@furszy furszy added this to the 5.4.0 milestone Sep 10, 2021
random-zebra
random-zebra reviewed Sep 13, 2021
View reviewed changes
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

Have checked only the refactoring, not the tests yet. Left some notes.

This should probably wait for #2421, or be based on top of it, to avoid some code churn.
2421 introduces BLS signatures for mnw messages created by deterministic nodes, changing exactly these functions (e.g. see c12d50a5ce4929c556cead67c4c607e571c516c2).

@furszy furszy force-pushed the 2021_mnpayments_test_final branch from 4eb8c52 to d1de379 Compare September 13, 2021 16:58
@random-zebra random-zebra mentioned this pull request Oct 4, 2021
Merged
@furszy furszy force-pushed the 2021_mnpayments_test_final branch 2 times, most recently from 238843f to 5af4373 Compare October 9, 2021 01:07
@furszy
Copy link
Author

furszy commented Oct 9, 2021

Rebased on master, conflicts and feedback tackled.
Ended up moving CreateMNWinnerPayment inside the new MN payments test file. Will work on adding test coverage for the recently merged DMN signed mnwinners in a follow up PR.

Ready to go finally.

@furszy furszy requested a review from Fuzzbawls October 10, 2021 13:34
@furszy furszy mentioned this pull request Oct 10, 2021
Merged
random-zebra
random-zebra previously approved these changes Oct 11, 2021
View reviewed changes
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

Few non-blocking notes. All else looking good. utACK 5af43735abdafc583cf12ab1301e3d990f29536b

@furszy furszy force-pushed the 2021_mnpayments_test_final branch from 5af4373 to a7a5796 Compare October 11, 2021 12:39
@furszy
Copy link
Author

furszy commented Oct 11, 2021

Done, zebra's feedback tackled ☕
Added a last commit correcting the masternodePayments object calls from the masternodePayments methods. There was another one.

random-zebra
random-zebra previously approved these changes Oct 11, 2021
View reviewed changes
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

Easy diff. re-utACK a7a579652b041b81d2c324f4f8d6d096cb4201ef

Fuzzbawls
Fuzzbawls previously approved these changes Oct 12, 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.

utACK a7a579652b041b81d2c324f4f8d6d096cb4201ef with a nit

@furszy furszy dismissed stale reviews from Fuzzbawls and random-zebra via 5833e69 October 12, 2021 13:28
@furszy furszy force-pushed the 2021_mnpayments_test_final branch from a7a5796 to 5833e69 Compare October 12, 2021 13:28
random-zebra
random-zebra approved these changes Oct 12, 2021
View reviewed changes
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

utACK 5833e69

@furszy furszy merged commit 3e2d8e3 into PIVX-Project:master Oct 12, 2021
furszy added a commit that referenced this pull request Nov 8, 2021
@furszy
Merge #2584: [Refactor] Reorder mnw processing and validation
c5ef662 
e8559fc [Refactor] make AddWinningMasternode a void function (random-zebra)
5aaa02a [Test] add mnwinner case for non-existent MN. (furszy)
e68773b [Refactor] Fix checks order for mnw processing (random-zebra)

Pull request description:

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

ACKs for top commit:
  furszy:
    utACK e8559fc after rebase

Tree-SHA512: 659d4ca43502f8e1e8f6ddbf7c815f6a07a69625f00c8ab71b7d8e9c97b85e90d97b18ccc988dcfd4d345d5f79932128abdaa0f0634380be7d715d3a3e45c6fc
@furszy furszy deleted the 2021_mnpayments_test_final branch May 27, 2023 01:55
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

@random-zebra random-zebra random-zebra approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@furszy furszy

Labels

Masternode Refactoring Tests

Projects

None yet

Milestone

5.4.0

Development

Successfully merging this pull request may close these issues.

3 participants

@furszy @Fuzzbawls @random-zebra