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

Set default validator OFA share to 50% #22

Merged
merged 5 commits into from
Jan 24, 2024
Merged

Conversation

jj1980a
Copy link
Contributor

@jj1980a jj1980a commented Jan 5, 2024

This sets the default refund share to 50% instead of 0.

Public method getValidatorRefundShare should be used by frontends to retrieve a validator refund share (instead of former public accessor validatorsRefundShareMap).

Copy link
Contributor

@BenSparksCode BenSparksCode left a comment

Choose a reason for hiding this comment

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

Not sure if int instead of uint for validatorsRefundShareMap is an intentional choice or not. It looks safe as it is now, just unexpected var type

@jj1980a
Copy link
Contributor Author

jj1980a commented Jan 5, 2024

Not sure if int instead of uint for validatorsRefundShareMap is an intentional choice or not. It looks safe as it is now, just unexpected var type

The intent is to set the refund share to 50% by default for all validators.

As validatorsRefundShareMap map is holding 0 values for all entries, we add DEFAULT_VALIDATOR_REFUND_SHARE to each of them (when calling getValidatorRefundShare).
When a validator calls updateValidatorRefundShare to set a new value, we subtract DEFAULT_VALIDATOR_REFUND_SHARE from that value and store it in validatorsRefundShareMap.

E.g. a validator calls:

updateValidatorRefundShare(1000); // Sets share to 10%

The validatorsRefundShareMap entry for this validator will be 1000 - 5000 = -4000.
And when calling:

getValidatorRefundShare(aboveValidator);

It returns -4000 + 5000 = 1000.

Hence validatorsRefundShareMap can hold negative values yes.
We could define DEFAULT_VALIDATOR_REFUND_SHARE as an uint or an int, either way, it will have to be casted in updateValidatorRefundShare or getValidatorRefundShare.
It does feel funny to have it as an int when all other constants are unsigned, I'm not against switching it to an uint.

@BenSparksCode
Copy link
Contributor

BenSparksCode commented Jan 9, 2024

Hence validatorsRefundShareMap can hold negative values yes.

Cool, this looks good to me then. I can't see any security issues caused by this. We can leave it as is imo.

@BenSparksCode BenSparksCode self-requested a review January 9, 2024 15:23
@thogard785 thogard785 merged commit 32742a8 into main Jan 24, 2024
1 check passed
@thogard785 thogard785 deleted the default-ofa-share branch January 24, 2024 08:29
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.

3 participants