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

fix: edit finality provider commission-rate #296

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

samricotta
Copy link
Contributor

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.

@samricotta samricotta changed the title fix: update commission-rate fix: edit finality provider commission-rate Jan 23, 2025
@samricotta samricotta requested a review from gitferry January 23, 2025 06:39
Comment on lines 137 to 145
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
}

Copy link
Member

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

Copy link
Member

@Lazar955 Lazar955 left a comment

Choose a reason for hiding this comment

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

Nice work!

Copy link
Member

@gitferry gitferry left a 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.

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.

bug: commission-rate required on edit-finality-provider
3 participants