Enable/disable coin transfers by denom

[denalimarsh]

Summary

Enable/disable coin transfers by denom.

Problem Definition

Transfers of a specific token type cannot be paused/unpaused without disabling all transfers in the bank module. Some token implementations on other chains allow for fine-grain control of an individual token by the designated owner. The feature acts as a safeguard in potential attack scenarios and can increase trust in the system in a variety of situations. The authority to enable/disable transfers of a specific coin type could be vested in a trusted coin owner or subject to governance depending on the use case.

Proposal

Replace the global SendEnabled parameter with a per coin one indexed by denom:

1. Update the bank module's SendKeeper interface

type SendKeeper interface {
    // Current:
    GetSendEnabled(ctx sdk.Context) bool
    SetSendEnabled(ctx sdk.Context, enabled bool)
 
    // Proposed:
    GetSendEnabled(ctx sdk.Context, string denom) bool
    SetSendEnabled(ctx sdk.Context, string denom, enabled bool)
}

2. Update bank module's params to include SendEnabledParams

ParamStoreKeySendEnabledParams = []byte("sendenabledparams")

type Params struct {
	SendEnabledParams  SendEnabledParams  `json:"send_enabled_params" yaml:"send_enabled_params"`
}

func ParamKeyTable() paramtypes.KeyTable {
	return paramtypes.NewKeyTable(
		paramtypes.NewParamSetPair(ParamStoreKeySendEnabledParams, SendEnabledParams{}, validateSendEnabledParams),
	)
}

type SendEnabledParams struct {
	SendEnabled bool `json:"send_enabled,omitempty" yaml:"send_enabled,omitempty"`
}

// ...validateSendEnabledParams(), DefaultSendEnabledParams(), etc.

3. Update bank module handler functions handleMsgSend and handleMsgMultiSend to reject attempted transfers that include a disabled coin

for _, a := range msg.Amount {
	if !k.GetSendEnabled(ctx, a.Denom) {
		return nil, sdkerrors.Wrapf(sdkerrors.ErrSendDisabled, "%s transfers are currently disabled", a.Denom)
	}
}

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[fedekunze]

I'm in favor of this proposal 👍 I can work on this if we all agree to @clevinson @aaronc

[alexanderbez]

@fedekunze I would advise sticking to the current 0.39 milestone and not allow any scope creep.

[iramiller]

For the widest compatibility (especially with regards to existing chain upgrades) I recommend a global send enabled flag in the configuration and use of specific overrides. With this approach users who depend on the current behavior can be updated with no impact. In addition this type of setup means that section of the code base that handles simulation can assert correct behavior for existing configurations as well as the new per-denomination setup.

[iramiller]

For the widest compatibility (especially with regards to existing chain upgrades) I recommend a global send enabled flag in the configuration and use of specific overrides.

The easiest way to implement this seems to be to use an empty string denom rule for the global policy/default.

[iramiller]

For the widest compatibility (especially with regards to existing chain upgrades) I recommend a global send enabled flag in the configuration and use of specific overrides.

The easiest way to implement this seems to be to use an empty string denom rule for the global policy/default.

In the end this was implemented with a proper Params object for the bank module which includes a DefaultSendEnabled flag to control overall behavior in addition to an array of coin specific SendEnabled flags that extend the default. While this increases the size of the PR appreciably, it results in a more polished solution implemented using explicit behavior everywhere.