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

New API endpoint to expose total stake. #1836

Merged
merged 1 commit into from
Sep 4, 2020
Merged

Conversation

kantp
Copy link
Contributor

@kantp kantp commented Sep 4, 2020

This will allow the wallet to display saturation as an additional information to
delegators, as requested in IntersectMBO/cardano-node#1659

This will allow Daedalus to display saturation as an additional information to
delegators.
Copy link
Contributor

@nc6 nc6 left a comment

Choose a reason for hiding this comment

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

Code looks good, but a reference for why "total" stake is defined as it is would be nice!

@@ -52,6 +54,14 @@ import Shelley.Spec.Ledger.Rewards
import Shelley.Spec.Ledger.STS.Tickn (TicknState (..))
import Shelley.Spec.Ledger.TxData (PoolParams (..), TxOut (..))
import Shelley.Spec.Ledger.UTxO (UTxO (..))
import qualified Shelley.Spec.Ledger.Val as Val

-- | Calculate the current total stake.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it might be useful to reference the spot where this is defined as "total". E.g. it's not obvious why reserves should be excluded, but treasury not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! The reason I'm using this here is that that's also what we're using in the reward calculation, in createRUpd. So yes, either a comment, or maybe it's even better to have a single function calculate the total stake, and use that everywhere (this endpoint, reward calculation, and the non myopic rewards api endpoint).

Copy link
Contributor

Choose a reason for hiding this comment

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

It's called "circulation" in the createRUpd function, perhaps that's a name we can standardise on

@nc6
Copy link
Contributor

nc6 commented Sep 4, 2020

I've added #1838 to capture the requirement to extract this,

@nc6 nc6 merged commit bc6aefd into master Sep 4, 2020
@iohk-bors iohk-bors bot deleted the kantp/total-stake-endpoint branch September 4, 2020 11:17
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.

2 participants