Skip to content

Commit

Permalink
[Bank] Remove the unsafe balance changing API (cosmos#8473)
Browse files Browse the repository at this point in the history
* temp commit

* setbalance now is internal

* remove set balances in genesis

* feedback test commit

* update tests

* fix: genesis panic message

* fix not bonded pool

* fix(staking): genesis test

* fix(simapp): rollback state fix change

* fix(staking): genesis large val set test

* [Bank Refactor] Frojdi jonathan/remove setsupply (cosmos#8491)

* init supply in a different way

* remove external usage of set supply

* change(staking): replace SetSupply with MintCoins in tests

* change(evidence): replace SetSupply with MintCoins in tests

* change(crisis): remove SetSupply in tests

* change(bank): remove set supply from genesis tests

* change(bank): remove set supply from keeper tests

* change(bank): remove remaining set supply usage from keeper tests

* change(bank): remove set supply usage from grpc query and querier tests

* change(bank): remove SetSupply from keeper interface

Co-authored-by: Frojdi Dymylja <[email protected]>

* remove setbalances from genesis in gov

* remove keyring

* add init genesis state

* change(staking): make genesis checks coherent and add tests

* remove setbalances on distribution

* fix(staking): genesis tests

* [Bank Refactor]: Remove SetBalances usage from the code and tests (cosmos#8509)

* change(distribution): remove SetBalances usage from keeper tests

* add(simapp): FundAccount utility function

* chore(staking): use FundAccount in keeper tests

* change(staking): remove usage of SetBalance in allocation tests

* change(staking): remove usage of SetBalance in delegation tests

* change(staking): remove usage of SetBalance in proposal handler tests

* change(staking): remove usage of SetBalances in grpc query tests

* change(staking): remove usage of SetBalances in operations tests

* change(distribution): remove usage of SetBalances in genesis

* change(authz): remove usage of SetBalances keeper and operations test

* fix(authz): TestKeeperFees failing test

* change(slashing): remove SetBalances from expected BankKeeper

* change(slashing): remove usage of SetBalances in tests

* change(distribution): remove SetBalances from expected BankKeeper

* change(genutil): remove usage of SetBalances from tests

* change(gov): remove SetBalances from expected BankKeeper

* change(gov): remove usage of SetBalances from tests

* change(staking): remove usage of SetBalances from slash tests

* change(staking): remove SetBalances from expected BankKeeper

* change(staking): remove usage of SetBalances from delegation tests

* change(staking): remove usage of SetBalances from operations tests

* change(staking): remove usage of SetBalances from validator tests

* change(bank): remove usage of SetBalances from app tests

* change(bank): remove usage of SetBalances from bench tests

* change(bank): remove usage of SetBalances from querier tests

* change(bank): remove usage of SetBalances from grpc query tests

* change(bank): remove usage of SetBalances from operations tests

* change(bank): partially remove usage of SetBalances from keeper tests

* change(bank): finalize removal of usage of SetBalances from keeper tests

* change(auth): remove usage of SetBalances from verify tests

* change(auth): partially remove usage of SetBalances from tests

* [Bank refactor]: finalize removal of setbalances from auth (cosmos#8527)

* add tests with is check tx

* temp commit

* fix test

* fix other test and remove setbalances

* change(auth): remove usage of SetBalances is vesting tests

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

* change(types): remove usage of SetBalances in queries

* fix(types): pagination tests

* [Bank refactor] fix pagination tests (cosmos#8550)

* fix tests

* lint

* change(bank): remove SetBalances from keeper public API

Co-authored-by: Jonathan Gimeno <[email protected]>
Co-authored-by: SaReN <[email protected]>

* change(bank): remove SubtractCoins from keeper public API

* change(ibc/transfer): remove AddCoins from relay tests

* change(bank): remove AddCoins from public keeper API

* fix imports

* remove set balances

* fix fee test

* remove set balances

* fix(staking): remove dependency on minter authorization for staking pools

* chore: update CHANGELOG.md

* update: x/distribution/keeper/keeper_test.go

Co-authored-by: Robert Zaremba <[email protected]>

* Update simapp/test_helpers.go

Co-authored-by: Robert Zaremba <[email protected]>

* Update x/staking/genesis_test.go

Co-authored-by: Robert Zaremba <[email protected]>

* fix(simapp): FundAccount amount variable name

* fix some PR issues

Co-authored-by: Frojdi Dymylja <[email protected]>
Co-authored-by: Frojdi Dymylja <[email protected]>
Co-authored-by: SaReN <[email protected]>
Co-authored-by: Alessio Treglia <[email protected]>
Co-authored-by: Robert Zaremba <[email protected]>
  • Loading branch information
6 people authored Feb 17, 2021
1 parent 3929db6 commit c637830
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 21 deletions.
54 changes: 54 additions & 0 deletions state.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,12 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
simappparams "github.com/cosmos/cosmos-sdk/simapp/params"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
simtypes "github.com/cosmos/cosmos-sdk/types/simulation"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
)

// AppStateFn returns the initial application state using a genesis or the simulation parameters.
Expand Down Expand Up @@ -68,6 +71,57 @@ func AppStateFn(cdc codec.JSONMarshaler, simManager *module.SimulationManager) s
appState, simAccs = AppStateRandomizedFn(simManager, r, cdc, accs, genesisTimestamp, appParams)
}

rawState := make(map[string]json.RawMessage)
err := json.Unmarshal(appState, &rawState)
if err != nil {
panic(err)
}

stakingStateBz, ok := rawState[stakingtypes.ModuleName]
if !ok {
panic("staking genesis state is missing")
}

stakingState := new(stakingtypes.GenesisState)
err = cdc.UnmarshalJSON(stakingStateBz, stakingState)
if err != nil {
panic(err)
}
// compute not bonded balance
notBondedTokens := sdk.ZeroInt()
for _, val := range stakingState.Validators {
if val.Status != stakingtypes.Unbonded {
continue
}
notBondedTokens = notBondedTokens.Add(val.GetTokens())
}
notBondedCoins := sdk.NewCoin(stakingState.Params.BondDenom, notBondedTokens)
// edit bank state to make it have the not bonded pool tokens
bankStateBz, ok := rawState[banktypes.ModuleName]
// TODO(fdymylja/jonathan): should we panic in this case
if !ok {
panic("bank genesis state is missing")
}
bankState := new(banktypes.GenesisState)
err = cdc.UnmarshalJSON(bankStateBz, bankState)
if err != nil {
panic(err)
}

bankState.Balances = append(bankState.Balances, banktypes.Balance{
Address: authtypes.NewModuleAddress(stakingtypes.NotBondedPoolName).String(),
Coins: sdk.NewCoins(notBondedCoins),
})

// change appState back
rawState[stakingtypes.ModuleName] = cdc.MustMarshalJSON(stakingState)
rawState[banktypes.ModuleName] = cdc.MustMarshalJSON(bankState)

// replace appstate
appState, err = json.Marshal(rawState)
if err != nil {
panic(err)
}
return appState, simAccs, chainID, genesisTimestamp
}
}
Expand Down
48 changes: 27 additions & 21 deletions test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"github.com/cosmos/cosmos-sdk/types/errors"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
minttypes "github.com/cosmos/cosmos-sdk/x/mint/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
)

Expand Down Expand Up @@ -119,7 +120,6 @@ func SetupWithGenesisValSet(t *testing.T, valSet *tmtypes.ValidatorSet, genAccs
delegations = append(delegations, stakingtypes.NewDelegation(genAccs[0].GetAddress(), val.Address.Bytes(), sdk.OneDec()))

}

// set validators and delegations
stakingGenesis := stakingtypes.NewGenesisState(stakingtypes.DefaultParams(), validators, delegations)
genesisState[stakingtypes.ModuleName] = app.AppCodec().MustMarshalJSON(stakingGenesis)
Expand All @@ -130,6 +130,12 @@ func SetupWithGenesisValSet(t *testing.T, valSet *tmtypes.ValidatorSet, genAccs
totalSupply = totalSupply.Add(b.Coins.Add(sdk.NewCoin(sdk.DefaultBondDenom, bondAmt))...)
}

// add bonded amount to bonded pool module account
balances = append(balances, banktypes.Balance{
Address: authtypes.NewModuleAddress(stakingtypes.BondedPoolName).String(),
Coins: sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, bondAmt)},
})

// update total supply
bankGenesis := banktypes.NewGenesisState(banktypes.DefaultGenesisState().Params, balances, totalSupply, []banktypes.Metadata{})
genesisState[banktypes.ModuleName] = app.AppCodec().MustMarshalJSON(bankGenesis)
Expand Down Expand Up @@ -231,21 +237,11 @@ func createIncrementalAccounts(accNum int) []sdk.AccAddress {
func AddTestAddrsFromPubKeys(app *SimApp, ctx sdk.Context, pubKeys []cryptotypes.PubKey, accAmt sdk.Int) {
initCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), accAmt))

setTotalSupply(app, ctx, accAmt, len(pubKeys))

// fill all the addresses with some coins, set the loose pool tokens simultaneously
for _, pubKey := range pubKeys {
saveAccount(app, ctx, sdk.AccAddress(pubKey.Address()), initCoins)
for _, pk := range pubKeys {
initAccountWithCoins(app, ctx, sdk.AccAddress(pk.Address()), initCoins)
}
}

// setTotalSupply provides the total supply based on accAmt * totalAccounts.
func setTotalSupply(app *SimApp, ctx sdk.Context, accAmt sdk.Int, totalAccounts int) {
totalSupply := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), accAmt.MulRaw(int64(totalAccounts))))
prevSupply := app.BankKeeper.GetSupply(ctx)
app.BankKeeper.SetSupply(ctx, banktypes.NewSupply(prevSupply.GetTotal().Add(totalSupply...)))
}

// AddTestAddrs constructs and returns accNum amount of accounts with an
// initial balance of accAmt in random order
func AddTestAddrs(app *SimApp, ctx sdk.Context, accNum int, accAmt sdk.Int) []sdk.AccAddress {
Expand All @@ -262,21 +258,21 @@ func addTestAddrs(app *SimApp, ctx sdk.Context, accNum int, accAmt sdk.Int, stra
testAddrs := strategy(accNum)

initCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), accAmt))
setTotalSupply(app, ctx, accAmt, accNum)

// fill all the addresses with some coins, set the loose pool tokens simultaneously
for _, addr := range testAddrs {
saveAccount(app, ctx, addr, initCoins)
initAccountWithCoins(app, ctx, addr, initCoins)
}

return testAddrs
}

// saveAccount saves the provided account into the simapp with balance based on initCoins.
func saveAccount(app *SimApp, ctx sdk.Context, addr sdk.AccAddress, initCoins sdk.Coins) {
acc := app.AccountKeeper.NewAccountWithAddress(ctx, addr)
app.AccountKeeper.SetAccount(ctx, acc)
err := app.BankKeeper.AddCoins(ctx, addr, initCoins)
func initAccountWithCoins(app *SimApp, ctx sdk.Context, addr sdk.AccAddress, coins sdk.Coins) {
err := app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, coins)
if err != nil {
panic(err)
}

err = app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr, coins)
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -440,3 +436,13 @@ type EmptyAppOptions struct{}
func (ao EmptyAppOptions) Get(o string) interface{} {
return nil
}

// FundAccount is a utility function that funds an account by minting and sending the coins to the address
// TODO(fdymylja): instead of using the mint module account, which has the permission of minting, create a "faucet" account
func FundAccount(app *SimApp, ctx sdk.Context, addr sdk.AccAddress, amounts sdk.Coins) error {
err := app.BankKeeper.MintCoins(ctx, minttypes.ModuleName, amounts)
if err != nil {
return err
}
return app.BankKeeper.SendCoinsFromModuleToAccount(ctx, minttypes.ModuleName, addr, amounts)
}

0 comments on commit c637830

Please sign in to comment.