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 Initgenesis bug in tokenfactory, when the denom creation fee para… #2011

Merged
merged 6 commits into from
Jul 10, 2022

Conversation

ValarDragon
Copy link
Member

@ValarDragon ValarDragon commented Jul 10, 2022

…m is set

What is the purpose of the change

Fixes a bug Adam pointed out, in TokenFactory genesis handling. (Which led to panics in his testnet generation)

Brief Changelog

  • Refactor TokenFactory createdenom logic, to make a direct method post-fee-charging that genesis can call
  • Switch genesis to no longer charge a fee
  • Add test cases to cover this.

Testing and Verifying

This change added tests and can be verified as follows:

  • Added a case for a denom with a different admin (previously untested in InitGenesis)
  • Sets the token creation fee

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? yes
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? yes
  • How is the feature or change documented? N/A

@ValarDragon ValarDragon requested a review from a team July 10, 2022 04:08
Comment on lines -20 to -30
// Send creation fee to community pool
creationFee := k.GetParams(ctx).DenomCreationFee
accAddr, err := sdk.AccAddressFromBech32(creatorAddr)
err = k.chargeForCreateDenom(ctx, creatorAddr, subdenom)
if err != nil {
return "", err
}
if len(creationFee) > 0 {
if err := k.distrKeeper.FundCommunityPool(ctx, creationFee, accAddr); err != nil {
return "", err
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to chargeForCreateDenom

@ValarDragon ValarDragon added the A:backport/v10.x backport patches to v10.x branch label Jul 10, 2022
@github-actions github-actions bot added the C:docs Improvements or additions to documentation label Jul 10, 2022
Copy link
Member

@czarcas7ic czarcas7ic left a comment

Choose a reason for hiding this comment

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

Tested this locally on a state export testnet and it works 👍

@czarcas7ic czarcas7ic merged commit 938f9bd into main Jul 10, 2022
@czarcas7ic czarcas7ic deleted the dev/fix_tokenfactory_init_genesis branch July 10, 2022 16:30
@ValarDragon ValarDragon added A:backport/v10.x backport patches to v10.x branch and removed A:backport/v10.x backport patches to v10.x branch labels Jul 11, 2022
mergify bot pushed a commit that referenced this pull request Jul 11, 2022
#2011)

* Fix Initgenesis bug in tokenfactory, when the denom creation fee param is set

* Add more to genesis test that would've caught this

* Update changelog

* Fix remaining bug adam pointed out

* Make test account for this

* Update code comment

(cherry picked from commit 938f9bd)

# Conflicts:
#	CHANGELOG.md
#	x/tokenfactory/keeper/createdenom.go
#	x/tokenfactory/keeper/createdenom_test.go
#	x/tokenfactory/keeper/genesis_test.go
ValarDragon added a commit that referenced this pull request Jul 11, 2022
…… (backport #2011) (#2020)

* Fix Initgenesis bug in tokenfactory, when the denom creation fee para… (#2011)

* Fix Initgenesis bug in tokenfactory, when the denom creation fee param is set

* Add more to genesis test that would've caught this

* Update changelog

* Fix remaining bug adam pointed out

* Make test account for this

* Update code comment

(cherry picked from commit 938f9bd)

# Conflicts:
#	CHANGELOG.md
#	x/tokenfactory/keeper/createdenom.go
#	x/tokenfactory/keeper/createdenom_test.go
#	x/tokenfactory/keeper/genesis_test.go

* Progress on merge conflicts

* Fix changelog bug

* Remove unused import

Co-authored-by: Dev Ojha <[email protected]>
Co-authored-by: Dev Ojha <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:backport/v10.x backport patches to v10.x branch C:docs Improvements or additions to documentation C:x/tokenfactory
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants