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

[Test] Add MN payments test coverage #2550

Merged
merged 7 commits into from
Oct 12, 2021
Merged

[Test] Add MN payments test coverage #2550

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
@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 c12d50a).

src/masternode-payments.cpp Outdated Show resolved Hide resolved
src/masternode-payments.cpp Outdated Show resolved Hide resolved
src/masternode-payments.cpp Outdated Show resolved Hide resolved
src/masternode-payments.cpp Outdated Show resolved Hide resolved
@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 5af4373

src/masternode-payments.cpp Outdated Show resolved Hide resolved
src/test/mnpayments_tests.cpp Outdated Show resolved Hide resolved
src/masternode-payments.cpp Outdated Show resolved Hide resolved
src/masternode-payments.cpp Outdated Show resolved Hide resolved
src/test/mnpayments_tests.cpp Outdated Show resolved Hide resolved
@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 a7a5796

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 a7a5796 with a nit

src/masternode-payments.h Outdated Show resolved Hide resolved
@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

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

@Fuzzbawls Fuzzbawls Fuzzbawls left review comments

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