[Consensus] New masternode payment logic #2309
Conversation
random-zebra
force-pushed
the
202103-dmn-payments
branch
from
b9b6664 to
b2bef3f
Compare
random-zebra
force-pushed
the
202103-dmn-payments
branch
3 times, most recently
from
1b99c7c to
c3c3202
Compare
|
Rebased on master. Next in line for the DIP3 list. |
Fuzzbawls
left a comment
Fuzzbawls
left a comment
There was a problem hiding this comment.
Code ACK c3c320204826faab99c6590a5fb27994d937d2d7
few non-blocking whitespace nits, but otherwise looks good
random-zebra
force-pushed
the
202103-dmn-payments
branch
from
c3c3202 to
69e6ab5
Compare
|
Fixed whitespaces. |
Fuzzbawls
left a comment
Fuzzbawls
left a comment
There was a problem hiding this comment.
re-Code ACK 69e6ab5fda2ea0d14f7175ac122db43ffec5611a
furszy
left a comment
furszy
left a comment
There was a problem hiding this comment.
Code review completed
Spent some time here, have expanded the test coverage to verify that an invalid block paying to a DMN that shouldn't be paying is being rejected. Plus, found out that the new DMN payments validation flow is not being tested, it is just skipped due the not finished masternode sync process and the disabled MN payments spork in the test (the IsBlockPayeeValid function is just returning true without performing any validation at line 245).
The work is here https://github.com/furszy/PIVX/commits/202103-dmn-payments, all yours ☕.
random-zebra
force-pushed
the
202103-dmn-payments
branch
from
69e6ab5 to
b39a504
Compare
|
Thanks for the review @furszy . Added a commit at the end, as, rebasing on master, I noticed that I broke this test in #2360. |
random-zebra
force-pushed
the
202103-dmn-payments
branch
from
b39a504 to
ed39000
Compare
Bug introduced in PIVX-Project@0b670c8 When the coinbase includes a masternode payment, if the entire block value is given to the miner (first coinbase output), the block over-mints.
random-zebra
force-pushed
the
202103-dmn-payments
branch
from
ed39000 to
6b7b9df
Compare
4fd564c [Tests] Add EvoNotificationInterface in the TestingSetup fixture (random-zebra) f6aa6ad [BUG] Check masternode/budget payments during block connection (random-zebra) 57fa99f [Tests] Introduce tiertwo_reorg_mempool test (random-zebra) 9a1bf27 [BUG] Add special tx processing to RollforwardBlock (random-zebra) 5d6bcef [RPC][Tests] Include proTx data in json formatted transactions (random-zebra) 93e8453 [Tests] Update deterministicmns test checking collateral locking (random-zebra) cc132a8 [Wallet] Implement auto-locking of masternode collaterals (random-zebra) c857636 [Tests] Introduce new functional test tiertwo_deterministicmns.py (random-zebra) Pull request description: Another one coming from #2267 This PR introduces a "auto-locking" feature for masternode collaterals: - at startup, the wallet is scanned, and any masternode collateral coin found gets locked. Can be disabled setting `-mnconflock=0` (although, there is no need for a `masternode.conf` anymore, so we might want to consider defining and using a new flag here, and removing `mnconflock` after 6.0). - during runtime, when a ProRegTx is processed by `CWallet::AddToWalletIfInvolvingMe`, the wallet checks for ownership of the collateral, and automatically locks it in case. Here, we also add two more functional tests (`tiertwo_deterministicmns.py`, `tiertwo_reorg_mempool.py`), update the other tests, checking the auto-locking feature, and fix a couple bugs: - Missing special tx processing in RollforwardBlock - Validity of tiertwo payments can (and must) be verified only during block connection, as we keep the deterministic state only for the active chain, not for all possible chaintips. Builds on top of: - [x] #2308 - [x] #2309 ACKs for top commit: furszy: re-ACK 4fd564c after the tiny fix and merging.. Tree-SHA512: b0f05d9265c02fe90baf48e5dcf66b34f078b90ffd0eef1488606013f84996b2ebaa7d70725c59db2f7456105a6eff2ba08da966858f81c4f19d014a72cd7692
3 participants
Estracted from #2267
This implements the new payment logic for deterministic masternodes, to be used after the deactivation of the legacy system with SPORK_21.
Based on top of: