-
Notifications
You must be signed in to change notification settings - Fork 1.2k
DIP3 MN reward payment logic #2258
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.
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
Conversation
UdjinM6
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of my usual complains :) otherwise looks good 👍
src/masternode-payments.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
map prefix + too general (mapPayments?)
src/masternode-payments.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
n prefix
src/masternode-payments.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
n prefix
src/rpc/masternode.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a string anymore (mapPayments?)
src/dsnotificationinterface.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should remove the one below then (line 18)
src/privatesend.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?
b5e2618 to
a0e45d8
Compare
|
Pushed review fixes |
UdjinM6
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
PastaPastaPasta
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See inline for formatting corrections
src/masternode-payments.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if( -> if (
src/masternode-payments.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if( -> if (
src/masternode-payments.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for( -> for (
src/masternode-payments.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
technically should be one line or brackets. ¯\(ツ)/¯ @nmarley
src/masternode-payments.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same above
src/masternode-payments.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
src/masternode-payments.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
src/rpc/masternode.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
above - brackets or same line
src/rpc/masternode.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: could probably squash 2 ifs above into || thing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/rpc/masternode.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
a0e45d8 to
3837215
Compare
|
@PastaPastaPasta applied a few of your suggestions, but left the ones out that are surrounded by the same type of code style violation. Better to create a PR with a general cleanup later. |
3837215 to
96e5695
Compare
UdjinM6
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-utACK
A couple of things here are not quite good for readability but let's get to it in some another cleanup PR.
|
ooops, merge conflit. sorry 🙈 |
Ensure correct locking order to fix deadlock.
1. Enforce payment to masternodes in IsBlockPayeeValid even if superblocks are triggered. This new rule only gets activated when spork15 activates. 2. Always enforce masternode payments when spork15 is activated and ignore spork8 in that case. spork8 can be removed after spork15 activation and hardening of the spork15 height into consensus params.
Needed for privatesend when choosing masternodes
96e5695 to
d8247df
Compare
|
Merge conflicts fixed. These were just minor ones caused by code cleanups |
UdjinM6
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-utACK
* Content - RPC - Update quick reference * RPC - Update getblockchaininfo to show BIP-9 progress Related to dashpay/dash#2435 * RPC - Update gobject prepare with new params Use-IS (dashpay/dash#2452) Use specific UTXO for fee (dashpay/dash#2482) * RPC - Update mode name * RPC - Update protx default mode dashpay/dash#2513 * Content - Add spork 17 * Content - Special transactions Add info for Quorum commitment Remove messages not in 13.0 (SubTx) * P2P - Add new txlvote fields masternodeProTxHash (dashpay/dash#2484) quorumModifierHash (dashpay/dash#2505) * RPC - Update protx list Make all options follow the same parameter format (dashpay/dash#2559) * Content - version bump 0.13.0.0 bumped to 70213 (dashpay/dash#2557) * Guide - PrivateSend dstx message limit Up to 5 simultaneous dstxs per MN allowed (dashpay/dash#2552) * RPC - Update getblock Add missing versionHex field (dashpay/dash@e7d9ffa) Change to use verbosity syntax (dashpay/dash#2506 and bitcoin/bitcoin#8704) * P2P - Add qfcommit message (no hexdump example) DIP6 quorum final commitment (dashpay/dash#2477) * P2P - qfcommit typo Change description of llmqType field * P2P - Special tx payload size clarification * Guide - Update MN payment description Related to dashpay/dash#2258 * Guide - fix broken link * Guide - Update some example txs Change to hashes on the chain following the 12.3.4 reset * P2P - Add QcTx hexdump * P2P - DIP4 message updates Add SML entry Update hexdump to include new fields Add getmnlistd and mnlistdiff to cross ref * P2P - minor DIP3-related comments
This is extracted from #2083. It introduces the new MN reward payment logic for deterministic masternodes.
It also enforces MN reward payments when superblocks are paid out (no more lucky miners getting 100% of the reward). This happens after spork15 activation.