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

feat(tokenfactory): execute msgs for CreateDenom, ChangeAdmin, UpdateModuleParams #1607

Merged
merged 18 commits into from
Oct 3, 2023

Conversation

Unique-Divine
Copy link
Member

@Unique-Divine Unique-Divine commented Sep 28, 2023

@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

Merging #1607 (bdf47b8) into master (f1e92a5) will decrease coverage by 0.11%.
Report is 5 commits behind head on master.
The diff coverage is 80.47%.

@@            Coverage Diff             @@
##           master    #1607      +/-   ##
==========================================
- Coverage   74.18%   74.08%   -0.11%     
==========================================
  Files         165      175      +10     
  Lines       13177    14268    +1091     
==========================================
+ Hits         9775    10570     +795     
- Misses       2855     3096     +241     
- Partials      547      602      +55     
Files Coverage Δ
app/ante.go 54.00% <100.00%> (+0.93%) ⬆️
app/ante/commission.go 100.00% <100.00%> (ø)
app/ante/errors.go 100.00% <100.00%> (ø)
app/ante/fixed_gas.go 100.00% <100.00%> (ø)
x/common/testutil/cli/network.go 74.05% <100.00%> (ø)
x/tokenfactory/keeper/genesis.go 84.21% <100.00%> (ø)
x/tokenfactory/keeper/store.go 100.00% <100.00%> (+5.35%) ⬆️
x/tokenfactory/types/codec.go 100.00% <100.00%> (ø)
x/tokenfactory/types/errors.go 100.00% <ø> (ø)
x/tokenfactory/types/genesis.go 89.28% <100.00%> (+89.28%) ⬆️
... and 9 more

... and 4 files with indirect coverage changes

@Unique-Divine Unique-Divine marked this pull request as ready for review September 28, 2023 15:52
@Unique-Divine Unique-Divine requested a review from a team as a code owner September 28, 2023 15:52
test(tokenfactory): more CLI test coverage

feat(perp): Add user discounts (#1594)

* add discounts

* impl genesis

* fix test

* make test more robust

* galaxy brain int encoding fix

* test and optimise

* lint

* CHANGELOG.md

* do merge

* clarify fee variable names in applyDiscountAndRebate func

* refactor dnr_test.go as per PR review

* add genesis tests

---------

Co-authored-by: unknown unknown <unknown@unknown>
Co-authored-by: godismercilex <[email protected]>
Co-authored-by: Jonathan Gimeno <[email protected]>

feat(perp): Add user discounts (#1594)

* add discounts

* impl genesis

* fix test

* make test more robust

* galaxy brain int encoding fix

* test and optimise

* lint

* CHANGELOG.md

* do merge

* clarify fee variable names in applyDiscountAndRebate func

* refactor dnr_test.go as per PR review

* add genesis tests

---------

Co-authored-by: unknown unknown <unknown@unknown>
Co-authored-by: godismercilex <[email protected]>
Co-authored-by: Jonathan Gimeno <[email protected]>
@Unique-Divine
Copy link
Member Author

Unique-Divine commented Sep 29, 2023

I'm encountering something strange here where it says the perp protos are breaking and I'm not even editing those files.

image

image

Update: Solved by

}

ctx := sdk.UnwrapSDKContext(goCtx)
iter := q.Keeper.Store.Denoms.Indexes.Creator.ExactMatch(ctx, req.Creator)
Copy link
Contributor

Choose a reason for hiding this comment

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

We might need pagination there in the future. But that should not be an issue just yet

@@ -5,7 +5,7 @@ import (
)

func (s *TestSuite) TestQueryModuleParams() {
res, err := s.queryClient.Params(s.GoCtx(), &types.QueryParamsRequest{})
res, err := s.querier.Params(s.GoCtx(), &types.QueryParamsRequest{})
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add query denoms and query denom info tests?

Copy link
Contributor

@matthiasmatt matthiasmatt left a comment

Choose a reason for hiding this comment

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

Minor details on test, we can merge, but I'd like to talk design to avoid the spamming

Creator: txMsg.Sender,
Subdenom: txMsg.Subdenom,
}
err = k.Store.InsertDenom(ctx, denom)
Copy link
Contributor

Choose a reason for hiding this comment

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

How do we avoid people spamming that to do denom flipping (I create [a-z]btc)? Should we make the user pay some nibi for that?

Copy link
Member Author

Choose a reason for hiding this comment

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

It costs 4 full NIBI plus the usual gas for the tx.

@jgimeno jgimeno merged commit e054b45 into master Oct 3, 2023
16 of 17 checks passed
@jgimeno jgimeno deleted the realu/tf-1 branch October 3, 2023 17:18
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