Skip to content

DIP3 MN reward payment logic#2258

Merged
UdjinM6 merged 7 commits into
dashpay:developfrom
codablock:pr_dip3_paymentlogic
Sep 5, 2018
Merged

DIP3 MN reward payment logic#2258
UdjinM6 merged 7 commits into
dashpay:developfrom
codablock:pr_dip3_paymentlogic

Conversation

@codablock
Copy link
Copy Markdown

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.

@codablock codablock changed the title Pr dip3 paymentlogic DIP3 MN reward payment logic Sep 4, 2018
@UdjinM6 UdjinM6 added this to the 12.4 milestone Sep 4, 2018
Copy link
Copy Markdown

@UdjinM6 UdjinM6 left a 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 👍

Comment thread src/masternode-payments.cpp Outdated
Copy link
Copy Markdown

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?)

Comment thread src/masternode-payments.cpp Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

n prefix

Comment thread src/masternode-payments.cpp Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

n prefix

Comment thread src/rpc/masternode.cpp Outdated
Copy link
Copy Markdown

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?)

Comment thread src/dsnotificationinterface.cpp Outdated
Copy link
Copy Markdown

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)

Comment thread src/privatesend.cpp Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

@codablock codablock force-pushed the pr_dip3_paymentlogic branch from b5e2618 to a0e45d8 Compare September 4, 2018 12:41
@codablock
Copy link
Copy Markdown
Author

Pushed review fixes

UdjinM6
UdjinM6 previously approved these changes Sep 4, 2018
Copy link
Copy Markdown

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

Copy link
Copy Markdown
Member

@PastaPastaPasta PastaPastaPasta left a 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

Comment thread src/masternode-payments.cpp Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if( -> if (

Comment thread src/masternode-payments.cpp Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if( -> if (

Comment thread src/masternode-payments.cpp Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for( -> for (

Comment thread src/masternode-payments.cpp Outdated
Copy link
Copy Markdown
Member

@PastaPastaPasta PastaPastaPasta Sep 4, 2018

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

Comment thread src/masternode-payments.cpp Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same above

Comment thread src/masternode-payments.cpp Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

Comment thread src/masternode-payments.cpp Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

Comment thread src/rpc/masternode.cpp Outdated
Copy link
Copy Markdown
Member

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

Comment thread src/rpc/masternode.cpp Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

Copy link
Copy Markdown

@UdjinM6 UdjinM6 Sep 5, 2018

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread src/rpc/masternode.cpp Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

@codablock
Copy link
Copy Markdown
Author

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

@codablock codablock force-pushed the pr_dip3_paymentlogic branch from 3837215 to 96e5695 Compare September 5, 2018 11:49
UdjinM6
UdjinM6 previously approved these changes Sep 5, 2018
Copy link
Copy Markdown

@UdjinM6 UdjinM6 left a 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.

@UdjinM6
Copy link
Copy Markdown

UdjinM6 commented Sep 5, 2018

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
@codablock
Copy link
Copy Markdown
Author

Merge conflicts fixed. These were just minor ones caused by code cleanups

Copy link
Copy Markdown

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-utACK

@UdjinM6 UdjinM6 merged commit f016638 into dashpay:develop Sep 5, 2018
@codablock codablock deleted the pr_dip3_paymentlogic branch September 14, 2018 12:51
thephez added a commit to dash-docs/dash-docs that referenced this pull request Dec 18, 2018
thephez added a commit to dash-docs/dash-docs that referenced this pull request Dec 26, 2018
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants