[Validation][RPC] ProUpReg and ProUpRev special tx types#2363
Merged
furszy merged 20 commits intoPIVX-Project:masterfrom Jul 7, 2021
Merged
[Validation][RPC] ProUpReg and ProUpRev special tx types#2363furszy merged 20 commits intoPIVX-Project:masterfrom
furszy merged 20 commits intoPIVX-Project:masterfrom
Conversation
82731f0 to
c4b4dc3
Compare
7cdfbff to
e6d827a
Compare
e6d827a to
13a82fd
Compare
1 task
fe111b3 to
ebcd13b
Compare
ebcd13b to
d19c4b8
Compare
3 tasks
d19c4b8 to
fb3f567
Compare
Author
|
Rebased on master. Ready to go. |
furszy
reviewed
Jun 25, 2021
furszy
left a comment
There was a problem hiding this comment.
Reviewed till e5292cf08a86a295983adc33ac08781225a82913. Still moving but looking great
furszy
reviewed
Jun 26, 2021
furszy
left a comment
There was a problem hiding this comment.
Initial code review ACK fb3f567a2e563caa7d3f0c5f45dfa4df9af39195, only found nits. Really nice work, awesome test coverage👌👌.
fb3f567 to
853d8e1
Compare
Author
|
Thanks for the review @furszy. Nits addressed. |
furszy
reviewed
Jun 26, 2021
furszy
left a comment
There was a problem hiding this comment.
Code ACK 853d8e1 after nits.
will start running it and test it live now.
853d8e1 to
663517e
Compare
furszy
previously approved these changes
Jun 30, 2021
|
Small conflict, needs rebase :\ |
663517e to
e60ada9
Compare
Author
|
Rebased... |
018a474 to
959f595
Compare
check: - valid payload - payload malleability - owner script payout change - duplicate operator key - duplicate owner key
Otherwise a block with two protx using the same key could be accepted and then cause an assertion failure in AddUniqueProperty.
check: - valid payload - payload malleability
As changing the payout address, could be used maliciously to game the old payment system.
MalleateProUpServTx shouldn't re-use the ip (port) of an active masternode. Select the port with a number > 1000
959f595 to
27d9f4e
Compare
Author
|
Rebased on master (once again), and serialization updated. |
Fuzzbawls
approved these changes
Jul 7, 2021
Collaborator
Fuzzbawls
left a comment
There was a problem hiding this comment.
Code ACK 3c76eaf1048b595f72f812d377ca3be00dc3edf0
furszy
added a commit
that referenced
this pull request
Oct 8, 2021
d3843cd [Refactor] No need to get the publickey of the active masternode (random-zebra) 483f509 [Refactor] De-duplicate operator BLS key parsing in rpc (random-zebra) 855c094 [Build] Fix CMake builds with chiabls/relic includes (random-zebra) 77ec208 [Tests] Fix TierTwo functional tests with BLS operator key (random-zebra) dd46f72 [TierTwo] Migration of DMN operator key to BLS (random-zebra) 1dfdcc1 [RPC] Implement generateblskeypair() RPC command (random-zebra) 619c0f8 scripted-diff: Rename keyIDOperator to pubKeyOperator (random-zebra) b0a7fa5 [QA] Add tests for fbv messages signed with BLS keys (random-zebra) dd0fb01 [TierTwo] Add functions to sign/verify messages with BLS (random-zebra) Pull request description: This builds on top of: - [x] #2363 - [x] #2419 - [x] #2420 As per title, introduce BLS keys/signatures for messages signed by the masternode operator (which is a requirement for [DIP 6](https://github.com/dashpay/dips/blob/master/dip-0006.md), and subsequent upgrades). - a new RPC command `generateblskeypair` returns a new publickey/secretkey BLS keypair. - `CDeterministicMNState::keyIDOperator` is now a `CBLSLazyPublicKey` (instead of a `CKeyID`), and it's renamed `pubKeyOperator`. - `-mnoperatorprivatekey` flag takes a BLS secret key in hex format. - `CActiveMasternodeInfo` stores a `CBLSPublicKey`/`CBLSSecretKey` - the BLS signature byte vector is serialized for legacy messages (mnw, fbv) in the compatibility code. ACKs for top commit: furszy: great, ACK d3843cd Fuzzbawls: ACK d3843cd Tree-SHA512: 1caf49615283d10b356caa049b57091dc70805c0bf2c11645d560c81223ce0d9936a46096cc4871c5ec25164518ebdf259d0f23a8c5bca39f2e436cc9e954519
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Built on top of:
This concludes the first DMN-milestone list #2267 (comment).
Here we introduce the last two payloads and transaction types, adding relative RPC commands, and updating the tests:
PROUPREG(provider-update-registrar) submitted by the owner (the payload must be signed with the owner key) to update the operator key and/or the voting key and/or the payout address.PROUPREV(provider-update-revoke) submitted by the operator, to revoke the service, and put the mn in PoSe-banned state (e.g. in case of compromised keys). The masternode can be "revived" later, by sending a ProUpReg tx, which sets a new operator key, and then a ProUpServ tx (signed with the new operator key), which sets the new IP address for the masternode.