[Consensus][Refactoring] Blocks v10: TierTwo payments in the coinbase transaction #2274
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Mar 26, 2021
random-zebra
force-pushed
the
202103-mnpayee-refactoring
branch
from
68ff087
to
1a98bf8
Compare
random-zebra
force-pushed
the
202103-mnpayee-refactoring
branch
3 times, most recently
from
e7d61f6
to
98667d5
Compare
|
Can be rebased now. |
random-zebra
force-pushed
the
202103-mnpayee-refactoring
branch
from
98667d5
to
89416a1
Compare
|
Rebased. |
|
Thanks for the feedback @furszy! |
furszy
reviewed
Apr 29, 2021
There was a problem hiding this comment.
Nice cool 👌, thanks for jumping over the feedback so quickly. Left some more comments after doing another full round.
The only comment that you missed to tackle from the previous round was the addMnToScores refactor that has the same rationale as the CanScheduleMN refactor, from this comment #2274 (comment)
Other than that and as a final line, awesome unit tests inclusion + further changes into the budget one, loved them 🥃 .
1 task
random-zebra
force-pushed
the
202103-mnpayee-refactoring
branch
from
2651d4f
to
2410ee6
Compare
|
Updated as per review and rebased on master. |
furszy
previously approved these changes
Apr 30, 2021
There was a problem hiding this comment.
Code review ACK 2410ee6, great work 👌 ☕ .
A good future PR work can be to create a functional test for the coinbase and coinstake changes pre and post v6 enforcement, overminting, sending to a different payee etc.
Crafting invalid blocks similar to how it is done in #2346. So we can validate the entire block processing flow as well.
random-zebra
force-pushed
the
202103-mnpayee-refactoring
branch
from
2410ee6
to
42005be
Compare
|
Sadly, this PR requires a rebase. Will re-re-review the changes after it. |
this will be needed in the following commits
Masternode and budgets are paid as outputs of the coinbase transaction rather than the coinstake.
There is no change in the behavior here. We just prepare the functions for a simpler integration with the new payment logic by: - passing the pindexPrev (instead of just the height), so we don't introduce additional cs_main locks, to look into the active chain, before accessing the deterministic list. - using a *vector* of outputs (instead of a single one) since DMNs could require a payment for the mn operator.
- use MasternodeRef for const mn pointers - Initialize CMasternodePaymentWinner with the height - CMasternodeMan: - CompareLastPaid/CompareScoreTxIn/CompareScoreMN (same) - Save MasternodeRefs in vecMasternodeLastPaid (instead of ins) - Refactor/Simplify GetMasternodeRank and GetCurrentMasterNode
random-zebra
force-pushed
the
202103-mnpayee-refactoring
branch
from
42005be
to
c2060ef
Compare
Fuzzbawls
approved these changes
May 5, 2021
random-zebra
added a commit
that referenced
this pull request
May 11, 2021
…get_tests 42be32c [Trivial] Rename AddSporkMessage --> AddOrUpdateSporkMessage (random-zebra) 76fbe44 [Refactoring] Use AddSporkMessage globally for maps update (random-zebra) 7c68750 [Cleanup][Trivial] Remove un-implemented function ExecuteSpork (random-zebra) eb271ef [Refactoring] Remove mocked BudgetManagerTest::IsBlockValueValid (random-zebra) Pull request description: Extracted from: - [x] #2274 and based on top of it. Forcing the update of the spork message and finalized budget, we can test the actual `IsBlockValueValid` function. This also refactors the spork code to update the maps in a single place, as suggested by @furszy here #2274 (comment). ACKs for top commit: furszy: ACK 42be32c 👍 . Fuzzbawls: ACK 42be32c Tree-SHA512: e1ef9aed9a4ff37f1dac4505d6213a93d75eba28d4592168f267c3089b60ff4e81c0d292bd1c8a34746124c3efbf479f2f42862fea414f4f31ed07e701ac8a57
furszy
added a commit
that referenced
this pull request
May 11, 2021
4a5baf3 [Doc] Add cold-staking changes to release notes (random-zebra) c19416f [GUI] Use new opcode for P2CS after v6 enforcement (random-zebra) 45ebfee [RPC] Use new opcode for P2CS after v6 enforcement (random-zebra) 8eefab5 [Tests] Add script test for new opcode (random-zebra) df11631 [Script] Introduce new OP_CHECKCOLDSTAKEVERIFY opcode (random-zebra) b194386 [Refactor] Rename CCSV opcode to OP_CHECKCOLDSTAKEVERIFY_LOF (random-zebra) Pull request description: Extracted from #2267, and rebased on top of #2258. Given the consensus change, introduced in #2274, we can define a more secure `OP_CHECKCOLDSTAKEVERIFY` opcode, which doesn't leave the last output of the coinstake "free" (as we no longer pay masternode/budgets in the coinstake tx). Built on top of: - [x] #2258 - [x] #2274 ACKs for top commit: Fuzzbawls: re-Code ACK 4a5baf3 after rebase, no code changes. furszy: ACK 4a5baf3. Tree-SHA512: ee78b76137e40df05b854f8959755f1b99e7c7328fb13708ba7e3e6dae6357450210ce19ed7a99dc54aa0d6051d0dbd745af70f4b6e9927de5d3cbb2e04fffb6
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Extracted from #2267.
This PR introduces an important consensus change. As per title, mn/budget payments are now outputs of the coinbase transaction, rather than the coinstake (which, now, contains only outputs belonging to the staker).
The block version is bumped to 10.
Also refactor the budget-payment functions, in preparation for the compatibility code and the new payment logic.
Built on top of: