-
Notifications
You must be signed in to change notification settings - Fork 0
feat: fixed commision rate and max rate parameter #19
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
base: release/v0.50.x
Are you sure you want to change the base?
Conversation
…at/fixed-comission-rate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't you need a store migration to initialize the max commission value ?
EDIT: sorry I was on a review rush and didn't realize this PR was marked as draft 😬
|
||
if msg.CommissionRate.GT(maxCommissionRate) { | ||
return nil, errorsmod.Wrapf(types.ErrCommissionLTMaxRate, "cannot set validator (%s) commission to more than minimum rate of %s", msg.CommissionRate.String(), maxCommissionRate.String()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicated code.
Could also be simplified with a single condition, something like this:
if msg.CommissionRate.LT(minCommissionRate) || msg.CommissionRate.GT(maxCommissionRate) {
return nil, errorsmod.Wrapf(types.ErrCommissionOutOfBound,
"commission rate (%s) must be between %s and %s",
msg.CommissionRate.String(), minCommissionRate.String(), maxCommissionRate.String(),
)
}
|
||
simState.AppParams.GetOrGenerate(historicalEntries, &histEntries, simState.Rand, func(r *rand.Rand) { histEntries = getHistEntries(r) }) | ||
|
||
simState.AppParams.GetOrGenerate(maximumCommissionRate, &histEntries, simState.Rand, func(r *rand.Rand) { maxCommissionRate = getMaxCommissionRate(r) }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some curious reason, minCommissionRate
is not initialized here. Secondly I don't think you should pass the address og histEntries
here.
DefaultHistoricalEntries uint32 = 10000 | ||
|
||
// DefaultMaxCommission default maximum commission. | ||
DefaultMaxCommission = 100 // 30% (30/100 = 0.3 = 30% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// 30% (30/100 = 0.3 = 30%
This comment seems wrong.
close atomone-hub/atomone#172