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

funding: issue delegation token rewards #5046

Closed
Tracked by #5010
erwanor opened this issue Feb 3, 2025 · 7 comments
Closed
Tracked by #5010

funding: issue delegation token rewards #5046

erwanor opened this issue Feb 3, 2025 · 7 comments
Assignees

Comments

@erwanor
Copy link
Member

erwanor commented Feb 3, 2025

To do this, we can:

  1. Extend the ranked voter state key to track the IdentityKey that corresponds to the delegator's voting notes (no group element decoding overhead since the underlying struct is bytes)
  2. Include it the stream of votes
  3. Fetch the current validator rate for the correspond IK
  4. Apply it to the unbonded amount
  5. reward_to_voter takes a Value where the amount is rate * unbonded_amount and the asset_id is DelegationToken::new(IK).id()

We don't need to adjust the rates at all, since they are independent issuance events. The rate are determined by the chain native issuance rate schedule.

@erwanor erwanor mentioned this issue Feb 3, 2025
28 tasks
@erwanor erwanor reopened this Feb 4, 2025
@erwanor erwanor self-assigned this Feb 5, 2025
@hdevalence
Copy link
Member

How does the staking component learn about the newly issued delegation tokens?

@erwanor
Copy link
Member Author

erwanor commented Feb 5, 2025

We are adding a new inter-component API to increase the validator delegation pool, and outsource it the conversion of unbonded rewards to a Value which is the delegation token equivalent.

This is done after the validator powers are ratified for the epoch, and after the cometbft update is built. So those changes take place at the next epoch boundary. Prototyping a PR.

@hdevalence
Copy link
Member

Hmm, shouldn't the newly minted delegator rewards happen before the end of the epoch in which they are issued, so that they are reflected in the validator voting power and available for voting / staking etc in the following epoch?

@erwanor
Copy link
Member Author

erwanor commented Feb 5, 2025

They are available immediately to the user, but the downstream effects of increased VP are only taking effect in the following epoch.

We are at epoch e, on the last block h:

  • staking: processed delegations/undelegations, tallied VP, adjusted rates
  • funding: mint delegation tokens, increase validator delegation pool

this means that, at block h+1, when we enter a new epoch e+1:

  • voters: have increased LQT VP from their rewards (because minting happened in e)
  • validators: have bigger delegation pools but no increased VP yet (as if the user had just delegated) (because consensus view update happens at the end of e+1)

at the end of the epoch, the staking component will refresh the chain's view of the active set and actualize the VPs.

@hdevalence
Copy link
Member

Yeah, what I'm saying is shouldn't it be made so that there is not a one-epoch offset? Wouldn't that be better?

@erwanor
Copy link
Member Author

erwanor commented Feb 5, 2025

I think it's conceptually cleaner and better, which is a lot, but worse in terms of lift because of how we decided to structure the component stack.

In retrospect, LQT should have its own component that executes ahead of staking, so that pool/cometbft VP stay aligned. But when we drafted the ADR, we did so with staking token rewards in mind so doing work in funding seemed sufficient.

@erwanor
Copy link
Member Author

erwanor commented Feb 6, 2025

Discussed out of band, picking the smaller lift

erwanor added a commit that referenced this issue Feb 6, 2025
## Describe your changes

This PR adds a component API to allow `funding` to deposit into a
validator's delegation pool. It does so by exposing a
`ValidatorPoolDeposit::deposit_to_validator_pool` method to safely
handle it.

One tension in the current design, is that the LQT logic lives in the
`funding` component which execute last. As a result, delegation pool
increases do not affect the validator's voting power until the following
epoch. LQT rewards however, are immediately useful to awardees and can
be directed towards future LQT seasons.

One notable change to this PR is that we now track voters based on:
- the beneficiary address for the rewards
- the identity key of the validator corresponding to the delegation note
used for voting

**Alternative solution**
If we want to solve this another way, we could close the current PR, and
refactor the LQT logic into its own component. The approach should be
the same: the `lqt` component calls the safe pool increase handle and
the `staking` component takes care of the rest.

## Issue ticket number and link

Discussing and overview in #5046 

## Checklist before requesting a review

- [X] I have added guiding text to explain how a reviewer should test
these changes.

- [X] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:

  > LQT
@erwanor erwanor closed this as completed Feb 6, 2025
conorsch pushed a commit that referenced this issue Feb 14, 2025
## Describe your changes

This PR adds a component API to allow `funding` to deposit into a
validator's delegation pool. It does so by exposing a
`ValidatorPoolDeposit::deposit_to_validator_pool` method to safely
handle it.

One tension in the current design, is that the LQT logic lives in the
`funding` component which execute last. As a result, delegation pool
increases do not affect the validator's voting power until the following
epoch. LQT rewards however, are immediately useful to awardees and can
be directed towards future LQT seasons.

One notable change to this PR is that we now track voters based on:
- the beneficiary address for the rewards
- the identity key of the validator corresponding to the delegation note
used for voting

**Alternative solution**
If we want to solve this another way, we could close the current PR, and
refactor the LQT logic into its own component. The approach should be
the same: the `lqt` component calls the safe pool increase handle and
the `staking` component takes care of the rest.

## Issue ticket number and link

Discussing and overview in #5046 

## Checklist before requesting a review

- [X] I have added guiding text to explain how a reviewer should test
these changes.

- [X] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:

  > LQT
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

3 participants