Merge pull request cosmos#446 from CosmWasm/send_coins_enabled_414

Check coin sendable status

x/wasm/internal/keeper/keeper.go

@@ -15,10 +15,7 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper"
-	bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper"
-	distributionkeeper "github.com/cosmos/cosmos-sdk/x/distribution/keeper"
 	paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
-	stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
 	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/libs/log"
 )
@@ -56,12 +53,17 @@ type messenger interface {
 	DispatchMsg(ctx sdk.Context, contractAddr sdk.AccAddress, contractIBCPortID string, msg wasmvmtypes.CosmosMsg) (events []sdk.Event, data [][]byte, err error)
 }
 
+type coinTransferrer interface {
+	// TransferCoins sends the coin amounts from the source to the destination with rules applied.
+	TransferCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error
+}
+
 // Keeper will have a reference to Wasmer with it's own data directory.
 type Keeper struct {
 	storeKey         sdk.StoreKey
 	cdc              codec.Marshaler
-	accountKeeper    authkeeper.AccountKeeper
-	bankKeeper       bankkeeper.Keeper
+	accountKeeper    types.AccountKeeper
+	bank             coinTransferrer
 	ChannelKeeper    types.ChannelKeeper
 	portKeeper       types.PortKeeper
 	capabilityKeeper types.CapabilityKeeper
@@ -82,9 +84,9 @@ func NewKeeper(
 	storeKey sdk.StoreKey,
 	paramSpace paramtypes.Subspace,
 	accountKeeper authkeeper.AccountKeeper,
-	bankKeeper bankkeeper.Keeper,
-	stakingKeeper stakingkeeper.Keeper,
-	distKeeper distributionkeeper.Keeper,
+	bankKeeper types.BankKeeper,
+	stakingKeeper types.StakingKeeper,
+	distKeeper types.DistributionKeeper,
 	channelKeeper types.ChannelKeeper,
 	portKeeper types.PortKeeper,
 	capabilityKeeper types.CapabilityKeeper,
@@ -111,7 +113,7 @@ func NewKeeper(
 		cdc:              cdc,
 		wasmer:           wasmer,
 		accountKeeper:    accountKeeper,
-		bankKeeper:       bankKeeper,
+		bank:             NewBankCoinTransferrer(bankKeeper),
 		ChannelKeeper:    channelKeeper,
 		portKeeper:       portKeeper,
 		capabilityKeeper: capabilityKeeper,
@@ -233,13 +235,10 @@ func (k Keeper) instantiate(ctx sdk.Context, codeID uint64, creator, admin sdk.A
 
 	// deposit initial contract funds
 	if !deposit.IsZero() {
-		if k.bankKeeper.BlockedAddr(creator) {
-			return nil, nil, sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "blocked address can not be used")
-		}
-		sdkerr := k.bankKeeper.SendCoins(ctx, creator, contractAddress, deposit)
-		if sdkerr != nil {
-			return nil, nil, sdkerr
+		if err := k.bank.TransferCoins(ctx, creator, contractAddress, deposit); err != nil {
+			return nil, nil, err
 		}
+
 	} else {
 		// create an empty account (so we don't have issues later)
 		// TODO: can we remove this?
@@ -328,13 +327,8 @@ func (k Keeper) Execute(ctx sdk.Context, contractAddress sdk.AccAddress, caller
 
 	// add more funds
 	if !coins.IsZero() {
-		if k.bankKeeper.BlockedAddr(caller) {
-			return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "blocked address can not be used")
-		}
-
-		sdkerr := k.bankKeeper.SendCoins(ctx, caller, contractAddress, coins)
-		if sdkerr != nil {
-			return nil, sdkerr
+		if err := k.bank.TransferCoins(ctx, caller, contractAddress, coins); err != nil {
+			return nil, err
 		}
 	}
 
@@ -1014,18 +1008,18 @@ func addrFromUint64(id uint64) sdk.AccAddress {
 }
 
 // MultipliedGasMeter wraps the GasMeter from context and multiplies all reads by out defined multiplier
-type MultipiedGasMeter struct {
+type MultipliedGasMeter struct {
 	originalMeter sdk.GasMeter
 }
 
-var _ wasmvm.GasMeter = MultipiedGasMeter{}
+var _ wasmvm.GasMeter = MultipliedGasMeter{}
 
-func (m MultipiedGasMeter) GasConsumed() sdk.Gas {
+func (m MultipliedGasMeter) GasConsumed() sdk.Gas {
 	return m.originalMeter.GasConsumed() * GasMultiplier
 }
 
-func gasMeter(ctx sdk.Context) MultipiedGasMeter {
-	return MultipiedGasMeter{
+func gasMeter(ctx sdk.Context) MultipliedGasMeter {
+	return MultipliedGasMeter{
 		originalMeter: ctx.GasMeter(),
 	}
 }
@@ -1034,3 +1028,31 @@ func gasMeter(ctx sdk.Context) MultipiedGasMeter {
 func (k Keeper) Logger(ctx sdk.Context) log.Logger {
 	return ctx.Logger().With("module", fmt.Sprintf("x/%s", types.ModuleName))
 }
+
+// CoinTransferrer replicates the cosmos-sdk behaviour as in
+// https://github.com/cosmos/cosmos-sdk/blob/v0.41.4/x/bank/keeper/msg_server.go#L26
+type CoinTransferrer struct {
+	keeper types.BankKeeper
+}
+
+func NewBankCoinTransferrer(keeper types.BankKeeper) CoinTransferrer {
+	return CoinTransferrer{
+		keeper: keeper,
+	}
+}
+
+// TransferCoins transfers coins from source to destination account when coin send was enabled for them and the recipient
+// is not in the blocked address list.
+func (c CoinTransferrer) TransferCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error {
+	if err := c.keeper.SendEnabledCoins(ctx, amt...); err != nil {
+		return err
+	}
+	if c.keeper.BlockedAddr(fromAddr) {
+		return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "blocked address can not be used")
+	}
+	sdkerr := c.keeper.SendCoins(ctx, fromAddr, toAddr, amt)
+	if sdkerr != nil {
+		return sdkerr
+	}
+	return nil
+}

x/wasm/internal/keeper/keeper_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/CosmWasm/wasmd/x/wasm/internal/keeper/wasmtesting"
 	wasmvm "github.com/CosmWasm/wasmvm"
 	wasmvmtypes "github.com/CosmWasm/wasmvm/types"
+	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	"io/ioutil"
 	"testing"
 	"time"
@@ -515,7 +516,7 @@ func TestExecute(t *testing.T) {
 	// make sure gas is properly deducted from ctx
 	gasAfter := ctx.GasMeter().GasConsumed()
 	if types.EnableGasVerification {
-		require.Equal(t, uint64(0x12121), gasAfter-gasBefore)
+		require.Equal(t, uint64(0x12963), gasAfter-gasBefore)
 	}
 	// ensure bob now exists and got both payments released
 	bobAcct = accKeeper.GetAccount(ctx, bob)
@@ -543,10 +544,11 @@ func TestExecuteWithDeposit(t *testing.T) {
 	)
 
 	specs := map[string]struct {
-		srcActor    sdk.AccAddress
-		beneficiary sdk.AccAddress
-		expError    bool
-		fundAddr    bool
+		srcActor      sdk.AccAddress
+		beneficiary   sdk.AccAddress
+		newBankParams *banktypes.Params
+		expError      bool
+		fundAddr      bool
 	}{
 		"actor with funds": {
 			srcActor:    bob,
@@ -564,6 +566,23 @@ func TestExecuteWithDeposit(t *testing.T) {
 			beneficiary: fred,
 			expError:    true,
 		},
+		"coin transfer with all transfers disabled": {
+			srcActor:      bob,
+			fundAddr:      true,
+			beneficiary:   fred,
+			newBankParams: &banktypes.Params{DefaultSendEnabled: false},
+			expError:      true,
+		},
+		"coin transfer with transfer denom disabled": {
+			srcActor:    bob,
+			fundAddr:    true,
+			beneficiary: fred,
+			newBankParams: &banktypes.Params{
+				DefaultSendEnabled: true,
+				SendEnabled:        []*banktypes.SendEnabled{{Denom: "denom", Enabled: false}},
+			},
+			expError: true,
+		},
 		"blocked address as beneficiary": {
 			srcActor:    bob,
 			fundAddr:    true,
@@ -575,7 +594,9 @@ func TestExecuteWithDeposit(t *testing.T) {
 		t.Run(msg, func(t *testing.T) {
 			ctx, keepers := CreateTestInput(t, false, SupportedFeatures, nil, nil)
 			accKeeper, bankKeeper, keeper := keepers.AccountKeeper, keepers.BankKeeper, keepers.WasmKeeper
-
+			if spec.newBankParams != nil {
+				bankKeeper.SetParams(ctx, *spec.newBankParams)
+			}
 			if spec.fundAddr {
 				fundAccounts(t, ctx, accKeeper, bankKeeper, spec.srcActor, sdk.NewCoins(sdk.NewInt64Coin("denom", 200)))
 			}

x/wasm/internal/keeper/options.go

@@ -17,8 +17,15 @@ func WithWasmEngine(x types.WasmerEngine) Option {
 }
 
 // WithMessageHandler is an optional constructor parameter to set a custom message handler.
-func WithMessageHandler(n messenger) Option {
+func WithMessageHandler(x messenger) Option {
 	return optsFn(func(k *Keeper) {
-		k.messenger = n
+		k.messenger = x
+	})
+}
+
+// WithCoinTransferrer is an optional constructor parameter to set a custom coin transferrer
+func WithCoinTransferrer(x coinTransferrer) Option {
+	return optsFn(func(k *Keeper) {
+		k.bank = x
 	})
 }

x/wasm/internal/keeper/options_test.go

@@ -28,6 +28,12 @@ func TestConstructorOptions(t *testing.T) {
 				assert.IsType(t, k.messenger, &wasmtesting.MockMessageHandler{})
 			},
 		},
+		"coin transferrer": {
+			srcOpt: WithCoinTransferrer(&wasmtesting.MockCoinTransferrer{}),
+			verify: func(k Keeper) {
+				assert.IsType(t, k.bank, &wasmtesting.MockCoinTransferrer{})
+			},
+		},
 	}
 	for name, spec := range specs {
 		t.Run(name, func(t *testing.T) {

x/wasm/internal/keeper/query_plugins.go

@@ -4,14 +4,12 @@ import (
 	"encoding/json"
 	"fmt"
 	"github.com/CosmWasm/wasmd/x/wasm/internal/types"
+	channeltypes "github.com/cosmos/cosmos-sdk/x/ibc/core/04-channel/types"
 
 	wasmvmtypes "github.com/CosmWasm/wasmvm/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
-	bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper"
-	distributionkeeper "github.com/cosmos/cosmos-sdk/x/distribution/keeper"
 	distributiontypes "github.com/cosmos/cosmos-sdk/x/distribution/types"
-	stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 	abci "github.com/tendermint/tendermint/abci/types"
 )
@@ -91,7 +89,7 @@ type QueryPlugins struct {
 	Wasm     func(ctx sdk.Context, request *wasmvmtypes.WasmQuery) ([]byte, error)
 }
 
-func DefaultQueryPlugins(bank bankkeeper.ViewKeeper, staking stakingkeeper.Keeper, distKeeper distributionkeeper.Keeper, channelKeeper types.ChannelKeeper, queryRouter GRPCQueryRouter, wasm *Keeper) QueryPlugins {
+func DefaultQueryPlugins(bank types.BankViewKeeper, staking types.StakingKeeper, distKeeper types.DistributionKeeper, channelKeeper types.ChannelKeeper, queryRouter GRPCQueryRouter, wasm *Keeper) QueryPlugins {
 	return QueryPlugins{
 		Bank:     BankQuerier(bank),
 		Custom:   NoCustomQuerier,
@@ -128,7 +126,7 @@ func (e QueryPlugins) Merge(o *QueryPlugins) QueryPlugins {
 	return e
 }
 
-func BankQuerier(bankKeeper bankkeeper.ViewKeeper) func(ctx sdk.Context, request *wasmvmtypes.BankQuery) ([]byte, error) {
+func BankQuerier(bankKeeper types.BankViewKeeper) func(ctx sdk.Context, request *wasmvmtypes.BankQuery) ([]byte, error) {
 	return func(ctx sdk.Context, request *wasmvmtypes.BankQuery) ([]byte, error) {
 		if request.AllBalances != nil {
 			addr, err := sdk.AccAddressFromBech32(request.AllBalances.Address)
@@ -176,7 +174,7 @@ func IBCQuerier(wasm *Keeper, channelKeeper types.ChannelKeeper) func(ctx sdk.Co
 		if request.ListChannels != nil {
 			portID := request.ListChannels.PortID
 			var channels wasmvmtypes.IBCEndpoints
-			channelKeeper.IterateChannels(ctx, func(ch types.IdentifiedChannel) bool {
+			channelKeeper.IterateChannels(ctx, func(ch channeltypes.IdentifiedChannel) bool {
 				if portID == "" || portID == ch.PortId {
 					newChan := wasmvmtypes.IBCEndpoint{
 						PortID:    ch.PortId,
@@ -243,7 +241,7 @@ func StargateQuerier(queryRouter GRPCQueryRouter) func(ctx sdk.Context, request
 	}
 }
 
-func StakingQuerier(keeper stakingkeeper.Keeper, distKeeper distributionkeeper.Keeper) func(ctx sdk.Context, request *wasmvmtypes.StakingQuery) ([]byte, error) {
+func StakingQuerier(keeper types.StakingKeeper, distKeeper types.DistributionKeeper) func(ctx sdk.Context, request *wasmvmtypes.StakingQuery) ([]byte, error) {
 	return func(ctx sdk.Context, request *wasmvmtypes.StakingQuery) ([]byte, error) {
 		if request.BondedDenom != nil {
 			denom := keeper.BondDenom(ctx)
@@ -308,7 +306,7 @@ func StakingQuerier(keeper stakingkeeper.Keeper, distKeeper distributionkeeper.K
 	}
 }
 
-func sdkToDelegations(ctx sdk.Context, keeper stakingkeeper.Keeper, delegations []stakingtypes.Delegation) (wasmvmtypes.Delegations, error) {
+func sdkToDelegations(ctx sdk.Context, keeper types.StakingKeeper, delegations []stakingtypes.Delegation) (wasmvmtypes.Delegations, error) {
 	result := make([]wasmvmtypes.Delegation, len(delegations))
 	bondDenom := keeper.BondDenom(ctx)
 
@@ -339,7 +337,7 @@ func sdkToDelegations(ctx sdk.Context, keeper stakingkeeper.Keeper, delegations
 	return result, nil
 }
 
-func sdkToFullDelegation(ctx sdk.Context, keeper stakingkeeper.Keeper, distKeeper distributionkeeper.Keeper, delegation stakingtypes.Delegation) (*wasmvmtypes.FullDelegation, error) {
+func sdkToFullDelegation(ctx sdk.Context, keeper types.StakingKeeper, distKeeper types.DistributionKeeper, delegation stakingtypes.Delegation) (*wasmvmtypes.FullDelegation, error) {
 	delAddr, err := sdk.AccAddressFromBech32(delegation.DelegatorAddress)
 	if err != nil {
 		return nil, sdkerrors.Wrap(err, "delegator address")
@@ -387,7 +385,7 @@ func sdkToFullDelegation(ctx sdk.Context, keeper stakingkeeper.Keeper, distKeepe
 
 // FIXME: simplify this enormously when
 // https://github.com/cosmos/cosmos-sdk/issues/7466 is merged
-func getAccumulatedRewards(ctx sdk.Context, distKeeper distributionkeeper.Keeper, delegation stakingtypes.Delegation) ([]wasmvmtypes.Coin, error) {
+func getAccumulatedRewards(ctx sdk.Context, distKeeper types.DistributionKeeper, delegation stakingtypes.Delegation) ([]wasmvmtypes.Coin, error) {
 	// Try to get *delegator* reward info!
 	params := distributiontypes.QueryDelegationRewardsRequest{
 		DelegatorAddress: delegation.DelegatorAddress,

x/wasm/internal/keeper/wasmtesting/coin_transferrer.go

@@ -0,0 +1,14 @@
+package wasmtesting
+
+import sdk "github.com/cosmos/cosmos-sdk/types"
+
+type MockCoinTransferrer struct {
+	TransferCoinsFn func(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error
+}
+
+func (m *MockCoinTransferrer) TransferCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error {
+	if m.TransferCoinsFn == nil {
+		panic("not expected to be called")
+	}
+	return m.TransferCoinsFn(ctx, fromAddr, toAddr, amt)
+}