feat: make whitelist works with composite commands for platform needs #6100

knst · 2024-07-07T11:18:00Z

Issue being fixed or feature implemented

https://github.com/dashpay/dash-issues/issues/66
https://github.com/dashpay/dash-issues/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:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added or updated relevant unit/integration/functional/e2e tests
I have made corresponding changes to the documentation
I have assigned this pull request to a milestone

src/init.cpp

src/httprpc.cpp

UdjinM6 · 2024-07-07T20:17:52Z

src/httprpc.cpp

+    if (!jreq.params.isArray() || jreq.params.empty()) return false;
+    if (!jreq.params[0].isStr()) return false;
+
+    return g_rpc_whitelist[jreq.authUser].count(jreq.strMethod + "_" + jreq.params[0].get_str());


How about recognizing squashed versions of composite commands instead i.e. 37e6d7b ?

quorumselectquorum will look strange.

Also there's a corner case with masternodelist and mastnode list - both of them are available now, but I guess that's only one case for now.

quorumselectquorum will look strange.

Yeah, and this subcommand should probably be simply select in the first place.

Also there's a corner case with masternodelist and mastnode list - both of them are available now, but I guess that's only one case for now.

They are essentially the same, you could drop masternodelist after 37e6d7b.

autocomplete should not provide both version IMO - happens with 37e6d7b

Also they appear also in list of command for help:

How about 26ef7c9 ?

How about 26ef7c9 ?

Check my version, I just pushed it (waiting CI)

…ists over Related changes for better support of whitelist for composite commands are in dashpay#6100

UdjinM6 · 2024-07-10T20:28:47Z

We should split this PR in 2 separate ones imo:

support for composite commands in -rpcwhitelist
introducing quorum platformsign

UdjinM6

utACK 2ac4c0e

github-actions · 2024-07-11T16:40:51Z

This pull request has conflicts, please rebase.

knst · 2024-07-11T17:23:56Z

@knst knst force-pushed the feat-whitelist-composite branch from 2ac4c0e to 85abbb9

resolved conflicts

UdjinM6

LGTM, utACK 85abbb9

PastaPastaPasta

utACK 85abbb9

2db69d7 chore: add release notes for "quorum platformsign" (Konstantin Akimov) 283c5f8 feat: create new composite command "quorum platformsign" (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented It splits from #6100 With just whitelist it is impossible to limit the RPC `quorum sign` to use only one specific quorum type, this PR aim to provide ability for quorum signing for platform quorum only. ## What was done? Implemented a new composite command "quorum platformsign" This composite command let to limit quorum type for signing for case of whitelist. After that old way to limit platform commands can be deprecated - #6105 ## How Has This Been Tested? Updated a functional tests to use platform signing for Asset Unlocks feature. ## 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 2db69d7 PastaPastaPasta: utACK 2db69d7 Tree-SHA512: b0dff9934137c4faa85664058e1e77f85067cc8d931e6d76ee5b9e610164ac8b0609736d5f09475256cb78d65bf92466624d784f0b13d20136df7e75613662cb

…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

…msign 2db69d7 chore: add release notes for "quorum platformsign" (Konstantin Akimov) 283c5f8 feat: create new composite command "quorum platformsign" (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented It splits from dashpay#6100 With just whitelist it is impossible to limit the RPC `quorum sign` to use only one specific quorum type, this PR aim to provide ability for quorum signing for platform quorum only. ## What was done? Implemented a new composite command "quorum platformsign" This composite command let to limit quorum type for signing for case of whitelist. After that old way to limit platform commands can be deprecated - dashpay#6105 ## How Has This Been Tested? Updated a functional tests to use platform signing for Asset Unlocks feature. ## 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 2db69d7 PastaPastaPasta: utACK 2db69d7 Tree-SHA512: b0dff9934137c4faa85664058e1e77f85067cc8d931e6d76ee5b9e610164ac8b0609736d5f09475256cb78d65bf92466624d784f0b13d20136df7e75613662cb

db82817 Merge #6106: feat: create new composite quorum-command platformsign (pasta) a45e6df Merge #6104: fix: adjust incorrect parameter description that says there is a default that doesn't exist (pasta) 7330982 Merge #6100: feat: make whitelist works with composite commands for platform needs (pasta) 9998ffd Merge #6096: feat: split type of error in submitchainlock - return enum in CL verifying code (pasta) cdf7a25 Merge #6095: fix: createwallet to require 'load_on_startup' for descriptor wallets (pasta) c1c2c55 Merge #6092: fix: mixing for partially unlocked descriptor wallets (pasta) 1175486 Merge #6073: feat: add logging for RPC HTTP requests: command, user, http-code, time of running (pasta) Pull request description: ## Issue being fixed or feature implemented Backports a set of 6 PRs needed in rc.2 ## What was done? Backported PRs with labels ## How Has This Been Tested? ## Breaking Changes ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] 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 _(for repository code-owners and collaborators only)_ ACKs for top commit: kwvg: LGTM, utACK db82817 UdjinM6: utACK db82817 Tree-SHA512: 1b242c5db04bd5873ef622543bc2a25e29567f15962c677ea51ff05cb784291968d18f419bf611c206b912e8f15d687208ae75af33aab89038b6f0167d99c4bf

…ists over Related changes for better support of whitelist for composite commands are in dashpay#6100

PastaPastaPasta · 2024-07-23T17:45:00Z

back ported in #6115

98a3393 chore: set release to true (pasta) cd0a3a6 Merge #6154: chore: remove trailing whitespaces in release notes (pasta) 6bc60a7 Merge #6151: chore: update seeds for v21 release (pasta) 88e949a Merge #6146: chore: bump assumevalid, minchainwork, checkpoints, chaintxdata (pasta) cc14427 Merge #6144: docs: release notes for v21.0.0 (pasta) 0a8ece1 Merge #6122: chore: translations 2024-07 (pasta) 146d244 Merge #6140: feat: harden all sporks on mainnet to current values (pasta) 024d272 Merge #6126: feat: enable EHF activation of MN_RR on mainnet (pasta) e780b3d Merge #6125: docs: update manpages for 21.0 (pasta) 5ede23c Merge #6118: docs: add release notes notifying change of default branch to `develop` (pasta) 1b6fe9c Merge #6117: docs: update supported versions in SECURITY.md (pasta) 27d20be Merge #6116: fix: mitigate crashes associated with some upgradetohd edge cases (pasta) db82817 Merge #6106: feat: create new composite quorum-command platformsign (pasta) a45e6df Merge #6104: fix: adjust incorrect parameter description that says there is a default that doesn't exist (pasta) 7330982 Merge #6100: feat: make whitelist works with composite commands for platform needs (pasta) 9998ffd Merge #6096: feat: split type of error in submitchainlock - return enum in CL verifying code (pasta) cdf7a25 Merge #6095: fix: createwallet to require 'load_on_startup' for descriptor wallets (pasta) c1c2c55 Merge #6092: fix: mixing for partially unlocked descriptor wallets (pasta) 1175486 Merge #6073: feat: add logging for RPC HTTP requests: command, user, http-code, time of running (pasta) Pull request description: ## Issue being fixed or feature implemented Suppressed changes from be83865 so the diff is empty. ## What was done? ## How Has This Been Tested? ## Breaking Changes ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] 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 _(for repository code-owners and collaborators only)_ ACKs for top commit: PastaPastaPasta: ACK 158cf86; no diff knst: ACK 158cf86 Tree-SHA512: 3310a39fbcb45bdf09f885fe77ba769c0a715869a3bb287eaf0f2cf54b35a7e1f832c88df3bd31097eabf2d375515c1b87ff05e0c3282cef642833a154c42bbe

knst added this to the 21 milestone Jul 7, 2024

PastaPastaPasta added the backport-candidate-21.0.x label Jul 7, 2024

UdjinM6 reviewed Jul 8, 2024

View reviewed changes

knst force-pushed the feat-whitelist-composite branch 2 times, most recently from 82d9c6a to fb6213b Compare July 9, 2024 10:40

knst requested review from UdjinM6 and PastaPastaPasta July 9, 2024 14:56

knst added a commit to knst/dash that referenced this pull request Jul 9, 2024

feat: drop hard-coded restrictions for platform-user to prefer whitel…

765294c

…ists over Related changes for better support of whitelist for composite commands are in dashpay#6100

knst mentioned this pull request Jul 9, 2024

feat: drop hard-coded restrictions for platform-user #6105

Closed

5 tasks

knst force-pushed the feat-whitelist-composite branch from fb6213b to 2ac4c0e Compare July 11, 2024 05:21

knst mentioned this pull request Jul 11, 2024

feat: create new composite quorum-command platformsign #6106

Merged

5 tasks

UdjinM6 previously approved these changes Jul 11, 2024

View reviewed changes

github-actions bot added the needs rebase label Jul 11, 2024

knst added 3 commits July 12, 2024 00:07

feat: add support of composite commands in RPC'c whitelists

a102a59

feat: test composite commands in functional test for whitelist

78ad778

chore: add release notes for composite command for whitelist

85abbb9

knst dismissed UdjinM6’s stale review via 85abbb9 July 11, 2024 17:23

knst force-pushed the feat-whitelist-composite branch from 2ac4c0e to 85abbb9 Compare July 11, 2024 17:23

github-actions bot removed the needs rebase label Jul 11, 2024

knst requested a review from UdjinM6 July 11, 2024 17:23

UdjinM6 approved these changes Jul 11, 2024

View reviewed changes

PastaPastaPasta approved these changes Jul 15, 2024

View reviewed changes

PastaPastaPasta merged commit ebd1d05 into dashpay:develop Jul 15, 2024
9 checks passed

knst added a commit to knst/dash that referenced this pull request Jul 19, 2024

feat: drop hard-coded restrictions for platform-user to prefer whitel…

fcf143e

…ists over Related changes for better support of whitelist for composite commands are in dashpay#6100

PastaPastaPasta removed the backport-candidate-21.0.x label Jul 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: make whitelist works with composite commands for platform needs #6100

feat: make whitelist works with composite commands for platform needs #6100

knst commented Jul 7, 2024 •

edited

Loading

UdjinM6 Jul 7, 2024

knst Jul 8, 2024

UdjinM6 Jul 8, 2024

knst Jul 9, 2024

UdjinM6 Jul 9, 2024

knst Jul 9, 2024

UdjinM6 commented Jul 10, 2024

UdjinM6 left a comment

github-actions bot commented Jul 11, 2024

knst commented Jul 11, 2024

UdjinM6 left a comment

PastaPastaPasta left a comment

PastaPastaPasta commented Jul 23, 2024

feat: make whitelist works with composite commands for platform needs #6100

feat: make whitelist works with composite commands for platform needs #6100

Conversation

knst commented Jul 7, 2024 • edited Loading

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

UdjinM6 Jul 7, 2024

Choose a reason for hiding this comment

knst Jul 8, 2024

Choose a reason for hiding this comment

UdjinM6 Jul 8, 2024

Choose a reason for hiding this comment

knst Jul 9, 2024

Choose a reason for hiding this comment

UdjinM6 Jul 9, 2024

Choose a reason for hiding this comment

knst Jul 9, 2024

Choose a reason for hiding this comment

UdjinM6 commented Jul 10, 2024

UdjinM6 left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 11, 2024

knst commented Jul 11, 2024

UdjinM6 left a comment

Choose a reason for hiding this comment

PastaPastaPasta left a comment

Choose a reason for hiding this comment

PastaPastaPasta commented Jul 23, 2024

knst commented Jul 7, 2024 •

edited

Loading