-
Notifications
You must be signed in to change notification settings - Fork 725
[Validation][RPC] ProUpReg and ProUpRev special tx types #2363
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Validation][RPC] ProUpReg and ProUpRev special tx types #2363
Conversation
82731f0 to
c4b4dc3
Compare
7cdfbff to
e6d827a
Compare
e6d827a to
13a82fd
Compare
fe111b3 to
ebcd13b
Compare
ebcd13b to
d19c4b8
Compare
d19c4b8 to
fb3f567
Compare
|
Rebased on master. Ready to go. |
furszy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed till e5292cf08a86a295983adc33ac08781225a82913. Still moving but looking great
furszy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial code review ACK fb3f567a2e563caa7d3f0c5f45dfa4df9af39195, only found nits. Really nice work, awesome test coverage👌👌.
fb3f567 to
853d8e1
Compare
|
Thanks for the review @furszy. Nits addressed. |
furszy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code ACK 853d8e1 after nits.
will start running it and test it live now.
853d8e1 to
663517e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-utACK after rebase 663517e. Last one in the 2267 work path 🍻 .
|
Small conflict, needs rebase :\ |
663517e to
e60ada9
Compare
|
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
|
Rebased on master (once again), and serialization updated. |
furszy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fuzzbawls
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code ACK 3c76eaf1048b595f72f812d377ca3be00dc3edf0
furszy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have run all the tests locally one more time, ACK 27d9f4e. Merging..
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
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.