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.

Sign up for GitHub

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

SIMD-0079: Allow commission decrease at any time during an epoch #79

Merged
merged 1 commit into from
Nov 10, 2023
Merged

SIMD-0079: Allow commission decrease at any time during an epoch #79

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions proposals/0079-allow_commission_decrease_at_any_time.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
---
simd: '0079'
title: Allow Commission Decrease at Any Time
authors:
- Bryan Ischo ([email protected])
category: Standard
type: Core
status: Draft
created: 2023-10-26
feature: https://github.com/solana-labs/solana/issues/29361
---

## Summary

The commission_updates_only_allowed_in_first_half_of_epoch feature disallows
bji marked this conversation as resolved.
Show resolved Hide resolved
commission decrease in the second half of epoch. Given that the purpose of this
feature was to prevent 'rug pulls' which are accomplished by increasing
commission at the end of epoch and then decreasing commission at the beginning
of next epoch, disallowing decreases during the second half of epoch is
unnecessary.

A feature gate must be added to support this SIMD as all validators' vote
programs must treat commission change instructions the same or else consensus
will diverge.

## Motivation

Some validator operators may need to decrease commission in order to satisfy
their own operational criteria.

As an example, a validator operator may have a policy whereby any error that
results in reduced stake account rewards for the epoch, will result in the
operator choosing to reduce commission to 0% for that epoch to ensure that
stake accounts are not disadvantaged by that error. Not being allowed to do
this in the second half of an epoch is a problem because it would prevent that
commission change until the next epoch, which will not allow this policy to
take effect for stake accounts which were de-activating during the epoch.

## Alternatives Considered

No alternatives were considered.

## New Terminology

None

## Detailed Design

A feature will be added which, when enabled, must cause all node
implementations' vote program's set-commission instruction handling to first
check whether the proposed commission change is a decrease or no change, and if
so, do not invoke the "only allow commission change in first half of epoch"
rule.

## Impact

Validators will now be able to decrease commission at any time in the epoch,
but only increase commission in the first half of epochs (because of the
commission_updates_only_allowed_in_first_half_of_epoch feature already
implemented).
Comment on lines +57 to +60
Copy link
Contributor

@ripatel-fd ripatel-fd Nov 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is just stating a technical fact.

But the impact section should describe how it affects the ecosystem. (Does it fix previously broken behavior, does it unblock a project, does it benefit delegators, etc...)

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 think this is splitting hairs on the purpose of SIMD sections. I'm going to leave this as-is.

Copy link
Contributor

@ripatel-fd ripatel-fd Nov 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Well, the purpose of sections is documented for a reason in the template. Clearly stating the impact on each SIMD helps with prioritizing work on these features (which always involve a lot of testing, btw). Unless I'm mistaken, this seems like "nice to have" level of importance to the ecosystem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

## Impact

How will the implemented proposal impacts dapp developers, validators, and core contributors?

The text included states how the change will impact validators. There is no impact to the other categories outside of the truism that "any change could affect anyone".

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we know how many validators have the policy mentioned in the motivation section, or how common this problem is? Something like that would be nice to document as impact, because I agree with Richard that unless we understand that this change does seem "nice to have"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is one good reason not enough?

Copy link
Contributor

@ripatel-fd ripatel-fd Nov 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Is one good reason not enough?

@bji Yes -- Long story short, we should to scrutinize feature activations more, especially by triaging them by usefulness. The impact section would be the place to make an argument why the feature is important.

The text included states how the change will impact validators.

If most validators didn't notice activation of this feature, is it really impactful? But your point is valid, the template should be clearer about the purpose of this section.

Considering this doesn't improve performance or reliability and it's unclear whether anyone else will use this feature, I would say it's barely enough justification. The risk of any feature flag is that it halts the network if the implementation is botched. Even though this change looks small, it's still a lot of work to write up all the test cases and test compatibility between Labs / Firedancer. The reward for this one is ... somewhat unknown.

This was one of the discussions at Block Zero, which I think you were also participating in: The feature activation backlog keeps growing because core contributors can only implement so many features per month.

IMHO The best way to address this problem is to focus on the most impactful features first.

To be clear, I'm still fine with merging this as-is. This is just a suggested change, not a request. Also, there's no need to keep resolving this conversation when it's still active.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original feature which prevented changes to the commission in the second half was an over-reach. It had a goal ("prevent rug pulls") and it created collateral damage by implementing a more restrictive than necessary policy.

It doesn't seem like the best way to have technical discussions to have to provide justification for whether or not the reasons are "good enough". What you think is important, someone else might not. It's necessary to include importance in prioritization of effort, but when discussing technical merits of a proposal, I think justification just should be a minor factor, and having "one good reason" should be good enough.

In this case, I've already given a justification of why this feature is important to me.

I can imagine other scenarios where it might matter. Will these sway anyone? Or is a back and forth discussion about whether "enough reasons have been imagined yet" going to be a part of the process?

  • Someone may accidentally set a high commission in the first half of epoch, and not realize it until the second half. It would be better if they could correct their mistake than if they were prevented from correcting it.

  • Someone may set a high commission in the first half of the epoch but change their mind and want to change it back down in the second half. It would be better if they could make their change rather than being restricted without reason from doing so.

  • Someone may receive a command from their government or other authority notifying them that if they accept commission, they are breaking the law and will go to jail. If this occurs in the second half of an epoch, and they can't set commission to 0, they will have no way to prevent going to jail since their validator already earned vote credits and cannot be prevented otherwise from earning commission (YES this is super far-out, but since the threshold for someone feeling this feature is justified is arbitrary, I have no idea how far out to go here).

  • Someone may run a sophisticated commission selection scheme that sets a 100% commission in the first half of the epoch, and then has their stakers vote on reducing it incrementally until they reach a compromise (there would be some other, not disclosed here, reason for stakers to vote for 'some commission' above 0 instead of just 0). If commission cannot be set in the second half of epochs, this mechanism may not work properly (YES this is super far out; see the previous paragraph for why)

...


## Security Considerations

None

## Drawbacks

It may cause additional confusion to validators who might not understand why
some types of commission changes succeed only in the first half of epochs while
others succeed always.

## Backwards Compatibility

This feature requires a feature gate because software which includes the
implementation will allow certain set-commission transactions to succeed where
software without the implementation would fail those transactions. Thus all
validators must be updated to the new functionality at an epoch boundary so
that all validators agree on the result of executing those transactions.
bji marked this conversation as resolved.
Show resolved Hide resolved

When activated, breaks the ability of older Solana node software to verify
ledgers with this feature.
Loading