Skip to content

Comments

CIP: lockup account & staking rewards#253

Merged
jcstein merged 4 commits intocelestiaorg:mainfrom
01builders:marko/rewards
Mar 6, 2025
Merged

CIP: lockup account & staking rewards#253
jcstein merged 4 commits intocelestiaorg:mainfrom
01builders:marko/rewards

Conversation

@tac0turtle
Copy link
Contributor

Overview

This CIP proposes modifying lockup accounts to account for staking rewards. The change will modify lockup accounts total amount and how much is unlocked.

@tac0turtle tac0turtle changed the title CIP: lockup account rewards CIP: lockup account & staking rewards Feb 12, 2025
@gavinly
Copy link

gavinly commented Feb 12, 2025

suggest including the option to apply a reward-rate discount for staking locked tokens

protocol is overpaying for security, since it's much easier to attract locked tokens to staking than unlocked

cips/cip-31.md Outdated

# Backwards Compatibility

No backwards incompatibilities are introduced for non-lockup accounts. Existing lockup accounts not subject to the new reward integration will continue to operate as before. For accounts that do use the new functionality, the changes are additive and maintain the original lockup semantics.
Copy link
Collaborator

@rootulp rootulp Feb 12, 2025

Choose a reason for hiding this comment

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

Existing lockup accounts not subject to the new reward integration will continue to operate as before.

Which existing lockup accounts are not subject to this CIP? It's not entirely clear to me if this CIP impacts existing lockup accounts or only newly created lockup accounts. I assume the former but then this sentence doesn't make sense to me.

- **If it is:**
- Verify that it is of a modifiable lockup type.
- Calculate the new lockup amount:
`New Lockup Amount = Original Lockup Amount + Claimed Rewards`
Copy link
Collaborator

Choose a reason for hiding this comment

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

[question] In this context is "Claimed Rewards" only applicable to the claimed rewards at a particular moment in time? Put another way, this CIP only impacts claimed rewards after this feature has been added. Lockup accounts that have already claimed staking rewards (i.e. on celestia-app v3) don't contribute to the "Claimed Rewards" in this calculation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct, claims prior to the upgrade are not subject to locking at this would require large amount of offchain data

# Parameters

When upgrading to v4 we propose introducing a migration that will set the Parameter in [`x/Staking`](https://github.com/cosmos/cosmos-sdk/blob/release/v0.50.x/x/staking/types/staking.pb.go#L934) to 25% disallowing new validators from creating validators with \>25% commission rates.

Choose a reason for hiding this comment

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

2 questions:

  1. do I misunderstand the parameter in the referenced GOlang file or is the line item referring to a minimum commission, as opposed to a maximum commission that you are proposing?

  2. So somebody operating an active validator today with delegations today with a 100% commission set (e.g., for anti-money laundering reasons) would not be affected by this change for the existing validator? But if that operator wants to split his delegations to 2 validators (e.g., to mitigate slashing impact radius), the newly created validator would be restricted to a maximum 25% commission?

Copy link
Member

@liamsi liamsi Mar 3, 2025

Choose a reason for hiding this comment

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

do I misunderstand the parameter in the referenced GOlang file or is the line item referring to a minimum commission, as opposed to a maximum commission that you are proposing?

My understanding is that this just points to where to expect a new param (MaxCommissionRate) which applies to all new validators. Agreed this is confusing and should be made clearer here.

So somebody operating an active validator today with delegations today with a 100% commission set (e.g., for anti-money laundering reasons) would not be affected by this change for the existing validator?

Yes.

But if that operator wants to split his delegations to 2 validators (e.g., to mitigate slashing impact radius), the newly created validator would be restricted to a maximum 25% commission?

Yes, the newly created validator would also be restricted to 25%.

@tac0turtle
Copy link
Contributor Author

suggest including the option to apply a reward-rate discount for staking locked tokens

protocol is overpaying for security, since it's much easier to attract locked tokens to staking than unlocked

I did a lot of thinking about this and talked to some people. I believe teams should account for this at the start and in contractual agreements. We could run into the issue that due to how contracts are written today it would cause issues. If a new team would like to explore this im happy to share more insights

@jcstein
Copy link
Member

jcstein commented Feb 21, 2025

Please use a filename such as cip-lockup-accounts.md instead of assigning a number. See the example: https://cips.celestia.org/cip-template.html

@tac0turtle tac0turtle requested review from jcstein and rootulp March 5, 2025 19:48
Copy link
Member

@jcstein jcstein left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

@jcstein jcstein marked this pull request as ready for review March 5, 2025 20:58
@celestia-bot celestia-bot requested review from ebuchman and jcstein March 5, 2025 20:58
@@ -0,0 +1,157 @@

| cip | XX |
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| cip | XX |
| cip | 31 |

@jcstein
Copy link
Member

jcstein commented Mar 6, 2025

I'll merge this and rename accordingly 🫡

@jcstein jcstein merged commit b371912 into celestiaorg:main Mar 6, 2025
4 of 5 checks passed
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.

7 participants