-
Notifications
You must be signed in to change notification settings - Fork 22
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
fix: edit finality provider commission-rate
#296
base: main
Are you sure you want to change the base?
Conversation
commission-rate
commission-rate
currentProvider, err := c.QueryFinalityProviderInfo(ctx, fpPk) | ||
if err != nil { | ||
return fmt.Errorf("failed to get current provider info: %w", err) | ||
} | ||
|
||
if rate == "" { | ||
rate = currentProvider.FinalityProvider.Commission | ||
} | ||
|
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.
rather, let's first check if rate == "" and do the c.QueryFinalityProviderInfo
inside the if statement
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.
Nice work!
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.
I'm OK with the current fix. But ideally we should fix it on Babylon side -- follow Cosmos SDK's convension where empty commission rate is accepted. However, I was worried that this would cause breaking change to state as this changes how some txs are handled. Maybe we can leave that to v2
.
closes: #282
Previously commission rate was required although it should be optional. This fix fetches the already set commission rate otherwise will submit the new one if it is specified by the operator.