Skip to content

Conversation

@codablock
Copy link

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

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

Copy link

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

Copy link

Choose a reason for hiding this comment

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

n prefix

Copy link

Choose a reason for hiding this comment

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

n prefix

Copy link

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

Copy link

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)

Copy link

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
Author

Pushed review fixes

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

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

Copy link
Member

Choose a reason for hiding this comment

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

if( -> if (

Copy link
Member

Choose a reason for hiding this comment

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

if( -> if (

Copy link
Member

Choose a reason for hiding this comment

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

for( -> for (

Copy link
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

Copy link
Member

Choose a reason for hiding this comment

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

same above

Copy link
Member

Choose a reason for hiding this comment

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

same

Copy link
Member

Choose a reason for hiding this comment

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

same

Copy link
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

Copy link
Member

Choose a reason for hiding this comment

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

same

Copy link

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

Choose a reason for hiding this comment

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

done

Copy link
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
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

@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

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
Author

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

Copy link

@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