Skip to content

Conversation

@furszy
Copy link

@furszy furszy commented Sep 21, 2021

Have refactored the mnb validation process, placing the code where it should had been in the first place (masternode manager and not be an internal masternode function that is called from and internally calls to the manager..).

Doing it found a bug over the MN-DMN compatibility code:
if the mnb received is from an active DMN, it needs to not be relayed to the network as it's an invalid mnb that no node will accept. Active DMNs have prevalence over regular MNs.

And improved an un-performant, on disk, tx look up to check the mnb pubkey association with the collateral key. Use the coins cache instead.

@furszy furszy self-assigned this Sep 21, 2021
@furszy furszy force-pushed the 2021_mnb_process_refactoring branch 2 times, most recently from 105b1d6 to 126314d Compare September 21, 2021 13:53
@furszy furszy added this to the 5.4.0 milestone Sep 21, 2021
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.

Good find for the un-checked return value of Add(). 👍
But I think it would be better to move the dmn-collateral-reuse test out of Add(), and place it at the beginning of ProcessMNBroadcast (where it should have been in the first place, as it is much faster than the following tests).

@furszy
Copy link
Author

furszy commented Sep 23, 2021

Agree on the dmn-collateral reuse check movement out of Add(), but better to do it in a follow up PR. Quite sure that will open another set of chained refactorings.. the function is being called from ProcessMNB and from UpdateMasternodeList (which is called from the RPC and GUI..).

@furszy furszy changed the title [TierTwo] MNB process refactor [BUG][TierTwo] MNB process refactor Oct 1, 2021
furszy added 6 commits October 1, 2021 18:55
The location where this function should have been placed in the first place..
if the mnb received is from an active DMN, it needs to not be relayed to the network as it's an invalid mnb that no node will accept.
@furszy furszy force-pushed the 2021_mnb_process_refactoring branch from 126314d to cb6740a Compare October 1, 2021 22:05
@furszy
Copy link
Author

furszy commented Oct 1, 2021

rebased on master so it gets the latest tier two bugs fixes.

@furszy furszy requested a review from Fuzzbawls October 3, 2021 13:40
@furszy furszy force-pushed the 2021_mnb_process_refactoring branch from cb6740a to 4a67ede Compare October 9, 2021 00:21
@furszy
Copy link
Author

furszy commented Oct 9, 2021

feedback tackled.

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.

Code ACK 4a67ede

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

@random-zebra random-zebra merged commit ccb2a43 into PIVX-Project:master Oct 11, 2021
@furszy furszy deleted the 2021_mnb_process_refactoring branch November 29, 2022 15:43
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