Skip to content

Split keyIDMasternode into keyIDOwner/keyIDOperator/keyIDVoting#2248

Merged
UdjinM6 merged 3 commits into
dashpay:developfrom
codablock:pr_dip3_keysplit
Aug 31, 2018
Merged

Split keyIDMasternode into keyIDOwner/keyIDOperator/keyIDVoting#2248
UdjinM6 merged 3 commits into
dashpay:developfrom
codablock:pr_dip3_keysplit

Conversation

@codablock
Copy link
Copy Markdown

This is extracted from #2083. It can stand on its own and without all the other DIP3 code, it will simply default to using the same key for all 3 new keys.

@codablock
Copy link
Copy Markdown
Author

Also added acf8203

@UdjinM6 UdjinM6 added this to the 12.4 milestone Aug 31, 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.

see inline comments

Comment thread src/rpc/governance.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.

nit: whitespaces

Comment thread src/rpc/governance.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.

same

Comment thread src/rpc/governance.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.

this looks unrelated/unused

Comment thread src/rpc/governance.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.

same

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.

keyIDMasternode? Is this intentional (for backwards compatibility) or ..?

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.

Yepp. This RPC will become unused after spork15 activation and at the same time I assume external tools are using this

Comment thread src/evo/providertx.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 probably alter rejection reason a bit so that it would be easier to debug i.e. bad-protx-payee-dest or smth like that

Comment thread src/evo/providertx.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.

same, e.g. bad-protx-payee-reuse

Comment thread src/evo/providertx.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.

The comment here is a bit confusing imo. This doesn't allow reuse of keys which control payout address for other purposes, not reuse of keys in general.

keyIDOwner is the key used for things which should stay in control of the
collateral owner, like proposal voting.

keyIDOperator is the key used for operational things, like signing network
messages, signing trigger/watchdog objects and trigger votes.

keyIDVoting is the key used for proposal voting

Legacy masternodes will always have the same key for all 3 to keep
compatibility.

Using different keys is only allowed after spork15 activation.
@codablock
Copy link
Copy Markdown
Author

Pushed review fixes

UdjinM6
UdjinM6 previously approved these changes Aug 31, 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

@codablock
Copy link
Copy Markdown
Author

@UdjinM6 Forgot to bump CMasternodeMan version string, pushed this now

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 25545fc into dashpay:develop Aug 31, 2018
@codablock codablock deleted the pr_dip3_keysplit branch September 14, 2018 12:50
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