diff --git a/x/wasm/keeper/keeper.go b/x/wasm/keeper/keeper.go index 26f5359f1c..9aac53954a 100644 --- a/x/wasm/keeper/keeper.go +++ b/x/wasm/keeper/keeper.go @@ -22,7 +22,7 @@ import ( "github.com/cosmos/cosmos-sdk/types/address" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" - vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" + vestingexported "github.com/cosmos/cosmos-sdk/x/auth/vesting/exported" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types" "github.com/tendermint/tendermint/libs/log" @@ -60,8 +60,8 @@ type CoinTransferrer interface { // CoinPruner handles the balances for accounts that are pruned on contract instantiate. // This is an extension point to attach custom logic type CoinPruner interface { - // PruneBalances handle balances for given address - PruneBalances(ctx sdk.Context, contractAddress sdk.AccAddress) error + // PruneBalances handle balances for given account + PruneBalances(ctx sdk.Context, existingAccount authtypes.AccountI) error } // WasmVMResponseHandler is an extension point to handles the response data returned by a contract call. @@ -82,19 +82,6 @@ var defaultAcceptedAccountTypes = map[reflect.Type]struct{}{ reflect.TypeOf(&authtypes.BaseAccount{}): {}, } -// list of account types that are replaced with base accounts. Chains importing wasmd -// can overwrite this list with the WithPruneAccountTypesOnContractInstantiation option. -// -// contains vesting account types that can be created post genesis -var defaultPruneAccountTypes = map[reflect.Type]struct{}{ - reflect.TypeOf(&vestingtypes.DelayedVestingAccount{}): {}, - reflect.TypeOf(&vestingtypes.ContinuousVestingAccount{}): {}, - // intentionally not added: genesis account types - // reflect.TypeOf(&vestingtypes.BaseVestingAccount{}): {}, - // reflect.TypeOf(&vestingtypes.PeriodicVestingAccount{}): {}, - // reflect.TypeOf(&vestingtypes.PermanentLockedAccount{}): {}, -} - // Keeper will have a reference to Wasmer with it's own data directory. type Keeper struct { storeKey sdk.StoreKey @@ -113,7 +100,6 @@ type Keeper struct { gasRegister GasRegister maxQueryStackSize uint32 acceptedAccountTypes map[reflect.Type]struct{} - pruneAccountTypes map[reflect.Type]struct{} coinPruner CoinPruner } @@ -153,7 +139,7 @@ func NewKeeper( wasmVM: wasmer, accountKeeper: accountKeeper, bank: NewBankCoinTransferrer(bankKeeper), - coinPruner: NewCoinBurner(bankKeeper), + coinPruner: NewVestingCoinBurner(bankKeeper), portKeeper: portKeeper, capabilityKeeper: capabilityKeeper, messenger: NewDefaultMessageHandler(router, channelKeeper, capabilityKeeper, bankKeeper, cdc, portSource), @@ -162,7 +148,6 @@ func NewKeeper( gasRegister: NewDefaultWasmGasRegister(), maxQueryStackSize: types.DefaultMaxQueryStackSize, acceptedAccountTypes: defaultAcceptedAccountTypes, - pruneAccountTypes: defaultPruneAccountTypes, } keeper.wasmVMQueryHandler = DefaultQueryPlugins(bankKeeper, stakingKeeper, distKeeper, channelKeeper, queryRouter, keeper) for _, o := range opts { @@ -321,17 +306,15 @@ func (k Keeper) instantiate(ctx sdk.Context, codeID uint64, creator, admin sdk.A if _, accept := k.acceptedAccountTypes[reflect.TypeOf(existingAcct)]; accept { // keep account and balance as it is k.Logger(ctx).Info("instantiate contract with existing account", "address", contractAddress.String()) - } else if _, clear := k.pruneAccountTypes[reflect.TypeOf(existingAcct)]; clear { - k.Logger(ctx).Info("pruning existing account for contract instantiation", "address", contractAddress.String()) + } else { // consider an account in the wasmd namespace spam and overwrite it. + k.Logger(ctx).Info("pruning existing account for contract instantiation", "address", contractAddress.String()) contractAccount := k.accountKeeper.NewAccountWithAddress(ctx, contractAddress) k.accountKeeper.SetAccount(ctx, contractAccount) // also handle balance to not open cases where these accounts are abused and become liquid - if err := k.coinPruner.PruneBalances(ctx, contractAddress); err != nil { + if err := k.coinPruner.PruneBalances(ctx, existingAcct); err != nil { return nil, nil, err } - } else { // unknown account type - return nil, nil, types.ErrAccountExists.Wrapf("refusing to overwrite special account type:: %T", existingAcct) } } else { // create an empty account (so we don't have issues later) @@ -1161,31 +1144,38 @@ func (c BankCoinTransferrer) TransferCoins(parentCtx sdk.Context, fromAddr sdk.A return nil } -var _ CoinPruner = CoinBurner{} +var _ CoinPruner = VestingCoinBurner{} -// CoinBurner default implementation for CoinPruner to burn the coins -type CoinBurner struct { +// VestingCoinBurner default implementation for CoinPruner to burn the coins +type VestingCoinBurner struct { bank types.BankKeeper } -// NewCoinBurner constructor -func NewCoinBurner(bank types.BankKeeper) CoinBurner { +// NewVestingCoinBurner constructor +func NewVestingCoinBurner(bank types.BankKeeper) VestingCoinBurner { if bank == nil { panic("bank keeper must not be nil") } - return CoinBurner{bank: bank} + return VestingCoinBurner{bank: bank} } // PruneBalances burns all coins owned by the account. -func (b CoinBurner) PruneBalances(ctx sdk.Context, address sdk.AccAddress) error { +func (b VestingCoinBurner) PruneBalances(ctx sdk.Context, existingAcc authtypes.AccountI) error { + v, ok := existingAcc.(vestingexported.VestingAccount) + if !ok { + return types.ErrAccountExists.Wrapf("refusing to overwrite special account type:: %T", existingAcc) + } + ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) - if amt := b.bank.GetAllBalances(ctx, address); !amt.IsZero() { - if err := b.bank.SendCoinsFromAccountToModule(ctx, address, types.ModuleName, amt); err != nil { - return sdkerrors.Wrap(err, "prune account balance") - } - if err := b.bank.BurnCoins(ctx, types.ModuleName, amt); err != nil { - return sdkerrors.Wrap(err, "burn account balance") - } + coinsToBurn := sdk.NewCoins() + for _, orig := range v.GetOriginalVesting() { // focus on the coin denoms that were setup originally; getAllBalances has some issues + coinsToBurn = coinsToBurn.Add(b.bank.GetBalance(ctx, existingAcc.GetAddress(), orig.Denom)) + } + if err := b.bank.SendCoinsFromAccountToModule(ctx, existingAcc.GetAddress(), types.ModuleName, coinsToBurn); err != nil { + return sdkerrors.Wrap(err, "prune account balance") + } + if err := b.bank.BurnCoins(ctx, types.ModuleName, coinsToBurn); err != nil { + return sdkerrors.Wrap(err, "burn account balance") } return nil } diff --git a/x/wasm/keeper/keeper_test.go b/x/wasm/keeper/keeper_test.go index 12ea4c8c95..f65ce46c8d 100644 --- a/x/wasm/keeper/keeper_test.go +++ b/x/wasm/keeper/keeper_test.go @@ -11,6 +11,8 @@ import ( "testing" "time" + "github.com/cosmos/cosmos-sdk/x/auth/vesting" + wasmvm "github.com/CosmWasm/wasmvm" wasmvmtypes "github.com/CosmWasm/wasmvm/types" "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" @@ -2197,12 +2199,59 @@ func TestAppendToContractHistory(t *testing.T) { } func TestCoinBurnerPruneBalances(t *testing.T) { - // should not use users gas parentCtx, keepers := CreateTestInput(t, false, AvailableCapabilities) - addr := keepers.Faucet.NewFundedRandomAccount(parentCtx, sdk.NewCoin("stake", sdk.NewInt(1))) - // when - ctx := parentCtx.WithGasMeter(sdk.NewGasMeter(0)) - gotErr := NewCoinBurner(keepers.BankKeeper).PruneBalances(ctx, addr) - require.NoError(t, gotErr) - assert.Empty(t, keepers.BankKeeper.GetAllBalances(parentCtx, addr)) + amts := sdk.NewCoins(sdk.NewInt64Coin("denom", 100)) + senderAddr := keepers.Faucet.NewFundedRandomAccount(parentCtx, amts...) + + // create vesting account + var vestingAddr sdk.AccAddress = rand.Bytes(types.ContractAddrLen) + msgCreateVestingAccount := vestingtypes.NewMsgCreateVestingAccount(senderAddr, vestingAddr, amts, time.Now().Add(time.Minute).Unix(), false) + _, err := vesting.NewMsgServerImpl(keepers.AccountKeeper, keepers.BankKeeper).CreateVestingAccount(sdk.WrapSDKContext(parentCtx), msgCreateVestingAccount) + require.NoError(t, err) + originalVestingAcc := keepers.AccountKeeper.GetAccount(parentCtx, vestingAddr) + require.NotNil(t, originalVestingAcc) + + specs := map[string]struct { + setupAcc func(t *testing.T, ctx sdk.Context) + expBalances sdk.Coins + expErr *sdkerrors.Error + }{ + "vesting account - all removed": { + setupAcc: func(t *testing.T, ctx sdk.Context) {}, + expBalances: sdk.NewCoins(), + }, + "vesting account with other tokens - only original denoms removed": { + setupAcc: func(t *testing.T, ctx sdk.Context) { + keepers.Faucet.Fund(ctx, vestingAddr, sdk.NewCoin("other", sdk.NewInt(2))) + }, + expBalances: sdk.NewCoins(sdk.NewCoin("other", sdk.NewInt(2))), + }, + "non vesting account": { + setupAcc: func(t *testing.T, ctx sdk.Context) { + originalVestingAcc = &authtypes.BaseAccount{Address: originalVestingAcc.GetAddress().String()} + }, + expErr: types.ErrAccountExists, + }, + } + for name, spec := range specs { + t.Run(name, func(t *testing.T) { + ctx, _ := parentCtx.CacheContext() + spec.setupAcc(t, ctx) + // overwrite account as in keeper before calling prune + contractAccount := keepers.AccountKeeper.NewAccountWithAddress(ctx, vestingAddr) + keepers.AccountKeeper.SetAccount(ctx, contractAccount) + + // when + noGasCtx := ctx.WithGasMeter(sdk.NewGasMeter(0)) // should not use callers gas + gotErr := NewVestingCoinBurner(keepers.BankKeeper).PruneBalances(noGasCtx, originalVestingAcc) + // then + if spec.expErr != nil { + require.ErrorIs(t, gotErr, spec.expErr) + return + } + require.NoError(t, gotErr) + assert.Equal(t, spec.expBalances, keepers.BankKeeper.GetAllBalances(ctx, vestingAddr)) + // and no out of gas panic + }) + } } diff --git a/x/wasm/keeper/options.go b/x/wasm/keeper/options.go index e6119b4478..dfa55e5be4 100644 --- a/x/wasm/keeper/options.go +++ b/x/wasm/keeper/options.go @@ -154,19 +154,6 @@ func WithAcceptedAccountTypesOnContractInstantiation(accts ...authtypes.AccountI }) } -// WithPruneAccountTypesOnContractInstantiation sets the account types that should be cleared. Account types of this list -// will be overwritten with the BaseAccount type and their balance burned, when they exist for an address on contract -// instantiation. -// -// Values should be references and contain the `*vestingtypes.DelayedVestingAccount`, `*vestingtypes.ContinuousVestingAccount` -// as post genesis account types with an open address range. -func WithPruneAccountTypesOnContractInstantiation(accts ...authtypes.AccountI) Option { - m := asTypeMap(accts) - return optsFn(func(k *Keeper) { - k.pruneAccountTypes = m - }) -} - func asTypeMap(accts []authtypes.AccountI) map[reflect.Type]struct{} { m := make(map[reflect.Type]struct{}, len(accts)) for _, a := range accts { diff --git a/x/wasm/keeper/options_test.go b/x/wasm/keeper/options_test.go index b4c7f0861e..b4bd2bee0b 100644 --- a/x/wasm/keeper/options_test.go +++ b/x/wasm/keeper/options_test.go @@ -95,20 +95,10 @@ func TestConstructorOptions(t *testing.T) { assert.Equal(t, exp, k.acceptedAccountTypes) }, }, - "prune account types": { - srcOpt: WithPruneAccountTypesOnContractInstantiation(&authtypes.BaseAccount{}, &vestingtypes.ContinuousVestingAccount{}), - verify: func(t *testing.T, k Keeper) { - exp := map[reflect.Type]struct{}{ - reflect.TypeOf(&authtypes.BaseAccount{}): {}, - reflect.TypeOf(&vestingtypes.ContinuousVestingAccount{}): {}, - } - assert.Equal(t, exp, k.pruneAccountTypes) - }, - }, "coin pruner": { - srcOpt: WithCoinPruner(CoinBurner{}), + srcOpt: WithCoinPruner(VestingCoinBurner{}), verify: func(t *testing.T, k Keeper) { - assert.Equal(t, CoinBurner{}, k.coinPruner) + assert.Equal(t, VestingCoinBurner{}, k.coinPruner) }, }, }