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

Use and combine chain data with LSQ data for stake pools #1769

Merged
merged 9 commits into from
Jun 23, 2020
Merged

Use and combine chain data with LSQ data for stake pools #1769

merged 9 commits into from
Jun 23, 2020

Conversation

Anviking
Copy link
Member

@Anviking Anviking commented Jun 16, 2020 •
edited
Loading

Issue Number

ADP-311, #1719

Overview

  • Add Shelley MsgFetchedNodePoolLsqData trace
  • Use and combine chain data with LSQ when listing pools
  • Update swagger to make pool cost and margin nullable
  • Make API types nullable
  • Add model implementation for readPoolMetadata

Comments

  • We should test that we order by non-myopic rewards, but that would require the pools producing blocks.
  • Maybe we should have a DB test checking that readMetadata returns what we previously put in the DB.

@Anviking Anviking self-assigned this Jun 16, 2020
@Anviking Anviking changed the title Use and combine chain data with LSQ data Use and combine chain data with LSQ data for stake pools Jun 16, 2020
@Anviking Anviking added this to the (ADP-311) List stake pools milestone Jun 16, 2020
@KtorZ KtorZ force-pushed the KtorZ/1761/extract-pool-registrations branch from 5c84984 to ffe1b45 Compare June 16, 2020 16:15
@Anviking Anviking force-pushed the anviking/ADP-311/use-chain-data branch from 57dbaf0 to f4cf074 Compare June 17, 2020 09:22
@KtorZ KtorZ force-pushed the KtorZ/1761/extract-pool-registrations branch from ffe1b45 to e8e1d83 Compare June 17, 2020 10:18
@Anviking Anviking force-pushed the anviking/ADP-311/use-chain-data branch 2 times, most recently from 1d36de6 to 24c0b70 Compare June 17, 2020 17:07
@KtorZ KtorZ force-pushed the KtorZ/1761/extract-pool-registrations branch 2 times, most recently from 2b6ea47 to d0386e5 Compare June 17, 2020 22:05
@Anviking Anviking force-pushed the KtorZ/1761/extract-pool-registrations branch from d0386e5 to 930a3b3 Compare June 18, 2020 08:54
@KtorZ KtorZ force-pushed the KtorZ/1761/extract-pool-registrations branch from 930a3b3 to a52ed79 Compare June 18, 2020 09:39
@Anviking Anviking force-pushed the anviking/ADP-311/use-chain-data branch 2 times, most recently from dbddbb8 to f076ad3 Compare June 18, 2020 13:35
@Anviking Anviking changed the base branch from KtorZ/1761/extract-pool-registrations to KtorZ/1762/fetch-metadata June 18, 2020 13:35
@Anviking Anviking force-pushed the anviking/ADP-311/use-chain-data branch from f076ad3 to a6bcca7 Compare June 18, 2020 13:39
@KtorZ KtorZ force-pushed the KtorZ/1762/fetch-metadata branch 3 times, most recently from 9f102c7 to bf962fb Compare June 18, 2020 16:53
@Anviking Anviking force-pushed the anviking/ADP-311/use-chain-data branch from a6bcca7 to af8d6f3 Compare June 18, 2020 17:07
@KtorZ KtorZ force-pushed the KtorZ/1762/fetch-metadata branch 2 times, most recently from 2a37581 to a58a193 Compare June 18, 2020 17:51
@Anviking Anviking force-pushed the anviking/ADP-311/use-chain-data branch from 4abf479 to da24894 Compare June 18, 2020 17:51
@KtorZ KtorZ force-pushed the KtorZ/1762/fetch-metadata branch from a58a193 to c18da60 Compare June 18, 2020 18:11
@Anviking Anviking force-pushed the anviking/ADP-311/use-chain-data branch 3 times, most recently from 7c94f4f to afb32af Compare June 18, 2020 18:58
@KtorZ KtorZ added the ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG label Jun 18, 2020
iohk-bors bot added a commit that referenced this pull request Jun 23, 2020
@iohk-bors
Try #1769:
b07b040
@Anviking Anviking requested a review from KtorZ June 23, 2020 09:50
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 23, 2020

try

Build failed

@Anviking Anviking force-pushed the anviking/ADP-311/use-chain-data branch 3 times, most recently from a866694 to c74ffb7 Compare June 23, 2020 11:13
@Anviking
Copy link
Member Author

bors try

iohk-bors bot added a commit that referenced this pull request Jun 23, 2020
@iohk-bors
Try #1769:
f7ff737
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 23, 2020 •
edited by Anviking
Loading

try

Build failed

error: unable to fork: Cannot allocate memory

@Anviking
Copy link
Member Author

bors try

iohk-bors bot added a commit that referenced this pull request Jun 23, 2020
@iohk-bors
Try #1769:
b78af91
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 23, 2020

try

Build failed

KtorZ
KtorZ reviewed Jun 23, 2020
View reviewed changes
lib/core-integration/src/Test/Integration/Scenario/API/Shelley/StakePools.hs Outdated
eventually "Listing stake pools shows expected information" $ do
r <- request @[ApiStakePool] ctx
(Link.listStakePools arbitraryStake) Default Empty
let fixTypeInference = True `shouldBe` True
Copy link
Member

Choose a reason for hiding this comment

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

You just need a @IO down below on request

KtorZ
KtorZ reviewed Jun 23, 2020
View reviewed changes
lib/shelley/test/data/cardano-node-shelley/genesis.yaml Outdated
@@ -6,7 +6,7 @@ protocolParams:
protocolVersion:
minor: 0
major: 0
decentralisationParam: 0.97 # means 3% decentralised
decentralisationParam: 0.1 # means 90% decentralised
Copy link
Member

Choose a reason for hiding this comment

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

This was very high to avoid too frequent rollbacks until we have a proper transaction scheduler. I don't think you'll get anything merged with 90% decentralization :)

Copy link
Member

Choose a reason for hiding this comment

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

We can try, but I am afraid this is going to block many PRs after that :/

Anviking and others added 8 commits June 23, 2020 19:33
@Anviking @KtorZ
Add Shelley MsgFetchedNodePoolLsqData trace
3cd052e
@Anviking @KtorZ
Allow pool cost and margin to be ommitted in swagger.yaml
d89814d 
If their pool registration hasn't been found yet the fields will be left out from the response.
@Anviking @KtorZ
Add readPoolMetadata DB function
791ce5d
@Anviking @KtorZ
Read and combine DB data with LSQ when listing pools
0074cff 
We already have the workers to pool registration certificates and
metadata in the data in the DB. Here we read it.

- Ignore blocks produced by BFT nodes

- Relocate functions and add documentation

- Use strategy `chainButNoLsq = dropMissing`

- Check that pools produce blocks

- fixup: combineDBAndLsqData -> combineDbAndLsqData

- fixup: rewrite fetching of certMap
@Anviking @KtorZ
Add MsgWillOpenDB log message
ee2a659
@Anviking @KtorZ
Log static metadata server urls when launched
736d523
@KtorZ
use type applications instead of 'fixTypeInference' function
12f7fc1
@KtorZ
revert changing decentralizationParam to 0.1
e543f19 
We need to keep this one quite high at the moment in order to prevent the cluster from rolling back too often. The wallet can handle rollback quite nicely, but the integration scenarios cannot (most of them). A lot of scenarios are built following a basic fixture -> action -> assertions principle, and rolling back the fixture can have very negative effects on the rest. We could run every scenario multiple times (at least twice) to maybe cope with intermediate failures due to rollback...
@KtorZ KtorZ force-pushed the anviking/ADP-311/use-chain-data branch from c74ffb7 to e543f19 Compare June 23, 2020 17:33
@KtorZ
split assertions on block production and saturation out in a separate…
d88fcf0 
… scenario

So that we don't mix concerns between scenarios and we can lower down a bit the assertions without loosing much of the testing benefits.
KtorZ
KtorZ approved these changes Jun 23, 2020
View reviewed changes
Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

reviewed, tweaked a few things 👍

@KtorZ
Copy link
Member

KtorZ commented Jun 23, 2020

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jun 23, 2020

Build succeeded

@iohk-bors iohk-bors bot merged commit 4dc50fc into master Jun 23, 2020
@iohk-bors iohk-bors bot deleted the anviking/ADP-311/use-chain-data branch June 23, 2020 18:53
@Anviking Anviking mentioned this pull request Jul 1, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rvl rvl rvl left review comments

@KtorZ KtorZ KtorZ approved these changes

Assignees

@Anviking Anviking

Labels
ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG
Projects
None yet
Milestone
(ADP-311) List stake pools
Development

Successfully merging this pull request may close these issues.
3 participants
@Anviking @KtorZ @rvl