Skip to content

cip: prevent auto claiming of rewards. #251

Merged
jcstein merged 6 commits intocelestiaorg:mainfrom
01builders:marko/distribution_claiming
Feb 19, 2025
Merged

cip: prevent auto claiming of rewards. #251
jcstein merged 6 commits intocelestiaorg:mainfrom
01builders:marko/distribution_claiming

Conversation

@tac0turtle
Copy link
Copy Markdown
Contributor

@tac0turtle tac0turtle commented Feb 7, 2025

Overview

This cip proposes a change to the distribution module, the change stops auto claiming of rewards when modifying delegation shares (delegation, redelgation, unbonding) and makes the movement of funds from the distribution module to a users account explicit.

@tac0turtle tac0turtle changed the title add cip 30 cip: prevent auto claiming of rewards. Feb 7, 2025
@tac0turtle tac0turtle force-pushed the marko/distribution_claiming branch from 2b9a082 to f3f35d6 Compare February 7, 2025 13:49

## Abstract

Currently, when delegators modify their staking positions (e.g., redelegate or undelegate), the distribution module automatically claims all accrued staking rewards. This CIP proposes removing that automatic functionality, thereby allowing users to intentionally choose *when* to claim their rewards. Specifically, the distribution module will no longer auto-claim rewards on delegation-share changes. Instead, it will store the accrued rewards until a user explicitly calls `MsgWithdrawDelegatorReward`. This avoids unintended reward-claiming events that can trigger immediate tax or regulatory obligations.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

seems like a simple enough change, do we have data on the rate of redelegations or undelegations or how much demand there is for this change? I couldn't think of any firm blockers to this, but its been a while since I dug through the maze that is the distribution and staking module logic.

separate to this limitted change,

This is a simple change, if we wanted something more complex we could propose an LST like design

could we do, or has any sdk based chain done, claimless auto compounding, redelegation, undelegation without creating an LST? presumably this requires a significant change

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yea i can get the data for the last few months.

we have explored this, its a deeper change in the distribution module. I can look into this as an alternative.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

so changing to be native compounding can be done, its not that hard. The main change is the UX around, users will need to undelegate to claim rewards. This requires a conversation on UX before moving forward. The nice part is that we would reduce state machine complexity per block significantly.


## Abstract

Currently, when delegators modify their staking positions (e.g., redelegate or undelegate), the distribution module automatically claims all accrued staking rewards. This CIP proposes removing that automatic functionality, thereby allowing users to intentionally choose *when* to claim their rewards. Specifically, the distribution module will no longer auto-claim rewards on delegation-share changes. Instead, it will store the accrued rewards until a user explicitly calls `MsgWithdrawDelegatorReward`. This avoids unintended reward-claiming events that can trigger immediate tax or regulatory obligations.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would this require going back to (or keeping) a fork of the sdk using v0.50?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we may be able to get away without a fork, but due to the global registry we may need to maintain a small fork.

@tac0turtle tac0turtle force-pushed the marko/distribution_claiming branch from f3f35d6 to 296b5fc Compare February 7, 2025 14:29
@ValarDragon
Copy link
Copy Markdown

Even independent of taxation, this is just generally a massive UX win. Its very annoying to have to explain or track as is. (Note, spec of this was definitely in large part my fault for not thinking of the UX pain here)

Copy link
Copy Markdown
Collaborator

@rootulp rootulp left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@tac0turtle tac0turtle marked this pull request as ready for review February 11, 2025 09:17
Co-authored-by: Rootul P <rootulp@gmail.com>
Co-authored-by: Rootul P <rootulp@gmail.com>
Copy link
Copy Markdown
Collaborator

@rootulp rootulp left a comment

Choose a reason for hiding this comment

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

Still LGTM

Copy link
Copy Markdown
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 with one nit: if this is merged before #249 we should title this CIP-29 and 249 as CIP-30, when adding to readme and be sure to change the filename.

@jcstein
Copy link
Copy Markdown
Member

jcstein commented Feb 12, 2025

it looks like editors don't have permission to edit this PR @tac0turtle, can you please grant permission? thank you

Co-authored-by: Josh Stein <46639943+jcstein@users.noreply.github.com>
Co-authored-by: Josh Stein <46639943+jcstein@users.noreply.github.com>
@jcstein jcstein merged commit 64ad5be into celestiaorg:main Feb 19, 2025
2 checks passed
@jcstein
Copy link
Copy Markdown
Member

jcstein commented Feb 19, 2025

I'm going to follow up with a PR to add to readme

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.

6 participants