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

Launchpad: Make distKeeper.calculateDelegationRewards public #7466

Closed
4 tasks
ethanfrey opened this issue Oct 6, 2020 · 7 comments
Closed
4 tasks

Launchpad: Make distKeeper.calculateDelegationRewards public #7466

ethanfrey opened this issue Oct 6, 2020 · 7 comments

Comments

@ethanfrey
Copy link
Contributor

ethanfrey commented Oct 6, 2020

Summary

I am trying to query the pending delegator rewards (this will be a very common request when people build staking derivates or other such instruments). But there is no public method to find this.

distKeeper.calculateDelegationRewards provides the info, but cannot be called from another module.

Problem Definition

It turns out the distribution.NewQuerier() indirectly exposes this call. My current workaround is:

	// Try to get *delegator* reward info!
	params := distribution.QueryDelegationRewardsParams{
		DelegatorAddress: contractAddr,
		ValidatorAddress: valAddr,
	}
	req := abci.RequestQuery{
		Data: mustMarshal(t, params),
	}
	qres, err := distribution.NewQuerier(distKeeper)(ctx, []string{distribution.QueryDelegationRewards}, req)
	require.NoError(t, err)
	var rewards sdk.DecCoins
	mustParse(t, qres, &rewards)
	fmt.Printf("delegator rewards: %#v\n", rewards)

It works, but I feel it is a bit ugly

Proposal

Make the needed functions public, so I can write this easily.

Actually, looking deeper, the above may have side effects... in the query logic I see:

	endingPeriod := k.incrementValidatorPeriod(ctx, val)
	rewards := k.calculateDelegationRewards(ctx, val, del, endingPeriod)

incrementValidatorPeriod??

It would be great to have k.GetValidatorPeriod() and then use period +1 or such. That is actually exposed via k.GetValidatorCurrentRewards().Period, but maybe we could make this nicer....

My final proposal would be to add a new function to distribution:

func (k Keeper)CalculateDelegationRewards(ctx sdk.Context, val sdk.ValAddress, del sdk.AccAddress) rewards sdk.DecCoins {
    endingPeriod := k.GetValidatorCurrentRewards(ctx, val).Period
    return k.calculateDelegationRewards(ctx, val, del, endingPeriod+1)
}

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alessio
Copy link
Contributor

alessio commented Oct 7, 2020

@ethanfrey there is a CalculateDelegationRewards in master:

func (k Keeper) CalculateDelegationRewards(ctx sdk.Context, val exported.ValidatorI, del exported.DelegationI, endingPeriod uint64) (rewards sdk.DecCoins) {

@ethanfrey
Copy link
Contributor Author

Great, so making this public should be a backport.

@clevinson
Copy link
Contributor

@ethanfrey can we PR into master first and then backport to Launchpad? I think that is the general process we want to follow, right?

@amaury1093
Copy link
Contributor

amaury1093 commented Oct 9, 2020

The PR #5725 is already in master, so only a backport to Launchpad is needed.

+1 for the process you described.

@ethanfrey
Copy link
Contributor Author

Just pull in parts of that PR #5725, right? Not all of it.

https://github.com/cosmos/cosmos-sdk/pull/5725/files#diff-de71192adba700b3255b1eab62bef50e in particular

@clevinson
Copy link
Contributor

@ethanfrey is this still a desired feature for your team to have this back ported to Launchpad?

@ethanfrey
Copy link
Contributor Author

@clevinson Nope, we are no longer developing on launchpad and have used the stargate improvement already. Thanks.

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

No branches or pull requests

5 participants