Skip to content

Conversation

@domob1812
Copy link

Currently, masternode rewards are paid to the address that holds the collateral. In some situations, this is restrictive, and it would be useful to give masternode owners a choice about where the reward should be paid to (for instance, this will allow easier setup of masternode vaults for improved security).

This set of changes implements a protocol upgrade, which then allows masternodes to (optinally) configure an arbitrary script to which rewards should be paid. Until the upgrade and until a particular masternode owner decides to explicitly opt into this feature, rewards will continue to be paid to the default script (matching the collateral address).

The protocol update itself will be activated together with the "staking vault fork" (#38), namely when 95% of the past blocks have version 5. This ensures that it happens in a decentralised and coordinated fashion, but only when a large majority of the network is already running code that supports the new logic.

@domob1812 domob1812 force-pushed the custom-reward-address branch from c46d48f to cc124f6 Compare October 22, 2020 14:32
@domob1812 domob1812 marked this pull request as draft November 6, 2020 14:07
@domob1812 domob1812 force-pushed the custom-reward-address branch from cc124f6 to 77bdf1c Compare November 6, 2020 15:26
@domob1812 domob1812 force-pushed the custom-reward-address branch from 77bdf1c to 0cdcc86 Compare November 9, 2020 13:25
@domob1812
Copy link
Author

Once #46 is merged (as well as #38), the last commit (activation of the protocol change) needs to be updated.

@domob1812 domob1812 force-pushed the custom-reward-address branch 2 times, most recently from a8c4114 to 5fdc036 Compare November 19, 2020 14:06
@domob1812 domob1812 mentioned this pull request Dec 1, 2020
@domob1812 domob1812 force-pushed the custom-reward-address branch 2 times, most recently from 48c146e to 804cc64 Compare December 1, 2020 13:36
@domob1812
Copy link
Author

This is mostly ready, except that once #38 is merged, we can drop the commit that introduces the StakingVaults fork dummy.

@domob1812 domob1812 force-pushed the custom-reward-address branch from 804cc64 to 87574f6 Compare December 4, 2020 13:14
@domob1812 domob1812 marked this pull request as ready for review December 4, 2020 13:14
@domob1812
Copy link
Author

#38 is merged now, so this is ready for review.

@domob1812 domob1812 force-pushed the custom-reward-address branch from 87574f6 to bd5b3f5 Compare December 8, 2020 11:12
@domob1812 domob1812 force-pushed the custom-reward-address branch 3 times, most recently from c5f19af to f72de76 Compare January 7, 2021 14:00
@domob1812 domob1812 marked this pull request as draft January 12, 2021 13:10
@domob1812 domob1812 force-pushed the custom-reward-address branch 8 times, most recently from 01297fe to 90051ea Compare January 18, 2021 14:47
@domob1812 domob1812 force-pushed the custom-reward-address branch from 90051ea to 674491b Compare February 15, 2021 15:24
@domob1812 domob1812 force-pushed the custom-reward-address branch 6 times, most recently from acc793e to 7cb5b5f Compare March 1, 2021 15:43
@domob1812 domob1812 force-pushed the custom-reward-address branch from 7cb5b5f to 0b2e452 Compare March 4, 2021 14:57
@domob1812 domob1812 force-pushed the custom-reward-address branch 2 times, most recently from dba5417 to 3f3d165 Compare March 16, 2021 15:32
@domob1812 domob1812 force-pushed the custom-reward-address branch from 3f3d165 to c4e3552 Compare April 6, 2021 14:24
Wrap some of the functions used only in main into an anonymous namespace;
some of them were already marked as "static", others were not (but they
were still not needed from anywhere else).

This makes it immediately clear that those functions are only used in the
context of main.

FindUndoPos had a declaration first and the definition later, but it is
fine to just move the definition to where the declaration was.  This
simplifies the code further (and is a pure move in the context of
this commit).
@domob1812 domob1812 force-pushed the custom-reward-address branch 3 times, most recently from b7cd8a1 to ed50ba2 Compare April 7, 2021 12:44
This is a bunch of related and straight-forward cleanups to the code
around masternode configuration:  Pass arguments as const& instead of
by value, mark functions as const, and move helper functions from
being static in a class and declared inside the header to just being
inside an anonymous namespace in the cpp file.
This adds a new field, CScript rewardScript, to the masternode data
structures.  For now, we just add it, return it from the RPCs, and
set it to the script corresponding to the collateral pubkey on
creation.

We also serialise the new field as part of CMasternode, which is used
e.g. in the on-disk cache, but not the wire protocol (where
CMasternodeBroadcast is used instead).  Thus this change invalidates the
mncache format, but has no other effects on compatibility.

In the future, we will implement a protocol upgrade and also network
fork that will use this new field so masternodes can specify an arbitrary
script to which they want to receive their payments, rather than being
paid always to the collateral address.
Define a new wire protocol version, and for peers that match the
version, send the reward script as explicit part of the masternode
broadcast messages.

Also, if the reward script is not the default value, include it in the
signature message of the broadcast (so that the field is authenticated
by the collateral signature).

Until the protocol version is bumped on the network (which is not yet
the case with this commit), masternodes with non-default reward script
will not be accepted as valid.
Instead of using the collateral pubkey, pay to the declared reward
script of masternodes (and use that to check payments).
@domob1812 domob1812 force-pushed the custom-reward-address branch from ed50ba2 to 51f8f48 Compare April 7, 2021 12:57
Allow setting a non-default reward script for masternodes, by editing
the masternode.conf file explicitly.  The RPC setupmasternode will not
generate such configuration lines for now, and also "old" masternode.conf
files will remain valid (with the default script).
Use a custom reward address for one of the two masternodes in the
mnoperation.py regtest, verifying that this works.
This defines a new time-activated "fork", Fork::CustomRewardAddresses.
For now, it is scheduled at timestamp 2000000000; that will be set to
the actual activation time once it is determined.

When this fork becomes active (i.e. the current best block's timestamp
goes beyond the activation time), we require peers to use the new protocol
version that supports custom reward addresses.  The fork has no other
(in particular, no consensus-level) consequences.
This adds a new regtest, custom_address_update.py.  The test runs
two masternodes, where we explicitly set the protocol version of
one of the nodes to the "old" version not supporting custom reward
scripts for masternodes.

With this test, we ensure that all the masternode networking works fine
between "old" and "new" nodes before the update is activated, at least
as closely as we can without running a real old node.
@domob1812
Copy link
Author

Reopened as #83.

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.

2 participants