Enable/disable coin transfers by denom (cosmos#6527)

* initial implementation of per denom sendenabled * Fix for accidentally removed keyword * Validate individual param in param array * Lint fix * Refactor bank params to use protobuf Modified SendEnabled property to be part of generic Params object Updated genesis functions to use default params structure * Refactor simulation genesis for clarity * update changelog for bank sendenable per denom * fix NoOpMsg type in multisend test * Add a coin denom send check utility function * Additional godoc comments and clarification * Add default send enabled parameter to bank. Remove empty denom capability from SendEnabled parameters Update simulation to exercise both configuration options independently * Minor suggested improvements. * simulation fix * bank proto sendenabled package name removed * Remove extra gogo proto yaml tags * Params rename IsSendEnabled to SendEnabledDenom * Refactor to SendEnabledCoin(s) * update slashing test to use bank params * Clean up change log entry for feature. Co-authored-by: Federico Kunze <[email protected]> Co-authored-by: Alessio Treglia <[email protected]> Co-authored-by: Alexander Bezobchuk <[email protected]>
augchan42 · Jul 8, 2020 · 589c1a5 · 589c1a5
1 parent a459b78
commit 589c1a5
Show file tree

Hide file tree

Showing 16 changed files with 938 additions and 100 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -203,6 +203,9 @@ invalid or incomplete requests.
 ### State Machine Breaking
 
 * (x/bank) [\#6283](https://github.com/cosmos/cosmos-sdk/pull/6283) Create account if recipient does not exist on handing `MsgMultiSend`.
+* (x/bank) [\#6518](https://github.com/cosmos/cosmos-sdk/pull/6518) Support for global and per-denomination send enabled flags.
+  * Existing send_enabled global flag has been moved into a Params structure as `default_send_enabled`.
+  * An array of: `{denom: string, enabled: bool}` is added to bank Params to support per-denomination override of global default value.
 * (x/staking) [\#6061](https://github.com/cosmos/cosmos-sdk/pull/6061) Allow a validator to immediately unjail when no signing info is present due to
 falling below their minimum self-delegation and never having been bonded. The validator may immediately unjail once they've met their minimum self-delegation.
 * (x/supply) [\#6010](https://github.com/cosmos/cosmos-sdk/pull/6010) Removed the `x/supply` module by merging the existing types and APIs into the `x/bank` module.

diff --git a/proto/cosmos/bank/bank.proto b/proto/cosmos/bank/bank.proto
@@ -7,6 +7,25 @@ import "cosmos/cosmos.proto";
 
 option go_package = "github.com/cosmos/cosmos-sdk/x/bank/types";
 
+// Params defines the set of bank parameters.
+message Params {
+  option (gogoproto.goproto_stringer) = false;
+  repeated SendEnabled send_enabled = 1[
+    (gogoproto.moretags) = "yaml:\"send_enabled,omitempty\""
+  ];
+  bool default_send_enabled = 2[
+    (gogoproto.moretags) = "yaml:\"default_send_enabled,omitempty\""
+  ];
+}
+
+// Send enabled configuration properties for each denomination
+message SendEnabled {
+  option (gogoproto.equal) = true;
+  option (gogoproto.goproto_stringer) = false;
+  string denom = 1;
+  bool enabled = 2;
+}
+
 // MsgSend - high level transaction of the coin module
 message MsgSend {
   option (gogoproto.equal) = true;

diff --git a/simapp/test_helpers.go b/simapp/test_helpers.go
@@ -118,7 +118,7 @@ func SetupWithGenesisValSet(t *testing.T, valSet *tmtypes.ValidatorSet, genAccs
 	}
 
 	// update total supply
-	bankGenesis := banktypes.NewGenesisState(banktypes.DefaultGenesisState().SendEnabled, balances, totalSupply)
+	bankGenesis := banktypes.NewGenesisState(banktypes.DefaultGenesisState().Params, balances, totalSupply)
 	genesisState[banktypes.ModuleName] = app.Codec().MustMarshalJSON(bankGenesis)
 
 	stateBytes, err := codec.MarshalJSONIndent(app.Codec(), genesisState)
@@ -157,7 +157,7 @@ func SetupWithGenesisAccounts(genAccs []authtypes.GenesisAccount, balances ...ba
 		totalSupply = totalSupply.Add(b.Coins...)
 	}
 
-	bankGenesis := banktypes.NewGenesisState(banktypes.DefaultGenesisState().SendEnabled, balances, totalSupply)
+	bankGenesis := banktypes.NewGenesisState(banktypes.DefaultGenesisState().Params, balances, totalSupply)
 	genesisState[banktypes.ModuleName] = app.Codec().MustMarshalJSON(bankGenesis)
 
 	stateBytes, err := codec.MarshalJSONIndent(app.Codec(), genesisState)

diff --git a/x/bank/genesis.go b/x/bank/genesis.go
@@ -10,7 +10,7 @@ import (
 
 // InitGenesis initializes the bank module's state from a given genesis state.
 func InitGenesis(ctx sdk.Context, keeper keeper.Keeper, genState types.GenesisState) {
-	keeper.SetSendEnabled(ctx, genState.SendEnabled)
+	keeper.SetParams(ctx, genState.Params)
 
 	var totalSupply sdk.Coins
 
@@ -57,11 +57,14 @@ func ExportGenesis(ctx sdk.Context, keeper keeper.Keeper) types.GenesisState {
 		})
 	}
 
-	return types.NewGenesisState(keeper.GetSendEnabled(ctx), balances, keeper.GetSupply(ctx).GetTotal())
+	return types.NewGenesisState(keeper.GetParams(ctx), balances, keeper.GetSupply(ctx).GetTotal())
 }
 
 // ValidateGenesis performs basic validation of supply genesis data returning an
 // error for any failed validation criteria.
 func ValidateGenesis(data types.GenesisState) error {
+	if err := data.Params.Validate(); err != nil {
+		return err
+	}
 	return types.NewSupply(data.Supply).ValidateBasic()
 }
diff --git a/x/bank/handler.go b/x/bank/handler.go
@@ -30,8 +30,8 @@ func NewHandler(k keeper.Keeper) sdk.Handler {
 
 // Handle MsgSend.
 func handleMsgSend(ctx sdk.Context, k keeper.Keeper, msg *types.MsgSend) (*sdk.Result, error) {
-	if !k.GetSendEnabled(ctx) {
-		return nil, types.ErrSendDisabled
+	if err := k.SendEnabledCoins(ctx, msg.Amount...); err != nil {
+		return nil, err
 	}
 
 	if k.BlockedAddr(msg.ToAddress) {
@@ -66,8 +66,10 @@ func handleMsgSend(ctx sdk.Context, k keeper.Keeper, msg *types.MsgSend) (*sdk.R
 // Handle MsgMultiSend.
 func handleMsgMultiSend(ctx sdk.Context, k keeper.Keeper, msg *types.MsgMultiSend) (*sdk.Result, error) {
 	// NOTE: totalIn == totalOut should already have been checked
-	if !k.GetSendEnabled(ctx) {
-		return nil, types.ErrSendDisabled
+	for _, in := range msg.Inputs {
+		if err := k.SendEnabledCoins(ctx, in.Coins...); err != nil {
+			return nil, err
+		}
 	}
 
 	for _, out := range msg.Outputs {

diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go
@@ -69,7 +69,7 @@ func (suite *IntegrationTestSuite) SetupTest() {
 	ctx := app.BaseApp.NewContext(false, abci.Header{})
 
 	app.AccountKeeper.SetParams(ctx, authtypes.DefaultParams())
-	app.BankKeeper.SetSendEnabled(ctx, true)
+	app.BankKeeper.SetParams(ctx, types.DefaultParams())
 
 	suite.app = app
 	suite.ctx = ctx
@@ -454,9 +454,45 @@ func (suite *IntegrationTestSuite) TestBalance() {
 
 func (suite *IntegrationTestSuite) TestSendEnabled() {
 	app, ctx := suite.app, suite.ctx
-	enabled := false
-	app.BankKeeper.SetSendEnabled(ctx, enabled)
-	suite.Require().Equal(enabled, app.BankKeeper.GetSendEnabled(ctx))
+	enabled := true
+	params := types.DefaultParams()
+	suite.Require().Equal(enabled, params.DefaultSendEnabled)
+
+	app.BankKeeper.SetParams(ctx, params)
+
+	bondCoin := sdk.NewCoin(sdk.DefaultBondDenom, sdk.OneInt())
+	fooCoin := sdk.NewCoin("foocoin", sdk.OneInt())
+	barCoin := sdk.NewCoin("barcoin", sdk.OneInt())
+
+	// assert with default (all denom) send enabled both Bar and Bond Denom are enabled
+	suite.Require().Equal(enabled, app.BankKeeper.SendEnabledCoin(ctx, barCoin))
+	suite.Require().Equal(enabled, app.BankKeeper.SendEnabledCoin(ctx, bondCoin))
+
+	// Both coins should be send enabled.
+	err := app.BankKeeper.SendEnabledCoins(ctx, fooCoin, bondCoin)
+	suite.Require().NoError(err)
+
+	// Set default send_enabled to !enabled, add a foodenom that overrides default as enabled
+	params.DefaultSendEnabled = !enabled
+	params = params.SetSendEnabledParam(fooCoin.Denom, enabled)
+	app.BankKeeper.SetParams(ctx, params)
+
+	// Expect our specific override to be enabled, others to be !enabled.
+	suite.Require().Equal(enabled, app.BankKeeper.SendEnabledCoin(ctx, fooCoin))
+	suite.Require().Equal(!enabled, app.BankKeeper.SendEnabledCoin(ctx, barCoin))
+	suite.Require().Equal(!enabled, app.BankKeeper.SendEnabledCoin(ctx, bondCoin))
+
+	// Foo coin should be send enabled.
+	err = app.BankKeeper.SendEnabledCoins(ctx, fooCoin)
+	suite.Require().NoError(err)
+
+	// Expect an error when one coin is not send enabled.
+	err = app.BankKeeper.SendEnabledCoins(ctx, fooCoin, bondCoin)
+	suite.Require().Error(err)
+
+	// Expect an error when all coins are not send enabled.
+	err = app.BankKeeper.SendEnabledCoins(ctx, bondCoin, barCoin)
+	suite.Require().Error(err)
 }
 
 func (suite *IntegrationTestSuite) TestHasBalance() {
@@ -531,7 +567,7 @@ func (suite *IntegrationTestSuite) TestMsgSendEvents() {
 func (suite *IntegrationTestSuite) TestMsgMultiSendEvents() {
 	app, ctx := suite.app, suite.ctx
 
-	app.BankKeeper.SetSendEnabled(ctx, true)
+	app.BankKeeper.SetParams(ctx, types.DefaultParams())
 
 	addr := sdk.AccAddress([]byte("addr1"))
 	addr2 := sdk.AccAddress([]byte("addr2"))

diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go
@@ -24,8 +24,11 @@ type SendKeeper interface {
 	SetBalance(ctx sdk.Context, addr sdk.AccAddress, balance sdk.Coin) error
 	SetBalances(ctx sdk.Context, addr sdk.AccAddress, balances sdk.Coins) error
 
-	GetSendEnabled(ctx sdk.Context) bool
-	SetSendEnabled(ctx sdk.Context, enabled bool)
+	GetParams(ctx sdk.Context) types.Params
+	SetParams(ctx sdk.Context, params types.Params)
+
+	SendEnabledCoin(ctx sdk.Context, coin sdk.Coin) bool
+	SendEnabledCoins(ctx sdk.Context, coins ...sdk.Coin) error
 
 	BlockedAddr(addr sdk.AccAddress) bool
 }
@@ -60,6 +63,17 @@ func NewBaseSendKeeper(
 	}
 }
 
+// GetParams returns the total set of bank parameters.
+func (k BaseSendKeeper) GetParams(ctx sdk.Context) (params types.Params) {
+	k.paramSpace.GetParamSet(ctx, &params)
+	return params
+}
+
+// SetParams sets the total set of bank parameters.
+func (k BaseSendKeeper) SetParams(ctx sdk.Context, params types.Params) {
+	k.paramSpace.SetParamSet(ctx, &params)
+}
+
 // InputOutputCoins performs multi-send functionality. It accepts a series of
 // inputs that correspond to a series of outputs. It returns an error if the
 // inputs and outputs don't lineup or if any single transfer of tokens fails.
@@ -256,16 +270,21 @@ func (k BaseSendKeeper) SetBalance(ctx sdk.Context, addr sdk.AccAddress, balance
 	return nil
 }
 
-// GetSendEnabled returns the current SendEnabled
-func (k BaseSendKeeper) GetSendEnabled(ctx sdk.Context) bool {
-	var enabled bool
-	k.paramSpace.Get(ctx, types.ParamStoreKeySendEnabled, &enabled)
-	return enabled
+// SendEnabledCoins checks the coins provide and returns an ErrSendDisabled if
+// any of the coins are not configured for sending.  Returns nil if sending is enabled
+// for all provided coin
+func (k BaseSendKeeper) SendEnabledCoins(ctx sdk.Context, coins ...sdk.Coin) error {
+	for _, coin := range coins {
+		if !k.SendEnabledCoin(ctx, coin) {
+			return sdkerrors.Wrapf(types.ErrSendDisabled, "%s transfers are currently disabled", coin.Denom)
+		}
+	}
+	return nil
 }
 
-// SetSendEnabled sets the send enabled
-func (k BaseSendKeeper) SetSendEnabled(ctx sdk.Context, enabled bool) {
-	k.paramSpace.Set(ctx, types.ParamStoreKeySendEnabled, &enabled)
+// SendEnabledCoin returns the current SendEnabled status of the provided coin's denom
+func (k BaseSendKeeper) SendEnabledCoin(ctx sdk.Context, coin sdk.Coin) bool {
+	return k.GetParams(ctx).SendEnabledDenom(coin.Denom)
 }
 
 // BlockedAddr checks if a given address is restricted from

diff --git a/x/bank/simulation/genesis.go b/x/bank/simulation/genesis.go
@@ -3,21 +3,36 @@ package simulation
 // DONTCOVER
 
 import (
+	"fmt"
 	"math/rand"
 
+	"github.com/cosmos/cosmos-sdk/codec"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/module"
 	"github.com/cosmos/cosmos-sdk/x/bank/types"
 )
 
-// Simulation parameter constants
-const (
-	SendEnabled = "send_enabled"
-)
+// RandomGenesisDefaultSendParam computes randomized allow all send transfers param for the bank module
+func RandomGenesisDefaultSendParam(r *rand.Rand) bool {
+	// 90% chance of transfers being enable or P(a) = 0.9 for success
+	return r.Int63n(101) <= 90
+}
 
-// GenSendEnabled randomized SendEnabled
-func GenSendEnabled(r *rand.Rand) bool {
-	return r.Int63n(101) <= 95 // 95% chance of transfers being enabled
+// RandomGenesisSendParams randomized Parameters for the bank module
+func RandomGenesisSendParams(r *rand.Rand) types.SendEnabledParams {
+	params := types.DefaultParams()
+	// 90% chance of transfers being DefaultSendEnabled=true or P(a) = 0.9 for success
+	// 50% of the time add an additional denom specific record (P(b) = 0.475 = 0.5 * 0.95)
+	if r.Int63n(101) <= 50 {
+		// set send enabled 95% of the time
+		bondEnabled := r.Int63n(101) <= 95
+		params = params.SetSendEnabledParam(
+			sdk.DefaultBondDenom,
+			bondEnabled)
+	}
+
+	// overall probability of enabled for bond denom is 94.75% (P(a)+P(b) - P(a)*P(b))
+	return params.SendEnabled
 }
 
 // RandomGenesisAccounts returns a slice of account balances. Each account has
@@ -37,16 +52,31 @@ func RandomGenesisBalances(simState *module.SimulationState) []types.Balance {
 
 // RandomizedGenState generates a random GenesisState for bank
 func RandomizedGenState(simState *module.SimulationState) {
-	var sendEnabled bool
+	var sendEnabledParams types.SendEnabledParams
 	simState.AppParams.GetOrGenerate(
-		simState.Cdc, SendEnabled, &sendEnabled, simState.Rand,
-		func(r *rand.Rand) { sendEnabled = GenSendEnabled(r) },
+		simState.Cdc, string(types.KeySendEnabled), &sendEnabledParams, simState.Rand,
+		func(r *rand.Rand) { sendEnabledParams = RandomGenesisSendParams(r) },
+	)
+
+	var defaultSendEnabledParam bool
+	simState.AppParams.GetOrGenerate(
+		simState.Cdc, string(types.KeyDefaultSendEnabled), &defaultSendEnabledParam, simState.Rand,
+		func(r *rand.Rand) { defaultSendEnabledParam = RandomGenesisDefaultSendParam(r) },
 	)
 
 	numAccs := int64(len(simState.Accounts))
 	totalSupply := sdk.NewInt(simState.InitialStake * (numAccs + simState.NumBonded))
 	supply := sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, totalSupply))
 
-	bankGenesis := types.NewGenesisState(sendEnabled, RandomGenesisBalances(simState), supply)
+	bankGenesis := types.GenesisState{
+		Params: types.Params{
+			SendEnabled:        sendEnabledParams,
+			DefaultSendEnabled: defaultSendEnabledParam,
+		},
+		Balances: RandomGenesisBalances(simState),
+		Supply:   supply,
+	}
+
+	fmt.Printf("Selected randomly generated bank parameters:\n%s\n", codec.MustMarshalJSONIndent(simState.Cdc, bankGenesis.Params))
 	simState.GenState[types.ModuleName] = simState.Cdc.MustMarshalJSON(bankGenesis)
 }
diff --git a/x/bank/simulation/operations.go b/x/bank/simulation/operations.go
@@ -59,13 +59,13 @@ func SimulateMsgSend(ak types.AccountKeeper, bk keeper.Keeper) simtypes.Operatio
 		r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context,
 		accs []simtypes.Account, chainID string,
 	) (simtypes.OperationMsg, []simtypes.FutureOperation, error) {
+		simAccount, toSimAcc, coins, skip := randomSendFields(r, ctx, accs, bk, ak)
 
-		if !bk.GetSendEnabled(ctx) {
-			return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgSend, "transfers are not enabled"), nil, nil
+		// Check send_enabled status of each coin denom
+		if err := bk.SendEnabledCoins(ctx, coins...); err != nil {
+			return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgSend, err.Error()), nil, nil
 		}
 
-		simAccount, toSimAcc, coins, skip := randomSendFields(r, ctx, accs, bk, ak)
-
 		if skip {
 			return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgSend, "skip all transfers"), nil, nil
 		}
@@ -130,10 +130,6 @@ func SimulateMsgMultiSend(ak types.AccountKeeper, bk keeper.Keeper) simtypes.Ope
 		accs []simtypes.Account, chainID string,
 	) (simtypes.OperationMsg, []simtypes.FutureOperation, error) {
 
-		if !bk.GetSendEnabled(ctx) {
-			return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgMultiSend, "transfers are not enabled"), nil, nil
-		}
-
 		// random number of inputs/outputs between [1, 3]
 		inputs := make([]types.Input, r.Intn(3)+1)
 		outputs := make([]types.Output, r.Intn(3)+1)
@@ -169,6 +165,11 @@ func SimulateMsgMultiSend(ak types.AccountKeeper, bk keeper.Keeper) simtypes.Ope
 			totalSentCoins = totalSentCoins.Add(coins...)
 		}
 
+		// Check send_enabled status of each sent coin denom
+		if err := bk.SendEnabledCoins(ctx, totalSentCoins...); err != nil {
+			return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgMultiSend, err.Error()), nil, nil
+		}
+
 		for o := range outputs {
 			outAddr, _ := simtypes.RandomAcc(r, accs)
 

diff --git a/x/bank/simulation/params.go b/x/bank/simulation/params.go
@@ -12,15 +12,18 @@ import (
 	"github.com/cosmos/cosmos-sdk/x/bank/types"
 )
 
-const keySendEnabled = "sendenabled"
-
 // ParamChanges defines the parameters that can be modified by param change proposals
 // on the simulation
 func ParamChanges(r *rand.Rand) []simtypes.ParamChange {
 	return []simtypes.ParamChange{
-		simulation.NewSimParamChange(types.ModuleName, keySendEnabled,
+		simulation.NewSimParamChange(types.ModuleName, string(types.KeySendEnabled),
+			func(r *rand.Rand) string {
+				return fmt.Sprintf("%s", types.ModuleCdc.MustMarshalJSON(RandomGenesisSendParams(r)))
+			},
+		),
+		simulation.NewSimParamChange(types.ModuleName, string(types.KeyDefaultSendEnabled),
 			func(r *rand.Rand) string {
-				return fmt.Sprintf("%v", GenSendEnabled(r))
+				return fmt.Sprintf("%v", RandomGenesisDefaultSendParam(r))
 			},
 		),
 	}