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

[FEATURE] - extend query stake-pool to query for multiple pools #4732

Closed
1 of 17 tasks
newhoggy opened this issue Dec 22, 2022 · 6 comments
Closed
1 of 17 tasks

[FEATURE] - extend query stake-pool to query for multiple pools #4732

newhoggy opened this issue Dec 22, 2022 · 6 comments
Labels
status: duplicate The PR or issue already exists

Comments

@newhoggy
Copy link
Contributor

What

Add the ability to query for multiple stake pools, or all stake pools in the query stake-pool command. Previously it was only possible to query for one specific stake pool:

$ cardano-cli query stake-snapshot --cardano-mode --mainnet --stake-pool-id fc79a939d5986c6565fc5e21bb5dd292261ac5da564c30385b3bb8d4
{
    "activeStakeGo": 24453012636354912,
    "activeStakeMark": 24525857119151547,
    "activeStakeSet": 24463445965412728,
    "poolStakeGo": 8595693051,
    "poolStakeMark": 8595693051,
    "poolStakeSet": 8595693051
 }

The change introduces a pools object to accomodate more than one stake pool in the result like this and thus is a breaking change:

$ cardano-cli query stake-snapshot --cardano-mode --mainnet --stake-pool-id fc79a939d5986c6565fc5e21bb5dd292261ac5da564c30385b3bb8d4
{
    "activeStakeGo": 24453012636354912,
    "activeStakeMark": 24525857119151547,
    "activeStakeSet": 24463445965412728,
    "pools": {
        "fc79a939d5986c6565fc5e21bb5dd292261ac5da564c30385b3bb8d4": {
            "poolStakeGo": 8595693051,
            "poolStakeMark": 8595693051,
            "poolStakeSet": 8595693051
        }
    }
}

Then querying for multiple stack pools can be done by adding more --stake-pool-id switches, for example:

$ cardano-cli query stake-snapshot --cardano-mode --mainnet --stake-pool-id fc79a939d5986c6565fc5e21bb5dd292261ac5da564c30385b3bb8d4 --stake-pool-id 0dedea6dd97bfd6b0d3ddf8e42375266ade6ec6a9c1aceb8efeecdab
{
    "activeStakeGo": 24453012636354912,
    "activeStakeMark": 24525857119151547,
    "activeStakeSet": 24463445965412728,
    "pools": {
        "0dedea6dd97bfd6b0d3ddf8e42375266ade6ec6a9c1aceb8efeecdab": {
            "poolStakeGo": 547234244,
            "poolStakeMark": 547234244,
            "poolStakeSet": 547234244
        },
        "fc79a939d5986c6565fc5e21bb5dd292261ac5da564c30385b3bb8d4": {
            "poolStakeGo": 8595693051,
            "poolStakeMark": 8595693051,
            "poolStakeSet": 8595693051
        }
    }
}

It will also be possible to query for all stake pools by omitting --stake-pool-id altogether:

$ cardano-cli query stake-snapshot --cardano-mode --mainnet
{
    "activeStakeGo": 24453012636354912,
    "activeStakeMark": 24525857119151547,
    "activeStakeSet": 24463445965412728,
    "pools": {
        "0dedea6dd97bfd6b0d3ddf8e42375266ade6ec6a9c1aceb8efeecdab": {
            "poolStakeGo": 547234244,
            "poolStakeMark": 547234244,
            "poolStakeSet": 547234244
        },
        ... ... ...
        "fc79a939d5986c6565fc5e21bb5dd292261ac5da564c30385b3bb8d4": {
            "poolStakeGo": 8595693051,
            "poolStakeMark": 8595693051,
            "poolStakeSet": 8595693051
        }
    }
}

Why

There is overhead in making each call for this sort of query so it would be useful for the user to be able to query for each of the stake pools they are interested in with a single query.

Personas

This will affect

  • SPOs
  • dApp Devs
  • Exchanges
  • Wallets
  • 3rd party tools
  • ADA holders

Definition of Done (DoD)

  • Acceptance Criteria + User Stories & DoD created and singed-off (by PO, dev & test owners)
  • Builds successfully on CI
  • Code & Test review (as per Acceptance Criteria)
  • There is documentation and/or examples for the new functionality (usage/response)
  • Log/record changes on Vnext (or similar depending on what we adopt)
  • Ticket number(s) included in PR description
  • All Acceptance Criteria met and covered by dev/unit/property/integration tests
  • System/E2E automated tests + System Test Engineer Owner Sign-off

NOTE: Ideally, we should merge only fully implemented and tested features into the master branch.
So all the above steps are required for the PR to be merged.
In order to avoid the PRs becoming stale and requiring to be rebased on master, these can be merged
after a reasonable time (current agreement is 3 days) if the System Test Engineer Owner's sign-off
was not provided (last step in the DoD).

IMPORTANT: Any deviation from the plan should be discussed and agreed as a comment in the Feature file.

Sign-off

  • Product Owner
  • Dev Owner
  • System Test Engineer Owner

Related PRs

  1. PR # here

Acceptance Criteria

Acceptance Criteria & User Stories define here (or in a separate file (linked here) for a big feature)

Example - #4453

@newhoggy newhoggy added the user type: internal Created by an IOG employee label Dec 22, 2022
@dorin100 dorin100 changed the title [FEATURE] - [FEATURE] - query stake-pool new CLI command Dec 22, 2022
@dorin100 dorin100 changed the title [FEATURE] - query stake-pool new CLI command [FEATURE] - extend query stake-pool to query for multiple pools Dec 22, 2022
@dorin100
Copy link

$ cardano-cli query stake-snapshot --cardano-mode --mainnet --stake-pool-id

Is the --cardano-mode parameter mandatory?

@dorin100
Copy link

The addition of the pool objects breaks the backward compatibility, right? If yes, we need to highlight this in the Feature file (here).

@dorin100
Copy link

Do we have any performance numbers for this CLI command (before and after this change)? Like how much time does it takes for the command to be executer before this change and after the change (for 1, 2, all pools on Mainnet)? Same for the RAM usage.

@dorin100
Copy link

@CarlosLopezDeLara can you please help us with defining the Acceptance Criteria for this feature (similar with #4453)?

@dorin100
Copy link

fyi @saratomaz , @mkoura

@dorin100 dorin100 added type: feature request Request a new functionality comp: cardano-cli labels Dec 22, 2022
@dorin100
Copy link

duplicate on #4570

@dorin100 dorin100 added status: duplicate The PR or issue already exists and removed type: feature request Request a new functionality user type: internal Created by an IOG employee comp: cardano-cli labels Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: duplicate The PR or issue already exists
Projects
None yet
Development

No branches or pull requests

2 participants