SDK Core Audit - simapp updates #9315
Conversation
github-actions
bot
added
C:Simulations
C:x/auth
C:x/authz
C:x/bank
C:x/distribution
| @@ -443,12 +444,12 @@ func (ao EmptyAppOptions) Get(o string) interface{} { | |||
| // | |||
| // TODO: Instead of using the mint module account, which has the | |||
| // permission of minting, create a "faucet" account. (@fdymylja) | |||
| func FundAccount(app *SimApp, ctx sdk.Context, addr sdk.AccAddress, amounts sdk.Coins) error { | |||
| if err := app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, amounts); err != nil { | |||
| func FundAccount(bankKeeper bankkeeper.Keeper, ctx sdk.Context, addr sdk.AccAddress, amounts sdk.Coins) error { | |||
There was a problem hiding this comment.
addressing leftover comments from #8473 cc/ @robert-zaremba
There was a problem hiding this comment.
My main desire was to move this function away from the simapp package. This functions is used in tests.
#8473 (review)
There was a problem hiding this comment.
@robert-zaremba makes sense, to testutil for instance?
There was a problem hiding this comment.
x/bank/client/testutil could also be a candidate, even though FundAccount is not really a client util.
There was a problem hiding this comment.
This has received enough approvals, so let's tackle #9315 (comment) as part of a separate issue: #9346
There was a problem hiding this comment.
yes, we could put it to testutil. So, as you prefer, we can do it here , or in other PR.
| Address: authtypes.NewModuleAddress(stakingtypes.NotBondedPoolName).String(), | ||
| Coins: sdk.NewCoins(notBondedCoins), | ||
| }) | ||
| stakingAddr := authtypes.NewModuleAddress(stakingtypes.NotBondedPoolName).String() |
There was a problem hiding this comment.
ref: #8473 cc/ @robert-zaremba
Codecov Report
@@ Coverage Diff @@
## master #9315 +/- ##
==========================================
- Coverage 60.39% 60.38% -0.01%
==========================================
Files 587 587
Lines 36914 36920 +6
==========================================
Hits 22294 22294
- Misses 12645 12651 +6
Partials 1975 1975
|
There was a problem hiding this comment.
Approving - it's an improvement. But it doesn't solve the main concern: #8473 (review)
|
@blushi , are you planning to solve #8473 (review) here as well? |
I can move it as part of this PR I guess, once we have agreed on #9315 (comment) |
It would be great, otherwise we can put it as a checklist item in #9346 |
* Make package imports consistent * Update comment * Update FundAccount/FundModuleAccount * Fix bench test Co-authored-by: atheeshp <[email protected]>
* Make package imports consistent * Update comment * Update FundAccount/FundModuleAccount * Fix bench test Co-authored-by: atheeshp <[email protected]>
Description
ref: #9218
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes