-
Notifications
You must be signed in to change notification settings - Fork 1.2k
refactor: proper support for composite commands such as 'bls generate' #6051
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
Conversation
This linter is subject to remove in 20012 anyway, which is todo after 18531 is done
UdjinM6
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.
LGTM, light ACK
PastaPastaPasta
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.
utACK 9413ecd
…ode NNN 64f3477 refactor: remove unused header spork.h from rpc/masternode (Konstantin Akimov) 43e9ab4 refactor: use new type of composite commands for masternode NNN (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented See #6051 ## What was done? Commands starting from 'masternode ...' uses new a new way to make composite commands. ## How Has This Been Tested? Run unit/functional tests. Please notice, we support both styles `masternodelist` and `masternode list` which are still both supported. ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone ACKs for top commit: PastaPastaPasta: utACK 64f3477 Tree-SHA512: 5a658588dff7f29d3025d2a720c8efe8096f79e8d632800911573f2f9a42450c2f27da070a94c5739e2027153413cdcacde82a7b4614467e036c008719692ed9
b478406 refactor: clean-up, missing const in rpc/quorums (Konstantin Akimov) 50c99e8 fix: adopt platform restriction for new composite commands (Konstantin Akimov) e3c4d66 refactor: quorum sign methods (Konstantin Akimov) 2af9d86 refactor: use new approach for rpc quorums NNN (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented See #6051 ## What was done? Commands starting from 'quorum ...' uses new a new way to make composite commands. ## How Has This Been Tested? Run unit/functional tests. Please notice, there's required extra changes for platform restrictions, the subcommand is no more first argument, but part of command, space separated. ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone ACKs for top commit: UdjinM6: utACK b478406 PastaPastaPasta: utACK b478406 Tree-SHA512: c53856a3f45fee44a8130876ed4545f6f2c0f59f74e4d4745e7354097fee3a74ed526cc8160de532dbb4aeb81b82638ee912a0961b910692e78889baa2c976c3
|
just note for history: |
7b44af2 refactor: move common code inside protx_register_common_wrapper (Konstantin Akimov) 89d5e26 refactor: replace bunch of bool flags to the enum (Konstantin Akimov) 1351bc9 refactor: move common code to the wrapper (Konstantin Akimov) 8d4c28a refactor: remove unused protx code from old implementation (Konstantin Akimov) 2a837f8 refactor: use new composite for RPC 'protx NNN' (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented See #6051 ## What was done? Commands starting from 'protx ...' uses new a new way to make composite commands. ## How Has This Been Tested? Run unit/functional tests. ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone ACKs for top commit: PastaPastaPasta: utACK 7b44af2 Tree-SHA512: a5124121de93ba566bfa6c7c58a5217446301dd027839d137e056df00388282af6be0feb0b81ea0217310e12231a68e3b405fa82b66df93a3fb159d341d7a199
…nce NNN 91aae7b refactor: use properly RPCHelpMan for governance RPC getsuperblockbudget (Konstantin Akimov) e92e5ef refactor: use properly RPCHelpMan for governance RPC voteraw (Konstantin Akimov) 87b3011 refactor: duplicated code between governance RPC list and diff (Konstantin Akimov) faa6179 docs: move comments from the old code to the new implementation of governance RPC (Konstantin Akimov) 39cd5e4 fix: format code for rpc governance (Konstantin Akimov) 0062421 refactor: use new style of composite commands for RPC gobject NNN (Konstantin Akimov) 42e0b37 refactor: use properly RPCHelpMan for governance RPC getgovernanceinfo (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented See #6051 ## What was done? Commands starting from 'governance ...' uses new a new way to make composite commands. This PR includes also a refactoring to remove common code between `gobject list` and `gobject diff` This PR includes also a refactoring to properly use RPCHelpMan for remaining governance's RPC. ## How Has This Been Tested? Run unit/functional tests. ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone ACKs for top commit: UdjinM6: utACK 91aae7b PastaPastaPasta: utACK 91aae7b Tree-SHA512: 46c00df924a7d2c5af799c605bfc479034019eed346f9d487827b0688993aa712ba3dc528d9bdd09b64f0e07b6940078337ca1baf0a1355f87eb792406c676ca
…latform needs 85abbb9 chore: add release notes for composite command for whitelist (Konstantin Akimov) 78ad778 feat: test composite commands in functional test for whitelist (Konstantin Akimov) a102a59 feat: add support of composite commands in RPC'c whitelists (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented dashpay/dash-issues#66 dashpay/dash-issues#65 ## What was done? Our composite commands such as "quorum list" have been refactored to make them truly compatible with other features, such as whitelist, see #6052 #6051 #6055 and other related PRs This PR makes whitelist feature to be compatible with composite commands. Instead implementing additional users such "dapi" better to provide universal way which do not require new build for every new API that has been used by platform, let's simplify things. Platform at their side can use config such as this one (created based on shumkov's example): ``` rpc: { host: '127.0.0.1', port: 9998, users: [ { user: 'dashmate', password: 'rpcpassword', whitelist: null, lowPriority: false, }, { username: 'platform-dapi', password: 'rpcpassword', whitelist: [], lowPriority: true, }, { username: 'platform-drive-consensus', password: 'rpcpassword', whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status] lowPriority: false, }, { username: 'platform-drive-other', password: 'rpcpassword', whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status] ], lowPriority: true, }, ], allowIps: ['127.0.0.1', '172.16.0.0/12', '192.168.0.0/16'], }, ``` ## How Has This Been Tested? Updated functional tests, see commits ## Breaking Changes n/a ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone ACKs for top commit: UdjinM6: LGTM, utACK 85abbb9 PastaPastaPasta: utACK 85abbb9 Tree-SHA512: 88608179c347420269880c352cf9f3b46272f3fc62e8e7158042e53ad69dc460d5210a1f89e1e09081d090250c87fcececade88e2ddec09f73f1175836d7867b
…s for platform needs 85abbb9 chore: add release notes for composite command for whitelist (Konstantin Akimov) 78ad778 feat: test composite commands in functional test for whitelist (Konstantin Akimov) a102a59 feat: add support of composite commands in RPC'c whitelists (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented dashpay/dash-issues#66 dashpay/dash-issues#65 ## What was done? Our composite commands such as "quorum list" have been refactored to make them truly compatible with other features, such as whitelist, see dashpay#6052 dashpay#6051 dashpay#6055 and other related PRs This PR makes whitelist feature to be compatible with composite commands. Instead implementing additional users such "dapi" better to provide universal way which do not require new build for every new API that has been used by platform, let's simplify things. Platform at their side can use config such as this one (created based on shumkov's example): ``` rpc: { host: '127.0.0.1', port: 9998, users: [ { user: 'dashmate', password: 'rpcpassword', whitelist: null, lowPriority: false, }, { username: 'platform-dapi', password: 'rpcpassword', whitelist: [], lowPriority: true, }, { username: 'platform-drive-consensus', password: 'rpcpassword', whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status] lowPriority: false, }, { username: 'platform-drive-other', password: 'rpcpassword', whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status] ], lowPriority: true, }, ], allowIps: ['127.0.0.1', '172.16.0.0/12', '192.168.0.0/16'], }, ``` ## How Has This Been Tested? Updated functional tests, see commits ## Breaking Changes n/a ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone ACKs for top commit: UdjinM6: LGTM, utACK 85abbb9 PastaPastaPasta: utACK 85abbb9 Tree-SHA512: 88608179c347420269880c352cf9f3b46272f3fc62e8e7158042e53ad69dc460d5210a1f89e1e09081d090250c87fcececade88e2ddec09f73f1175836d7867b
Issue being fixed or feature implemented
We have composite commands such as 'bls generate' that do not exist in bitcoin's implementation of rpc.
It doesn't let to backport yet bitcoin#18531 which enforced extra checks for arguments name (name of rpc and list arguments in rpc help and actual implementation must match).
What was done?
This PR improves support of composite commands in Dash Core. New style of composite commands are applied for
blscomposite commands:bls generateandbls fromsecretas proof of concept.Once this PR is merged, I will provide similar fixes for other "compose" rpc commands:
protx,masternode(and everything else if any).Beside better validation of arguments and command names, it improves suggest menu in Qt app (see a screenshot) for composite commands.
How Has This Been Tested?
Run unit and functional tests. Also extra tests to conduct in qt app:
Breaking Changes
N/A
Checklist: