Skip to content
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

refactor: use new type of composite commands for quorum NNN #6055

Merged
merged 4 commits into from
Jun 13, 2024

Conversation

knst
Copy link
Collaborator

@knst knst commented Jun 12, 2024

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:

  • 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

@knst knst added this to the 21 milestone Jun 12, 2024
@knst knst requested review from UdjinM6 and PastaPastaPasta June 12, 2024 09:32
@knst knst force-pushed the refactor-rpc-18531-p3 branch from 7262def to b478406 Compare June 12, 2024 14:03
Copy link

@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 b478406

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK b478406

@PastaPastaPasta PastaPastaPasta merged commit 692a076 into dashpay:develop Jun 13, 2024
7 of 8 checks passed
@knst knst deleted the refactor-rpc-18531-p3 branch June 14, 2024 08:13
PastaPastaPasta added a commit that referenced this pull request Jul 15, 2024
…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
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jul 15, 2024
…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
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.

3 participants