Skip to content

Conversation

@jstarry
Copy link
Contributor

@jstarry jstarry commented Feb 18, 2025

@jstarry jstarry changed the title SIMD-XXXX: Delay commission updates SIMD-0249: Delay commission updates Feb 18, 2025
@Benhawkins18
Copy link
Collaborator

This to me is a conceptual no brainer.

@bartenbach
Copy link

Agreed. No brainer.


NA

## Detailed Design
Copy link
Contributor

Choose a reason for hiding this comment

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

is this going to be updated to include implementation details? this section is supposed to be sufficiently descriptive that someone can implement the feature/change from this document alone

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@jstarry jstarry force-pushed the delay-commission-updates branch from 0a911c0 to 0965d97 Compare May 14, 2025 08:11
@jstarry jstarry changed the title SIMD-0249: Delay commission updates SIMD-0249: Commission Update Rules May 14, 2025
@jstarry jstarry changed the title SIMD-0249: Commission Update Rules SIMD-0249: Modify Commission Update Rules May 14, 2025
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Looks great to me! Just a couple of questions

Comment on lines 37 to 39
available in-protocol. The only exception is inflation reward distribution for
epoch `E == 0`. In that case, use the commission rate set by vote accounts in
the genesis config.
Copy link
Contributor

Choose a reason for hiding this comment

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

How exactly does this work for new validators?

If I create a vote account and start getting delegations in epoch E - 1, then those stakes aren't active until the start of epoch E, and they should receive rewards after epoch E.

Is the idea to use the commission as of the vote account's creation during E - 1 when calculating payouts at the end of epoch E?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I create a vote account and start getting delegations in epoch E - 1, then those stakes aren't active until the start of epoch E, and they should receive rewards after epoch E.

Actually at earliest they will start receiving rewards for epoch E + 1 (distributed in epoch E + 2) because that's the earliest epoch in which your vote account could be in the leader schedule. We use the stake distribution at the beginning of epoch E to determine the leader schedule for E + 1 and epoch E is the earliest epoch in which your vote account would be staked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated this section and explicitly called out the new validator case: 1175904

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh gotcha, I wasn't aware that it took an extra epoch to start earning voting rewards. I thought as soon as a validator had active stake, they could earn voting rewards. Thanks for the clarification!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry I was mixing up inflation rewards with block revenue. My explanation about the leader schedule makes no sense for inflation rewards. So yes, we need to handle new validators as you pointed out!

It looks like it's also possible to delegate to a vote account that isn't fully initialized yet. So when a vote account is initialized and starts voting in epoch E it would already be earning rewards in that epoch.

Since there are a few edge cases here that add complexity to the proposal I think it's best to simplify this SIMD to only being about commission rates being updated to basis points and then try to handle commission rugs in a separate SIMD.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simplified here a25eca4

I still want to do the delay commission updates change at some point but I don't want that change to hold up getting the new UpdateCommissionBps instruction in because it's a blocker for #123

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved the commission rate in basis points change to a new SIMD and reverted this SIMD back to dealing with delayed commission rate updates. New SIMD is here: #291

Comment on lines 117 to 118
- Validators will be able to set inflation rewards commission rates in basis
points.
Copy link
Contributor

Choose a reason for hiding this comment

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

This presupposes that VoteStateV4 already exists, correct? If so, can you add a reference to that SIMD?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a "Dependencies" section in 1175904

joncinque
joncinque previously approved these changes May 29, 2025
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@jstarry jstarry changed the title SIMD-0249: Modify Commission Update Rules SIMD-0249: Commission Rate in Basis Points May 29, 2025
@jstarry jstarry force-pushed the delay-commission-updates branch from a25eca4 to 1ef0635 Compare May 29, 2025 22:26
@jstarry jstarry changed the title SIMD-0249: Commission Rate in Basis Points SIMD-0249: Delay Commission Updates May 29, 2025
Copy link
Contributor

@joncinque joncinque left a comment

Choose a reason for hiding this comment

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

Short, sweet, and clear, looks good to me!

@Benhawkins18 Benhawkins18 merged commit 7395033 into solana-foundation:main Jun 9, 2025
4 checks passed
@jstarry jstarry deleted the delay-commission-updates branch June 9, 2025 21:13
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