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(x/bank): app wiring migration #12032

Merged
merged 11 commits into from
Jun 1, 2022
639 changes: 639 additions & 0 deletions api/cosmos/bank/module/v1/module.pulsar.go

Large diffs are not rendered by default.

13 changes: 10 additions & 3 deletions api/cosmos/tx/v1beta1/tx.pulsar.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion depinject/inject.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package depinject

// Build builds the container specified by containerConfig and extracts the
// Inject builds the container specified by containerConfig and extracts the
// requested outputs from the container or returns an error. It is the single
// entry point for building and running a dependency injection container.
// Each of the values specified as outputs must be pointers to types that
Expand Down
17 changes: 17 additions & 0 deletions proto/cosmos/bank/module/v1/module.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
syntax = "proto3";

package cosmos.bank.module.v1;

import "cosmos/app/v1alpha1/module.proto";

// Module is the config object of the params module.
message Module {
option (cosmos.app.v1alpha1.module) = {
go_import: "github.com/cosmos/cosmos-sdk/x/bank"
};

// blocked_module_accounts configures exceptional module accounts which should be blocked from receiving funds.
// If left empty defaults to the list of account names supplied in the auth module configuration as
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// If left empty defaults to the list of account names supplied in the auth module configuration as
// If left empty it defaults to the list of account names supplied in the auth module configuration as

// module_account_permissions
repeated string blocked_module_accounts = 1;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
repeated string blocked_module_accounts = 1;
repeated string blocked_module_accounts_override = 1;

}
25 changes: 7 additions & 18 deletions simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ func init() {
//go:embed app.yaml
var appConfigYaml []byte

var appConfig = appconfig.LoadYAML(appConfigYaml)
var AppConfig = appconfig.LoadYAML(appConfigYaml)

// NewSimApp returns a reference to an initialized SimApp.
func NewSimApp(
Expand All @@ -211,16 +211,18 @@ func NewSimApp(
var appBuilder *runtime.AppBuilder
var paramsKeeper paramskeeper.Keeper
var accountKeeper authkeeper.AccountKeeper
var bankKeeper bankkeeper.Keeper
var appCodec codec.Codec
var legacyAmino *codec.LegacyAmino
var interfaceRegistry codectypes.InterfaceRegistry
err := depinject.Inject(appConfig,
err := depinject.Inject(AppConfig,
&appBuilder,
&paramsKeeper,
&appCodec,
&legacyAmino,
&interfaceRegistry,
&accountKeeper,
&bankKeeper,
)
if err != nil {
panic(err)
Expand All @@ -229,7 +231,7 @@ func NewSimApp(
runtimeApp := appBuilder.Build(logger, db, traceStore, baseAppOptions...)

keys := sdk.NewKVStoreKeys(
banktypes.StoreKey, stakingtypes.StoreKey,
stakingtypes.StoreKey,
minttypes.StoreKey, distrtypes.StoreKey, slashingtypes.StoreKey,
govtypes.StoreKey, upgradetypes.StoreKey, feegrant.StoreKey,
evidencetypes.StoreKey, capabilitytypes.StoreKey,
Expand Down Expand Up @@ -266,9 +268,8 @@ func NewSimApp(
app.CapabilityKeeper.Seal()

// add keepers
app.BankKeeper = bankkeeper.NewBaseKeeper(
appCodec, keys[banktypes.StoreKey], app.AccountKeeper, app.GetSubspace(banktypes.ModuleName), app.ModuleAccountAddrs(),
)
app.BankKeeper = bankKeeper

stakingKeeper := stakingkeeper.NewKeeper(
appCodec, keys[stakingtypes.StoreKey], app.AccountKeeper, app.BankKeeper, app.GetSubspace(stakingtypes.ModuleName),
)
Expand Down Expand Up @@ -351,7 +352,6 @@ func NewSimApp(
encodingConfig.TxConfig,
),
vesting.NewAppModule(app.AccountKeeper, app.BankKeeper),
bank.NewAppModule(appCodec, app.BankKeeper, app.AccountKeeper),
capability.NewAppModule(appCodec, *app.CapabilityKeeper),
crisis.NewAppModule(&app.CrisisKeeper, skipGenesisInvariants),
feegrantmodule.NewAppModule(appCodec, app.AccountKeeper, app.BankKeeper, app.FeeGrantKeeper, app.interfaceRegistry),
Expand Down Expand Up @@ -498,16 +498,6 @@ func (app *SimApp) LoadHeight(height int64) error {
return app.LoadVersion(height)
}

// ModuleAccountAddrs returns all the app's module account addresses.
func (app *SimApp) ModuleAccountAddrs() map[string]bool {
modAccAddrs := make(map[string]bool)
for acc := range maccPerms {
modAccAddrs[authtypes.NewModuleAddress(acc).String()] = true
}

return modAccAddrs
}

// LegacyAmino returns SimApp's amino codec.
//
// NOTE: This is solely to be used for testing purposes as it may be desirable
Expand Down Expand Up @@ -606,7 +596,6 @@ func GetMaccPerms() map[string][]string {

// initParamsKeeper init params keeper and its subspaces
func initParamsKeeper(paramsKeeper paramskeeper.Keeper) {
paramsKeeper.Subspace(banktypes.ModuleName)
paramsKeeper.Subspace(stakingtypes.ModuleName)
paramsKeeper.Subspace(minttypes.ModuleName)
paramsKeeper.Subspace(distrtypes.ModuleName)
Expand Down
13 changes: 13 additions & 0 deletions simapp/app.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,16 @@ modules:
- name: params
config:
"@type": cosmos.params.module.v1.Module

- name: bank
config:
"@type": cosmos.bank.module.v1.Module
# uncomment to provide a block list different from the modules mentioned in auth.module_account_permissions
# blocked_module_accounts:
Copy link
Member

Choose a reason for hiding this comment

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

above we have

 module_account_permissions:
        - account: fee_collector
        - account: distribution

does this mean they can receive funds?

Copy link
Member Author

Choose a reason for hiding this comment

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

The default (existing) behavior is to disallow funds transfer to any of the module accounts in module_account_permissions including those you mentioned.

Copy link
Member Author

@kocubinski kocubinski Jun 1, 2022

Choose a reason for hiding this comment

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

The block that is commented out here is new functionality, providing a way to override bank behavior to differ from auth configuration. Previously they were both being injected from code in SimApp.

Copy link
Member

Choose a reason for hiding this comment

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

We should remove the commented out block. We shouldn't encourage anyone to override this for now I think. Or is there any module account which can receive sends?

Copy link
Member Author

Choose a reason for hiding this comment

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

My thinking was that it might be good to demonstrate the full functionality of app.yaml in comments, kind of like the lein sample.project.clj but perhaps this is better as a documentation task, not the live SimApp config.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we actually want to discourage this type of customization IMHO. This particular design issue has caused bugs. Honestly I can't wait to redesign it but for now we just need to limit damage

# - fee_collector
# - distribution
# - mint
# - bonded_tokens_pool
# - not_bonded_tokens_pool
# - gov
# - nft
4 changes: 2 additions & 2 deletions simapp/sim_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func BenchmarkFullAppSimulation(b *testing.B) {
AppStateFn(app.AppCodec(), app.SimulationManager()),
simtypes.RandomAccounts, // Replace with own random account function if using keys other than secp256k1
SimulationOperations(app, app.AppCodec(), config),
app.ModuleAccountAddrs(),
ModuleAccountAddrs(),
config,
app.AppCodec(),
)
Expand Down Expand Up @@ -87,7 +87,7 @@ func BenchmarkInvariants(b *testing.B) {
AppStateFn(app.AppCodec(), app.SimulationManager()),
simtypes.RandomAccounts, // Replace with own random account function if using keys other than secp256k1
SimulationOperations(app, app.AppCodec(), config),
app.ModuleAccountAddrs(),
ModuleAccountAddrs(),
config,
app.AppCodec(),
)
Expand Down
10 changes: 5 additions & 5 deletions simapp/sim_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func TestFullAppSimulation(t *testing.T) {
AppStateFn(app.AppCodec(), app.SimulationManager()),
simtypes.RandomAccounts, // Replace with own random account function if using keys other than secp256k1
SimulationOperations(app, app.AppCodec(), config),
app.ModuleAccountAddrs(),
ModuleAccountAddrs(),
config,
app.AppCodec(),
)
Expand Down Expand Up @@ -120,7 +120,7 @@ func TestAppImportExport(t *testing.T) {
AppStateFn(app.AppCodec(), app.SimulationManager()),
simtypes.RandomAccounts, // Replace with own random account function if using keys other than secp256k1
SimulationOperations(app, app.AppCodec(), config),
app.ModuleAccountAddrs(),
ModuleAccountAddrs(),
config,
app.AppCodec(),
)
Expand Down Expand Up @@ -229,7 +229,7 @@ func TestAppSimulationAfterImport(t *testing.T) {
AppStateFn(app.AppCodec(), app.SimulationManager()),
simtypes.RandomAccounts, // Replace with own random account function if using keys other than secp256k1
SimulationOperations(app, app.AppCodec(), config),
app.ModuleAccountAddrs(),
ModuleAccountAddrs(),
config,
app.AppCodec(),
)
Expand Down Expand Up @@ -277,7 +277,7 @@ func TestAppSimulationAfterImport(t *testing.T) {
AppStateFn(app.AppCodec(), app.SimulationManager()),
simtypes.RandomAccounts, // Replace with own random account function if using keys other than secp256k1
SimulationOperations(newApp, newApp.AppCodec(), config),
app.ModuleAccountAddrs(),
ModuleAccountAddrs(),
config,
app.AppCodec(),
)
Expand Down Expand Up @@ -328,7 +328,7 @@ func TestAppStateDeterminism(t *testing.T) {
AppStateFn(app.AppCodec(), app.SimulationManager()),
simtypes.RandomAccounts, // Replace with own random account function if using keys other than secp256k1
SimulationOperations(app, app.AppCodec(), config),
app.ModuleAccountAddrs(),
ModuleAccountAddrs(),
config,
app.AppCodec(),
)
Expand Down
14 changes: 14 additions & 0 deletions simapp/test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"encoding/hex"
"encoding/json"
"fmt"
"github.com/cosmos/cosmos-sdk/depinject"
bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper"
"strconv"
"testing"
"time"
Expand Down Expand Up @@ -505,3 +507,15 @@ type EmptyAppOptions struct{}
func (ao EmptyAppOptions) Get(o string) interface{} {
return nil
}

// ModuleAccountAddrs provides a list of blocked module accounts from configuration in app.yaml
//
// Ported from SimApp
func ModuleAccountAddrs() map[string]bool {
Copy link
Member Author

Choose a reason for hiding this comment

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

Replaces SimApp.ModuleAccountAddrs for test function usages.

var bk bankkeeper.Keeper
err := depinject.Inject(AppConfig, &bk)
if err != nil {
panic("unable to load DI container")
}
return bk.GetBlockedAddresses()
}
3 changes: 0 additions & 3 deletions simapp/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,6 @@ type App interface {
forZeroHeight bool, jailAllowedAddrs []string,
) (types.ExportedApp, error)

// All the registered module account addreses.
ModuleAccountAddrs() map[string]bool

// Helper for the simulation framework.
SimulationManager() *module.SimulationManager
}
13 changes: 10 additions & 3 deletions types/tx/tx.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions x/auth/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ type AccountKeeperI interface {

// Fetch the next account number, and increment the internal counter.
GetNextAccountNumber(sdk.Context) uint64

// GetModulePermissions fetches per-module account permissions
GetModulePermissions() map[string]types.PermissionsForAddress
}

// AccountKeeper encodes/decodes accounts using the go-amino (binary)
Expand Down Expand Up @@ -148,6 +151,11 @@ func (ak AccountKeeper) GetNextAccountNumber(ctx sdk.Context) uint64 {
return accNumber
}

// GetModulePermissions fetches per-module account permissions.
func (ak AccountKeeper) GetModulePermissions() map[string]types.PermissionsForAddress {
return ak.permAddrs
}

// ValidatePermissions validates that the module account has been granted
// permissions within its set of allowed permissions.
func (ak AccountKeeper) ValidatePermissions(macc types.ModuleAccountI) error {
Expand Down
14 changes: 11 additions & 3 deletions x/auth/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@ import (
"context"
"encoding/json"
"fmt"
"github.com/cosmos/cosmos-sdk/depinject"
"math/rand"

gwruntime "github.com/grpc-ecosystem/grpc-gateway/runtime"

"github.com/spf13/cobra"
abci "github.com/tendermint/tendermint/abci/types"

"cosmossdk.io/core/appmodule"
modulev1 "cosmossdk.io/api/cosmos/auth/module/v1"
"cosmossdk.io/core/appmodule"
"github.com/cosmos/cosmos-sdk/runtime"
store "github.com/cosmos/cosmos-sdk/store/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
Expand Down Expand Up @@ -206,11 +207,18 @@ func provideModuleBasic() runtime.AppModuleBasicWrapper {
return runtime.WrapAppModuleBasic(AppModuleBasic{})
}

type authOutputs struct {
depinject.Out

AccountKeeper keeper.AccountKeeper `key:"cosmos.auth.v1.AccountKeeper"`
Module runtime.AppModuleWrapper
}

func provideModule(
config *modulev1.Module,
key *store.KVStoreKey,
cdc codec.Codec,
subspace paramtypes.Subspace) (keeper.AccountKeeper, runtime.AppModuleWrapper) {
subspace paramtypes.Subspace) authOutputs {

maccPerms := map[string][]string{}
for _, permission := range config.ModuleAccountPermissions {
Expand All @@ -219,5 +227,5 @@ func provideModule(

k := keeper.NewAccountKeeper(cdc, key, subspace, types.ProtoBaseAccount, maccPerms, config.Bech32Prefix)
m := NewAppModule(cdc, k, simulation.RandomGenesisAccounts)
return k, runtime.WrapAppModule(m)
return authOutputs{AccountKeeper: k, Module: runtime.WrapAppModule(m)}
}
6 changes: 6 additions & 0 deletions x/bank/keeper/send.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ type SendKeeper interface {
IsSendEnabledCoins(ctx sdk.Context, coins ...sdk.Coin) error

BlockedAddr(addr sdk.AccAddress) bool
GetBlockedAddresses() map[string]bool
}

var _ SendKeeper = (*BaseSendKeeper)(nil)
Expand Down Expand Up @@ -322,3 +323,8 @@ func (k BaseSendKeeper) IsSendEnabledCoin(ctx sdk.Context, coin sdk.Coin) bool {
func (k BaseSendKeeper) BlockedAddr(addr sdk.AccAddress) bool {
return k.blockedAddrs[addr.String()]
}

// GetBlockedAddresses returns the full list of addresses restricted from receiving funds.
func (k BaseSendKeeper) GetBlockedAddresses() map[string]bool {
return k.blockedAddrs
}
Loading