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.

Sign up for GitHub

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

Jump to bottom

Get stake pool data using GetRewardInfoPools query #3195

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Get stake pool data using GetRewardInfoPools query #3195

wants to merge 2 commits into from

Conversation

HeinrichApfelmus
Copy link
Contributor

Issue number

ADP-1361, ADP-1543

Overview

In this pull request, we use the new GetRewardInfoPools query to retrieve stake pool data that informs a delegation choice. This data includes:

  • current pool stake distribution
  • current pool owner stake, for testing whether the pool owner meets their pledge
  • pool performance estimates

Details

  • This pull request does not yet implement caching.

Comments

@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-1491/rewards branch from a34b604 to 0811b14 Compare March 22, 2022 18:36
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-1491/local-state-query branch from bdda343 to 03ea8a5 Compare March 22, 2022 18:37
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-1491/rewards branch from 0811b14 to 92adb49 Compare March 22, 2022 18:38
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-1491/local-state-query branch from 03ea8a5 to 4614b3d Compare March 22, 2022 18:38
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-1491/rewards branch from 92adb49 to 83bd65c Compare March 22, 2022 20:34
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-1491/local-state-query branch 3 times, most recently from a6f04d7 to 1283ffc Compare March 23, 2022 14:33
@piotr-iohk
Copy link
Contributor

On mainnet, epoch 328.

Test: [GET] /v2/stake-pools?stake=1000000000

  • this branch v2022-01-18 (git revision: a6f04d71566f8871157fa14d3138f38fef5c7b2e), cardano-node 1.34.1
    • time: 22m 55s
    • logs:
[cardano-wallet.network:Notice:156] [2022-03-23 14:01:56.95 UTC] Query stakePoolsSummary took 1341.704617647s (too slow)
[cardano-wallet.network:Info:156] [2022-03-23 14:01:56.95 UTC] Fetched pool data from node tip using LSQ. Got Coin 34239675266895397 pools in the stake distribution, and 3199 pools in the reward .
[cardano-wallet.network:Notice:19] [2022-03-23 14:01:56.97 UTC] Reward observer: New values: {3e250d8d: 455.872117}
[cardano-wallet.api-server:Error:156] [2022-03-23 14:02:01.20 UTC] [RequestId 1] GET /v2/stake-pools 200 OK in 1375.412647801s
  • latest release v2022-01-18 (git revision: a5085acbd2670c24251cf8d76a4e83c77a2679ba), cardano-node 1.33.0
    • time: 4m 35s
    • logs:
[cardano-wallet.network:Notice:1633] [2022-03-23 15:11:03.62 UTC] Query stakePoolsSummary took 275.912293235s (too slow)
[cardano-wallet.network:Info:1633] [2022-03-23 15:11:03.62 UTC] Fetched pool data from node tip using LSQ. Got 3140 pools in the stake distribution, and 3199 pools in the non-myopic member reward map.
[cardano-wallet.api-server:Error:1633] [2022-03-23 15:11:08.29 UTC] [RequestId 1] GET /v2/stake-pools 200 OK in 288.961412018s

rvl
rvl approved these changes Mar 25, 2022
View reviewed changes
Copy link
Contributor

@rvl rvl left a comment

Choose a reason for hiding this comment

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

Looks good

lib/shelley/src/Cardano/Wallet/Shelley/Pools.hs Outdated Show resolved Hide resolved
lib/shelley/src/Cardano/Wallet/Shelley/Pools.hs
rewards = view (#metrics . #nonMyopicMemberRewards) . stakePool
(randomWeight, stakePool) = (fst, snd)
rewards :: Api.ApiStakePool -> Quantity "lovelace" Natural
rewards = view (#metrics . #nonMyopicMemberRewards)
Copy link
Contributor

Choose a reason for hiding this comment

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

OK so the new ranking formula is implemented but not yet enabled 👍

lib/shelley/src/Cardano/Wallet/Shelley/Pools.hs Outdated Show resolved Hide resolved
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-1491/rewards branch from 8e8596a to 992d3af Compare March 25, 2022 12:41
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-1491/local-state-query branch 2 times, most recently from 43c94e7 to 7f35885 Compare March 25, 2022 13:04
@HeinrichApfelmus
Copy link
Contributor Author

Thanks for the review, Rodney! 😊

Unfortunately, as the query (20mins) still takes takes much longer than the previous version (5mins), I'm going to put this pull request back in draft mode, to be tested again with a newer node version.

@HeinrichApfelmus HeinrichApfelmus marked this pull request as draft March 25, 2022 13:06
Base automatically changed from HeinrichApfelmus/ADP-1491/rewards to master March 25, 2022 18:15
HeinrichApfelmus and others added 2 commits March 28, 2022 12:12
@HeinrichApfelmus
Use the new GetRewardInfoPools
b959742 
… and drop the other local state queries related to rewards.

This depends on `cardano-node` version 1.33 or later.
@HeinrichApfelmus @rvl
Apply suggestions from code review
be7fa61 
Co-authored-by: Rodney Lorrimar <[email protected]>
@HeinrichApfelmus HeinrichApfelmus force-pushed the HeinrichApfelmus/ADP-1491/local-state-query branch from 7f35885 to be7fa61 Compare March 28, 2022 10:12
@HeinrichApfelmus HeinrichApfelmus self-assigned this Aug 24, 2022
@HeinrichApfelmus HeinrichApfelmus mentioned this pull request Jan 25, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rvl rvl rvl approved these changes

At least 1 approving review is required to merge this pull request.

Assignees

@HeinrichApfelmus HeinrichApfelmus

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@HeinrichApfelmus @piotr-iohk @rvl