From ea9512d02721cfbc65a1ac283800917acfeb34d2 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Mon, 9 Sep 2024 17:31:47 +0700 Subject: [PATCH 01/21] bring send & mint to v2 --- x/bank/v2/keeper/keeper.go | 277 ++++++++++++++++++++++++++++++++++++- 1 file changed, 276 insertions(+), 1 deletion(-) diff --git a/x/bank/v2/keeper/keeper.go b/x/bank/v2/keeper/keeper.go index 88799b4ac18b..20e3363a1b96 100644 --- a/x/bank/v2/keeper/keeper.go +++ b/x/bank/v2/keeper/keeper.go @@ -1,12 +1,21 @@ package keeper import ( + "context" + "cosmossdk.io/collections" + "cosmossdk.io/collections/indexes" "cosmossdk.io/core/address" appmodulev2 "cosmossdk.io/core/appmodule/v2" + "cosmossdk.io/core/event" + "cosmossdk.io/math" "cosmossdk.io/x/bank/v2/types" + errorsmod "cosmossdk.io/errors" "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" ) // Keeper defines the bank/v2 module keeper. @@ -14,20 +23,26 @@ import ( type Keeper struct { appmodulev2.Environment + ak types.AccountKeeper authority []byte addressCodec address.Codec schema collections.Schema params collections.Item[types.Params] + balances *collections.IndexedMap[collections.Pair[sdk.AccAddress, string], math.Int, BalancesIndexes] + supply collections.Map[string, math.Int] } -func NewKeeper(authority []byte, addressCodec address.Codec, env appmodulev2.Environment, cdc codec.BinaryCodec) *Keeper { +func NewKeeper(authority []byte, addressCodec address.Codec, env appmodulev2.Environment, cdc codec.BinaryCodec, ak types.AccountKeeper) *Keeper { sb := collections.NewSchemaBuilder(env.KVStoreService) k := &Keeper{ Environment: env, + ak: ak, authority: authority, addressCodec: addressCodec, // TODO(@julienrbrt): Should we add address codec to the environment? params: collections.NewItem(sb, types.ParamsKey, "params", codec.CollValue[types.Params](cdc)), + balances: collections.NewIndexedMap(sb, types.BalancesPrefix, "balances", collections.PairKeyCodec(sdk.AccAddressKey, collections.StringKey), types.BalanceValueCodec, newBalancesIndexes(sb)), + supply: collections.NewMap(sb, types.SupplyKey, "supply", collections.StringKey, sdk.IntValue), } schema, err := sb.Build() @@ -38,3 +53,263 @@ func NewKeeper(authority []byte, addressCodec address.Codec, env appmodulev2.Env return k } + +// MintCoins creates new coins from thin air and adds it to the module account. +// An error is returned if the module account does not exist or is unauthorized. +func (k Keeper) MintCoins(ctx context.Context, moduleName string, amounts sdk.Coins) error { + // err := k.mintCoinsRestrictionFn(ctx, amounts) + // if err != nil { + // k.Logger.Error(fmt.Sprintf("Module %q attempted to mint coins %s it doesn't have permission for, error %v", moduleName, amounts, err)) + // return err + // } + acc := k.ak.GetModuleAccount(ctx, moduleName) + if acc == nil { + return errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", moduleName) + } + + if !acc.HasPermission(authtypes.Minter) { + return errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "module account %s does not have permissions to mint tokens", moduleName) + } + + if !amounts.IsValid() { + return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, amounts.String()) + } + + err := k.addCoins(ctx, acc.GetAddress(), amounts) + if err != nil { + return err + } + + for _, amount := range amounts { + supply := k.GetSupply(ctx, amount.GetDenom()) + supply = supply.Add(amount) + k.setSupply(ctx, supply) + } + + k.Logger.Debug("minted coins from module account", "amount", amounts.String(), "from", moduleName) + + addrStr, err := k.addressCodec.BytesToString(acc.GetAddress()) + if err != nil { + return err + } + + // emit mint event + return k.EventService.EventManager(ctx).EmitKV( + types.EventTypeCoinMint, + event.NewAttribute(types.AttributeKeyMinter, addrStr), + event.NewAttribute(sdk.AttributeKeyAmount, amounts.String()), + ) +} + +// SendCoins transfers amt coins from a sending account to a receiving account. +// An error is returned upon failure. +func (k Keeper) SendCoins(ctx context.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) error { + if !amt.IsValid() { + return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, amt.String()) + } + + var err error + // TODO: handle send res + // toAddr, err = k.sendRestriction.apply(ctx, fromAddr, toAddr, amt) + // if err != nil { + // return err + // } + + err = k.subUnlockedCoins(ctx, fromAddr, amt) + if err != nil { + return err + } + + err = k.addCoins(ctx, toAddr, amt) + if err != nil { + return err + } + + fromAddrString, err := k.addressCodec.BytesToString(fromAddr) + if err != nil { + return err + } + toAddrString, err := k.addressCodec.BytesToString(toAddr) + if err != nil { + return err + } + + return k.EventService.EventManager(ctx).EmitKV( + types.EventTypeTransfer, + event.NewAttribute(types.AttributeKeyRecipient, toAddrString), + event.NewAttribute(types.AttributeKeySender, fromAddrString), + event.NewAttribute(sdk.AttributeKeyAmount, amt.String()), + ) +} + +// SendCoinsFromModuleToAccount transfers coins from a ModuleAccount to an AccAddress. +// An error is returned if the module account does not exist or if +// the recipient address is black-listed or if sending the tokens fails. +func (k Keeper) SendCoinsFromModuleToAccount( + ctx context.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins, +) error { + senderAddr := k.ak.GetModuleAddress(senderModule) + if senderAddr == nil { + return errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", senderModule) + } + + // TODO: restriction + + return k.SendCoins(ctx, senderAddr, recipientAddr, amt) +} + +// LockedCoins returns all the coins that are not spendable (i.e. locked) for an +// account by address. For standard accounts, the result will always be no coins. +// For vesting accounts, LockedCoins is delegated to the concrete vesting account +// type. +func (k Keeper) LockedCoins(ctx context.Context, addr sdk.AccAddress) sdk.Coins { + acc := k.ak.GetAccount(ctx, addr) + if acc != nil { + vacc, ok := acc.(types.VestingAccount) + if ok { + return vacc.LockedCoins(k.HeaderService.HeaderInfo(ctx).Time) + } + } + + return sdk.NewCoins() +} + +// GetSupply retrieves the Supply from store +func (k Keeper) GetSupply(ctx context.Context, denom string) sdk.Coin { + amt, err := k.supply.Get(ctx, denom) + if err != nil { + return sdk.NewCoin(denom, math.ZeroInt()) + } + return sdk.NewCoin(denom, amt) +} + +// GetBalance returns the balance of a specific denomination for a given account +// by address. +func (k Keeper) GetBalance(ctx context.Context, addr sdk.AccAddress, denom string) sdk.Coin { + amt, err := k.balances.Get(ctx, collections.Join(addr, denom)) + if err != nil { + return sdk.NewCoin(denom, math.ZeroInt()) + } + return sdk.NewCoin(denom, amt) +} + +// subUnlockedCoins removes the unlocked amt coins of the given account. +// An error is returned if the resulting balance is negative. +// +// CONTRACT: The provided amount (amt) must be valid, non-negative coins. +// +// A coin_spent event is emitted after the operation. +func (k Keeper) subUnlockedCoins(ctx context.Context, addr sdk.AccAddress, amt sdk.Coins) error { + lockedCoins := k.LockedCoins(ctx, addr) + + for _, coin := range amt { + balance := k.GetBalance(ctx, addr, coin.Denom) + ok, locked := lockedCoins.Find(coin.Denom) + if !ok { + locked = sdk.Coin{Denom: coin.Denom, Amount: math.ZeroInt()} + } + + spendable, hasNeg := sdk.Coins{balance}.SafeSub(locked) + if hasNeg { + return errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds, + "locked amount exceeds account balance funds: %s > %s", locked, balance) + } + + if _, hasNeg := spendable.SafeSub(coin); hasNeg { + if len(spendable) == 0 { + spendable = sdk.Coins{sdk.Coin{Denom: coin.Denom, Amount: math.ZeroInt()}} + } + return errorsmod.Wrapf( + sdkerrors.ErrInsufficientFunds, + "spendable balance %s is smaller than %s", + spendable, coin, + ) + } + + newBalance := balance.Sub(coin) + + if err := k.setBalance(ctx, addr, newBalance); err != nil { + return err + } + } + + addrStr, err := k.addressCodec.BytesToString(addr) + if err != nil { + return err + } + + return k.EventService.EventManager(ctx).EmitKV( + types.EventTypeCoinSpent, + event.NewAttribute(types.AttributeKeySpender, addrStr), + event.NewAttribute(sdk.AttributeKeyAmount, amt.String()), + ) +} + +// addCoins increases the balance of the given address by the specified amount. +// +// CONTRACT: The provided amount (amt) must be valid, non-negative coins. +// +// It emits a coin_received event after the operation. +func (k Keeper) addCoins(ctx context.Context, addr sdk.AccAddress, amt sdk.Coins) error { + for _, coin := range amt { + balance := k.GetBalance(ctx, addr, coin.Denom) + newBalance := balance.Add(coin) + + err := k.setBalance(ctx, addr, newBalance) + if err != nil { + return err + } + } + + addrStr, err := k.addressCodec.BytesToString(addr) + if err != nil { + return err + } + + return k.EventService.EventManager(ctx).EmitKV( + types.EventTypeCoinReceived, + event.NewAttribute(types.AttributeKeyReceiver, addrStr), + event.NewAttribute(sdk.AttributeKeyAmount, amt.String()), + ) +} + +// setSupply sets the supply for the given coin +func (k Keeper) setSupply(ctx context.Context, coin sdk.Coin) { + // Bank invariants and IBC requires to remove zero coins. + if coin.IsZero() { + _ = k.supply.Remove(ctx, coin.Denom) + } else { + _ = k.supply.Set(ctx, coin.Denom, coin.Amount) + } +} + +// setBalance sets the coin balance for an account by address. +func (k Keeper) setBalance(ctx context.Context, addr sdk.AccAddress, balance sdk.Coin) error { + if !balance.IsValid() { + return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, balance.String()) + } + + // x/bank invariants prohibit persistence of zero balances + if balance.IsZero() { + err := k.balances.Remove(ctx, collections.Join(addr, balance.Denom)) + if err != nil { + return err + } + return nil + } + return k.balances.Set(ctx, collections.Join(addr, balance.Denom), balance.Amount) +} + +func newBalancesIndexes(sb *collections.SchemaBuilder) BalancesIndexes { + return BalancesIndexes{ + Denom: indexes.NewReversePair[math.Int]( + sb, types.DenomAddressPrefix, "address_by_denom_index", + collections.PairKeyCodec(sdk.LengthPrefixedAddressKey(sdk.AccAddressKey), collections.StringKey), //nolint:staticcheck // Note: refer to the LengthPrefixedAddressKey docs to understand why we do this. + indexes.WithReversePairUncheckedValue(), // denom to address indexes were stored as Key: Join(denom, address) Value: []byte{0}, this will migrate the value to []byte{} in a lazy way. + ), + } +} + +type BalancesIndexes struct { + Denom *indexes.ReversePair[sdk.AccAddress, string, math.Int] +} From c3b3700d714bb9d903e09f232045d612f11b4a8e Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Mon, 9 Sep 2024 17:32:25 +0700 Subject: [PATCH 02/21] type --- x/bank/v2/types/events.go | 24 ++++++++++++++++++ x/bank/v2/types/expected_keepers.go | 15 ++++++++++++ x/bank/v2/types/keys.go | 38 ++++++++++++++++++++++++++--- x/bank/v2/types/vesting.go | 32 ++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 3 deletions(-) create mode 100644 x/bank/v2/types/events.go create mode 100644 x/bank/v2/types/expected_keepers.go create mode 100644 x/bank/v2/types/vesting.go diff --git a/x/bank/v2/types/events.go b/x/bank/v2/types/events.go new file mode 100644 index 000000000000..ef63dac90a31 --- /dev/null +++ b/x/bank/v2/types/events.go @@ -0,0 +1,24 @@ +package types + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// bank module event types +const ( + EventTypeTransfer = "transfer" + + AttributeKeyRecipient = "recipient" + AttributeKeySender = sdk.AttributeKeySender + + // supply and balance tracking events name and attributes + EventTypeCoinSpent = "coin_spent" + EventTypeCoinReceived = "coin_received" + EventTypeCoinMint = "coinbase" // NOTE(fdymylja): using mint clashes with mint module event + EventTypeCoinBurn = "burn" + + AttributeKeySpender = "spender" + AttributeKeyReceiver = "receiver" + AttributeKeyMinter = "minter" + AttributeKeyBurner = "burner" +) diff --git a/x/bank/v2/types/expected_keepers.go b/x/bank/v2/types/expected_keepers.go new file mode 100644 index 000000000000..87a033a5e7fb --- /dev/null +++ b/x/bank/v2/types/expected_keepers.go @@ -0,0 +1,15 @@ +package types + +import ( + "context" + + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// AccountKeeper defines the account contract that must be fulfilled when +// creating a x/bank keeper. +type AccountKeeper interface { + GetAccount(ctx context.Context, addr sdk.AccAddress) sdk.AccountI + GetModuleAccount(ctx context.Context, moduleName string) sdk.ModuleAccountI + GetModuleAddress(moduleName string) sdk.AccAddress +} diff --git a/x/bank/v2/types/keys.go b/x/bank/v2/types/keys.go index c4c6dfbc67af..f8546f9b0f53 100644 --- a/x/bank/v2/types/keys.go +++ b/x/bank/v2/types/keys.go @@ -1,6 +1,12 @@ package types -import "cosmossdk.io/collections" +import ( + "cosmossdk.io/collections" + collcodec "cosmossdk.io/collections/codec" + "cosmossdk.io/math" + + sdk "github.com/cosmos/cosmos-sdk/types" +) const ( // ModuleName is the name of the module @@ -8,7 +14,33 @@ const ( // GovModuleName duplicates the gov module's name to avoid a dependency with x/gov. GovModuleName = "gov" + + // MintModuleName duplicates the mint module's name to avoid a cyclic dependency with x/mint. + // It should be synced with the mint module's name if it is ever changed. + // See: https://github.com/cosmos/cosmos-sdk/blob/0e34478eb7420b69869ed50f129fc274a97a9b06/x/mint/types/keys.go#L13 + MintModuleName = "mint" +) + +var ( + // ParamsKey is the prefix for x/bank/v2 parameters + ParamsKey = collections.NewPrefix(2) + + // BalancesPrefix is the prefix for the account balances store. We use a byte + // (instead of `[]byte("balances")` to save some disk space). + BalancesPrefix = collections.NewPrefix(3) + + DenomAddressPrefix = collections.NewPrefix(4) + + SupplyKey = collections.NewPrefix(5) ) -// ParamsKey is the prefix for x/bank/v2 parameters -var ParamsKey = collections.NewPrefix(2) +// BalanceValueCodec is a codec for encoding bank balances in a backwards compatible way. +// Historically, balances were represented as Coin, now they're represented as a simple math.Int +var BalanceValueCodec = collcodec.NewAltValueCodec(sdk.IntValue, func(bytes []byte) (math.Int, error) { + c := new(sdk.Coin) + err := c.Unmarshal(bytes) + if err != nil { + return math.Int{}, err + } + return c.Amount, nil +}) diff --git a/x/bank/v2/types/vesting.go b/x/bank/v2/types/vesting.go new file mode 100644 index 000000000000..a288d4ec81da --- /dev/null +++ b/x/bank/v2/types/vesting.go @@ -0,0 +1,32 @@ +package types + +import ( + "time" + + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// VestingAccount defines an interface used for account vesting. +type VestingAccount interface { + // LockedCoins returns the set of coins that are not spendable (i.e. locked), + // defined as the vesting coins that are not delegated. + // + // To get spendable coins of a vesting account, first the total balance must + // be retrieved and the locked tokens can be subtracted from the total balance. + // Note, the spendable balance can be negative. + LockedCoins(blockTime time.Time) sdk.Coins + + // TrackDelegation performs internal vesting accounting necessary when + // delegating from a vesting account. It accepts the current block time, the + // delegation amount and balance of all coins whose denomination exists in + // the account's original vesting balance. + TrackDelegation(blockTime time.Time, balance, amount sdk.Coins) + + // TrackUndelegation performs internal vesting accounting necessary when a + // vesting account performs an undelegation. + TrackUndelegation(amount sdk.Coins) + + GetOriginalVesting() sdk.Coins + GetDelegatedFree() sdk.Coins + GetDelegatedVesting() sdk.Coins +} From 94e32d682d734a72955a2a6a00d1edc773efff52 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Mon, 9 Sep 2024 17:33:43 +0700 Subject: [PATCH 03/21] add account keeper --- x/bank/v2/depinject.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/bank/v2/depinject.go b/x/bank/v2/depinject.go index b5465d7e02df..a7df5a89afc0 100644 --- a/x/bank/v2/depinject.go +++ b/x/bank/v2/depinject.go @@ -32,6 +32,7 @@ type ModuleInputs struct { Cdc codec.Codec Environment appmodule.Environment AddressCodec address.Codec + AccountKeeper types.AccountKeeper } type ModuleOutputs struct { @@ -53,7 +54,7 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { } } - k := keeper.NewKeeper(authority, in.AddressCodec, in.Environment, in.Cdc) + k := keeper.NewKeeper(authority, in.AddressCodec, in.Environment, in.Cdc, in.AccountKeeper) m := NewAppModule(in.Cdc, k) return ModuleOutputs{ From 360ae0c96e5a0a72b2e19524241155afbc709247 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Mon, 9 Sep 2024 17:34:00 +0700 Subject: [PATCH 04/21] test & helper --- x/bank/v2/keeper/keeper_test.go | 155 +++++++++++++ x/bank/v2/testutil/expected_keepers_mocks.go | 218 +++++++++++++++++++ x/bank/v2/testutil/helpers.go | 25 +++ 3 files changed, 398 insertions(+) create mode 100644 x/bank/v2/keeper/keeper_test.go create mode 100644 x/bank/v2/testutil/expected_keepers_mocks.go create mode 100644 x/bank/v2/testutil/helpers.go diff --git a/x/bank/v2/keeper/keeper_test.go b/x/bank/v2/keeper/keeper_test.go new file mode 100644 index 000000000000..dd8ded256386 --- /dev/null +++ b/x/bank/v2/keeper/keeper_test.go @@ -0,0 +1,155 @@ +package keeper_test + +import ( + "context" + "testing" + "time" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/suite" + + "cosmossdk.io/core/header" + coretesting "cosmossdk.io/core/testing" + "cosmossdk.io/math" + storetypes "cosmossdk.io/store/types" + banktypes "cosmossdk.io/x/bank/types" + "cosmossdk.io/x/bank/v2/keeper" + banktestutil "cosmossdk.io/x/bank/v2/testutil" + + codectestutil "github.com/cosmos/cosmos-sdk/codec/testutil" + "github.com/cosmos/cosmos-sdk/runtime" + "github.com/cosmos/cosmos-sdk/testutil" + sdk "github.com/cosmos/cosmos-sdk/types" + moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +const ( + fooDenom = "foo" + barDenom = "bar" + ibcPath = "transfer/channel-0" + ibcBaseDenom = "farboo" + metaDataDescription = "IBC Token from %s" + initialPower = int64(100) + holder = "holder" + multiPerm = "multiple permissions account" + randomPerm = "random permission" +) + +var ( + holderAcc = authtypes.NewEmptyModuleAccount(holder) + randomAcc = authtypes.NewEmptyModuleAccount(randomPerm) + burnerAcc = authtypes.NewEmptyModuleAccount(authtypes.Burner, authtypes.Burner, authtypes.Staking) + minterAcc = authtypes.NewEmptyModuleAccount(authtypes.Minter, authtypes.Minter) + mintAcc = authtypes.NewEmptyModuleAccount(banktypes.MintModuleName, authtypes.Minter) + multiPermAcc = authtypes.NewEmptyModuleAccount(multiPerm, authtypes.Burner, authtypes.Minter, authtypes.Staking) + + baseAcc = authtypes.NewBaseAccountWithAddress(sdk.AccAddress([]byte("baseAcc"))) + + accAddrs = []sdk.AccAddress{ + sdk.AccAddress([]byte("addr1_______________")), + sdk.AccAddress([]byte("addr2_______________")), + sdk.AccAddress([]byte("addr3_______________")), + sdk.AccAddress([]byte("addr4_______________")), + sdk.AccAddress([]byte("addr5_______________")), + } + + // The default power validators are initialized to have within tests + initTokens = sdk.TokensFromConsensusPower(initialPower, sdk.DefaultPowerReduction) + initCoins = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, initTokens)) +) + +func newFooCoin(amt int64) sdk.Coin { + return sdk.NewInt64Coin(fooDenom, amt) +} + +func newBarCoin(amt int64) sdk.Coin { + return sdk.NewInt64Coin(barDenom, amt) +} + +func (suite *KeeperTestSuite) mockMintCoins(moduleAcc *authtypes.ModuleAccount) { + suite.authKeeper.EXPECT().GetModuleAccount(suite.ctx, moduleAcc.Name).Return(moduleAcc) +} + +func (suite *KeeperTestSuite) mockSendCoinsFromModuleToAccount(moduleAcc *authtypes.ModuleAccount, _ sdk.AccAddress) { + suite.authKeeper.EXPECT().GetModuleAddress(moduleAcc.Name).Return(moduleAcc.GetAddress()) + suite.authKeeper.EXPECT().GetAccount(suite.ctx, moduleAcc.GetAddress()).Return(moduleAcc) +} + +func (suite *KeeperTestSuite) mockFundAccount(receiver sdk.AccAddress) { + suite.mockMintCoins(mintAcc) + suite.mockSendCoinsFromModuleToAccount(mintAcc, receiver) +} + +func (suite *KeeperTestSuite) mockSendCoins(ctx context.Context, sender sdk.AccountI, _ sdk.AccAddress) { + suite.authKeeper.EXPECT().GetAccount(ctx, sender.GetAddress()).Return(sender) +} + +type KeeperTestSuite struct { + suite.Suite + + ctx context.Context + bankKeeper keeper.Keeper + authKeeper *banktestutil.MockAccountKeeper +} + +func TestKeeperTestSuite(t *testing.T) { + suite.Run(t, new(KeeperTestSuite)) +} + +func (suite *KeeperTestSuite) SetupTest() { + key := storetypes.NewKVStoreKey(banktypes.StoreKey) + testCtx := testutil.DefaultContextWithDB(suite.T(), key, storetypes.NewTransientStoreKey("transient_test")) + ctx := testCtx.Ctx.WithHeaderInfo(header.Info{Time: time.Now()}) + encCfg := moduletestutil.MakeTestEncodingConfig(codectestutil.CodecOptions{}) + + env := runtime.NewEnvironment(runtime.NewKVStoreService(key), coretesting.NewNopLogger()) + + ac := codectestutil.CodecOptions{}.GetAddressCodec() + authority, err := ac.BytesToString(authtypes.NewModuleAddress("gov")) + suite.Require().NoError(err) + + // gomock initializations + ctrl := gomock.NewController(suite.T()) + authKeeper := banktestutil.NewMockAccountKeeper(ctrl) + suite.ctx = ctx + suite.authKeeper = authKeeper + suite.bankKeeper = *keeper.NewKeeper( + []byte(authority), + ac, + env, + encCfg.Codec, + authKeeper, + ) +} + +func (suite *KeeperTestSuite) TestSendCoins() { + ctx := suite.ctx + require := suite.Require() + balances := sdk.NewCoins(newFooCoin(100), newBarCoin(50)) + sendAmt := sdk.NewCoins(newFooCoin(10), newBarCoin(10)) + + acc0 := authtypes.NewBaseAccountWithAddress(accAddrs[0]) + + // Try send with empty balances + suite.authKeeper.EXPECT().GetAccount(suite.ctx, accAddrs[0]).Return(acc0) + err := suite.bankKeeper.SendCoins(ctx, accAddrs[0], accAddrs[1], sendAmt) + require.Error(err) + + // Set balances for acc0 and then try send to acc1 + suite.mockFundAccount(accAddrs[0]) + require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, accAddrs[0], balances)) + suite.mockSendCoins(ctx, acc0, accAddrs[1]) + require.NoError(suite.bankKeeper.SendCoins(ctx, accAddrs[0], accAddrs[1], sendAmt)) + + // Check balances + acc0FooBalance := suite.bankKeeper.GetBalance(ctx, accAddrs[0], fooDenom) + require.Equal(acc0FooBalance.Amount, math.NewInt(90)) + acc0BarBalance := suite.bankKeeper.GetBalance(ctx, accAddrs[0], barDenom) + require.Equal(acc0BarBalance.Amount, math.NewInt(40)) + acc1FooBalance := suite.bankKeeper.GetBalance(ctx, accAddrs[1], fooDenom) + require.Equal(acc1FooBalance.Amount, math.NewInt(10)) + acc1BarBalance := suite.bankKeeper.GetBalance(ctx, accAddrs[1], barDenom) + require.Equal(acc1BarBalance.Amount, math.NewInt(10)) + +} diff --git a/x/bank/v2/testutil/expected_keepers_mocks.go b/x/bank/v2/testutil/expected_keepers_mocks.go new file mode 100644 index 000000000000..aeddace241f3 --- /dev/null +++ b/x/bank/v2/testutil/expected_keepers_mocks.go @@ -0,0 +1,218 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: x/bank/types/expected_keepers.go + +// Package testutil is a generated GoMock package. +package testutil + +import ( + context "context" + reflect "reflect" + + address "cosmossdk.io/core/address" + types "github.com/cosmos/cosmos-sdk/types" + types0 "github.com/cosmos/cosmos-sdk/x/auth/types" + gomock "github.com/golang/mock/gomock" +) + +// MockAccountKeeper is a mock of AccountKeeper interface. +type MockAccountKeeper struct { + ctrl *gomock.Controller + recorder *MockAccountKeeperMockRecorder +} + +// MockAccountKeeperMockRecorder is the mock recorder for MockAccountKeeper. +type MockAccountKeeperMockRecorder struct { + mock *MockAccountKeeper +} + +// NewMockAccountKeeper creates a new mock instance. +func NewMockAccountKeeper(ctrl *gomock.Controller) *MockAccountKeeper { + mock := &MockAccountKeeper{ctrl: ctrl} + mock.recorder = &MockAccountKeeperMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockAccountKeeper) EXPECT() *MockAccountKeeperMockRecorder { + return m.recorder +} + +// AddressCodec mocks base method. +func (m *MockAccountKeeper) AddressCodec() address.Codec { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AddressCodec") + ret0, _ := ret[0].(address.Codec) + return ret0 +} + +// AddressCodec indicates an expected call of AddressCodec. +func (mr *MockAccountKeeperMockRecorder) AddressCodec() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddressCodec", reflect.TypeOf((*MockAccountKeeper)(nil).AddressCodec)) +} + +// GetAccount mocks base method. +func (m *MockAccountKeeper) GetAccount(ctx context.Context, addr types.AccAddress) types.AccountI { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAccount", ctx, addr) + ret0, _ := ret[0].(types.AccountI) + return ret0 +} + +// GetAccount indicates an expected call of GetAccount. +func (mr *MockAccountKeeperMockRecorder) GetAccount(ctx, addr interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAccount", reflect.TypeOf((*MockAccountKeeper)(nil).GetAccount), ctx, addr) +} + +// GetModuleAccount mocks base method. +func (m *MockAccountKeeper) GetModuleAccount(ctx context.Context, moduleName string) types.ModuleAccountI { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetModuleAccount", ctx, moduleName) + ret0, _ := ret[0].(types.ModuleAccountI) + return ret0 +} + +// GetModuleAccount indicates an expected call of GetModuleAccount. +func (mr *MockAccountKeeperMockRecorder) GetModuleAccount(ctx, moduleName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetModuleAccount", reflect.TypeOf((*MockAccountKeeper)(nil).GetModuleAccount), ctx, moduleName) +} + +// GetModuleAccountAndPermissions mocks base method. +func (m *MockAccountKeeper) GetModuleAccountAndPermissions(ctx context.Context, moduleName string) (types.ModuleAccountI, []string) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetModuleAccountAndPermissions", ctx, moduleName) + ret0, _ := ret[0].(types.ModuleAccountI) + ret1, _ := ret[1].([]string) + return ret0, ret1 +} + +// GetModuleAccountAndPermissions indicates an expected call of GetModuleAccountAndPermissions. +func (mr *MockAccountKeeperMockRecorder) GetModuleAccountAndPermissions(ctx, moduleName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetModuleAccountAndPermissions", reflect.TypeOf((*MockAccountKeeper)(nil).GetModuleAccountAndPermissions), ctx, moduleName) +} + +// GetModuleAddress mocks base method. +func (m *MockAccountKeeper) GetModuleAddress(moduleName string) types.AccAddress { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetModuleAddress", moduleName) + ret0, _ := ret[0].(types.AccAddress) + return ret0 +} + +// GetModuleAddress indicates an expected call of GetModuleAddress. +func (mr *MockAccountKeeperMockRecorder) GetModuleAddress(moduleName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetModuleAddress", reflect.TypeOf((*MockAccountKeeper)(nil).GetModuleAddress), moduleName) +} + +// GetModuleAddressAndPermissions mocks base method. +func (m *MockAccountKeeper) GetModuleAddressAndPermissions(moduleName string) (types.AccAddress, []string) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetModuleAddressAndPermissions", moduleName) + ret0, _ := ret[0].(types.AccAddress) + ret1, _ := ret[1].([]string) + return ret0, ret1 +} + +// GetModuleAddressAndPermissions indicates an expected call of GetModuleAddressAndPermissions. +func (mr *MockAccountKeeperMockRecorder) GetModuleAddressAndPermissions(moduleName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetModuleAddressAndPermissions", reflect.TypeOf((*MockAccountKeeper)(nil).GetModuleAddressAndPermissions), moduleName) +} + +// GetModulePermissions mocks base method. +func (m *MockAccountKeeper) GetModulePermissions() map[string]types0.PermissionsForAddress { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetModulePermissions") + ret0, _ := ret[0].(map[string]types0.PermissionsForAddress) + return ret0 +} + +// GetModulePermissions indicates an expected call of GetModulePermissions. +func (mr *MockAccountKeeperMockRecorder) GetModulePermissions() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetModulePermissions", reflect.TypeOf((*MockAccountKeeper)(nil).GetModulePermissions)) +} + +// HasAccount mocks base method. +func (m *MockAccountKeeper) HasAccount(ctx context.Context, addr types.AccAddress) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "HasAccount", ctx, addr) + ret0, _ := ret[0].(bool) + return ret0 +} + +// HasAccount indicates an expected call of HasAccount. +func (mr *MockAccountKeeperMockRecorder) HasAccount(ctx, addr interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "HasAccount", reflect.TypeOf((*MockAccountKeeper)(nil).HasAccount), ctx, addr) +} + +// NewAccount mocks base method. +func (m *MockAccountKeeper) NewAccount(arg0 context.Context, arg1 types.AccountI) types.AccountI { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "NewAccount", arg0, arg1) + ret0, _ := ret[0].(types.AccountI) + return ret0 +} + +// NewAccount indicates an expected call of NewAccount. +func (mr *MockAccountKeeperMockRecorder) NewAccount(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NewAccount", reflect.TypeOf((*MockAccountKeeper)(nil).NewAccount), arg0, arg1) +} + +// NewAccountWithAddress mocks base method. +func (m *MockAccountKeeper) NewAccountWithAddress(ctx context.Context, addr types.AccAddress) types.AccountI { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "NewAccountWithAddress", ctx, addr) + ret0, _ := ret[0].(types.AccountI) + return ret0 +} + +// NewAccountWithAddress indicates an expected call of NewAccountWithAddress. +func (mr *MockAccountKeeperMockRecorder) NewAccountWithAddress(ctx, addr interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NewAccountWithAddress", reflect.TypeOf((*MockAccountKeeper)(nil).NewAccountWithAddress), ctx, addr) +} + +// SetAccount mocks base method. +func (m *MockAccountKeeper) SetAccount(ctx context.Context, acc types.AccountI) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetAccount", ctx, acc) +} + +// SetAccount indicates an expected call of SetAccount. +func (mr *MockAccountKeeperMockRecorder) SetAccount(ctx, acc interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetAccount", reflect.TypeOf((*MockAccountKeeper)(nil).SetAccount), ctx, acc) +} + +// SetModuleAccount mocks base method. +func (m *MockAccountKeeper) SetModuleAccount(ctx context.Context, macc types.ModuleAccountI) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetModuleAccount", ctx, macc) +} + +// SetModuleAccount indicates an expected call of SetModuleAccount. +func (mr *MockAccountKeeperMockRecorder) SetModuleAccount(ctx, macc interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetModuleAccount", reflect.TypeOf((*MockAccountKeeper)(nil).SetModuleAccount), ctx, macc) +} + +// ValidatePermissions mocks base method. +func (m *MockAccountKeeper) ValidatePermissions(macc types.ModuleAccountI) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ValidatePermissions", macc) + ret0, _ := ret[0].(error) + return ret0 +} + +// ValidatePermissions indicates an expected call of ValidatePermissions. +func (mr *MockAccountKeeperMockRecorder) ValidatePermissions(macc interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ValidatePermissions", reflect.TypeOf((*MockAccountKeeper)(nil).ValidatePermissions), macc) +} diff --git a/x/bank/v2/testutil/helpers.go b/x/bank/v2/testutil/helpers.go new file mode 100644 index 000000000000..879b3d00d9f8 --- /dev/null +++ b/x/bank/v2/testutil/helpers.go @@ -0,0 +1,25 @@ +package testutil + +import ( + "context" + + bankkeeper "cosmossdk.io/x/bank/v2/keeper" + "cosmossdk.io/x/bank/v2/types" + + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// FundAccount is a utility function that funds an account by minting and +// sending the coins to the address. This should be used for testing purposes +// only! +// +// TODO: Instead of using the mint module account, which has the +// permission of minting, create a "faucet" account. (@fdymylja) +func FundAccount(ctx context.Context, bankKeeper bankkeeper.Keeper, addr sdk.AccAddress, amounts sdk.Coins) error { + if err := bankKeeper.MintCoins(ctx, types.MintModuleName, amounts); err != nil { + return err + } + + return bankKeeper.SendCoinsFromModuleToAccount(ctx, types.MintModuleName, addr, amounts) +} + From d469cf71798aa2b128c01ad72da0ba1e5dff617f Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Mon, 9 Sep 2024 17:55:36 +0700 Subject: [PATCH 05/21] todo --- x/bank/v2/keeper/keeper.go | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/x/bank/v2/keeper/keeper.go b/x/bank/v2/keeper/keeper.go index 20e3363a1b96..54143afe8d70 100644 --- a/x/bank/v2/keeper/keeper.go +++ b/x/bank/v2/keeper/keeper.go @@ -57,11 +57,8 @@ func NewKeeper(authority []byte, addressCodec address.Codec, env appmodulev2.Env // MintCoins creates new coins from thin air and adds it to the module account. // An error is returned if the module account does not exist or is unauthorized. func (k Keeper) MintCoins(ctx context.Context, moduleName string, amounts sdk.Coins) error { - // err := k.mintCoinsRestrictionFn(ctx, amounts) - // if err != nil { - // k.Logger.Error(fmt.Sprintf("Module %q attempted to mint coins %s it doesn't have permission for, error %v", moduleName, amounts, err)) - // return err - // } + + // TODO: Mint restriction acc := k.ak.GetModuleAccount(ctx, moduleName) if acc == nil { return errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", moduleName) @@ -109,11 +106,8 @@ func (k Keeper) SendCoins(ctx context.Context, fromAddr, toAddr sdk.AccAddress, } var err error - // TODO: handle send res - // toAddr, err = k.sendRestriction.apply(ctx, fromAddr, toAddr, amt) - // if err != nil { - // return err - // } + + // TODO: Send restriction err = k.subUnlockedCoins(ctx, fromAddr, amt) if err != nil { From f472be14a31ee90bca003868c85d4c008533db57 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Mon, 9 Sep 2024 20:32:30 +0700 Subject: [PATCH 06/21] add store key --- x/bank/v2/types/keys.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/x/bank/v2/types/keys.go b/x/bank/v2/types/keys.go index f8546f9b0f53..b235b1a0b821 100644 --- a/x/bank/v2/types/keys.go +++ b/x/bank/v2/types/keys.go @@ -12,6 +12,9 @@ const ( // ModuleName is the name of the module ModuleName = "bankv2" + // StoreKey defines the primary module store key + StoreKey = ModuleName + // GovModuleName duplicates the gov module's name to avoid a dependency with x/gov. GovModuleName = "gov" From 9df7cb62c9fd64e2a238eacbf46addf2848b4b93 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Mon, 9 Sep 2024 20:35:00 +0700 Subject: [PATCH 07/21] remove vesting acc & wrap all send into one --- x/bank/v2/keeper/keeper.go | 71 ++++++++++++++------------------------ 1 file changed, 26 insertions(+), 45 deletions(-) diff --git a/x/bank/v2/keeper/keeper.go b/x/bank/v2/keeper/keeper.go index 54143afe8d70..ab84d084d88d 100644 --- a/x/bank/v2/keeper/keeper.go +++ b/x/bank/v2/keeper/keeper.go @@ -2,6 +2,7 @@ package keeper import ( "context" + "fmt" "cosmossdk.io/collections" "cosmossdk.io/collections/indexes" @@ -99,13 +100,35 @@ func (k Keeper) MintCoins(ctx context.Context, moduleName string, amounts sdk.Co } // SendCoins transfers amt coins from a sending account to a receiving account. +// Function take sender & receipient as string. +// They can be sdk address or module name. // An error is returned upon failure. -func (k Keeper) SendCoins(ctx context.Context, fromAddr, toAddr sdk.AccAddress, amt sdk.Coins) error { +func (k Keeper) SendCoins(ctx context.Context, from, to string, amt sdk.Coins) error { if !amt.IsValid() { return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, amt.String()) } + var fromAddr, toAddr sdk.AccAddress var err error + + // Detect from & to is address format or module name + fromAddr, err = sdk.AccAddressFromBech32(from) + if err != nil { + // Check if is a module name + fromAddr = k.ak.GetModuleAddress(from) + if fromAddr == nil { + return fmt.Errorf("%s is not an address or module name", from) + } + } + + toAddr, err = sdk.AccAddressFromBech32(to) + if err != nil { + // Check if is a module name + toAddr = k.ak.GetModuleAddress(to) + if toAddr == nil { + return fmt.Errorf("%s is not an address or module name", to) + } + } // TODO: Send restriction @@ -136,38 +159,6 @@ func (k Keeper) SendCoins(ctx context.Context, fromAddr, toAddr sdk.AccAddress, ) } -// SendCoinsFromModuleToAccount transfers coins from a ModuleAccount to an AccAddress. -// An error is returned if the module account does not exist or if -// the recipient address is black-listed or if sending the tokens fails. -func (k Keeper) SendCoinsFromModuleToAccount( - ctx context.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins, -) error { - senderAddr := k.ak.GetModuleAddress(senderModule) - if senderAddr == nil { - return errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", senderModule) - } - - // TODO: restriction - - return k.SendCoins(ctx, senderAddr, recipientAddr, amt) -} - -// LockedCoins returns all the coins that are not spendable (i.e. locked) for an -// account by address. For standard accounts, the result will always be no coins. -// For vesting accounts, LockedCoins is delegated to the concrete vesting account -// type. -func (k Keeper) LockedCoins(ctx context.Context, addr sdk.AccAddress) sdk.Coins { - acc := k.ak.GetAccount(ctx, addr) - if acc != nil { - vacc, ok := acc.(types.VestingAccount) - if ok { - return vacc.LockedCoins(k.HeaderService.HeaderInfo(ctx).Time) - } - } - - return sdk.NewCoins() -} - // GetSupply retrieves the Supply from store func (k Keeper) GetSupply(ctx context.Context, denom string) sdk.Coin { amt, err := k.supply.Get(ctx, denom) @@ -194,22 +185,12 @@ func (k Keeper) GetBalance(ctx context.Context, addr sdk.AccAddress, denom strin // // A coin_spent event is emitted after the operation. func (k Keeper) subUnlockedCoins(ctx context.Context, addr sdk.AccAddress, amt sdk.Coins) error { - lockedCoins := k.LockedCoins(ctx, addr) - for _, coin := range amt { balance := k.GetBalance(ctx, addr, coin.Denom) - ok, locked := lockedCoins.Find(coin.Denom) - if !ok { - locked = sdk.Coin{Denom: coin.Denom, Amount: math.ZeroInt()} - } + spendable := sdk.Coins{balance} - spendable, hasNeg := sdk.Coins{balance}.SafeSub(locked) + _, hasNeg := spendable.SafeSub(coin) if hasNeg { - return errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds, - "locked amount exceeds account balance funds: %s > %s", locked, balance) - } - - if _, hasNeg := spendable.SafeSub(coin); hasNeg { if len(spendable) == 0 { spendable = sdk.Coins{sdk.Coin{Denom: coin.Denom, Amount: math.ZeroInt()}} } From 2ac927587ee096fa17030824f73320fb9bc5cdab Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Mon, 9 Sep 2024 20:35:19 +0700 Subject: [PATCH 08/21] update tests --- x/bank/v2/keeper/keeper_test.go | 90 +++++++++++++++++++++++++++------ x/bank/v2/testutil/helpers.go | 7 +-- 2 files changed, 76 insertions(+), 21 deletions(-) diff --git a/x/bank/v2/keeper/keeper_test.go b/x/bank/v2/keeper/keeper_test.go index dd8ded256386..17b494c3e495 100644 --- a/x/bank/v2/keeper/keeper_test.go +++ b/x/bank/v2/keeper/keeper_test.go @@ -8,13 +8,14 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/suite" + "cosmossdk.io/core/address" "cosmossdk.io/core/header" coretesting "cosmossdk.io/core/testing" "cosmossdk.io/math" storetypes "cosmossdk.io/store/types" - banktypes "cosmossdk.io/x/bank/types" "cosmossdk.io/x/bank/v2/keeper" banktestutil "cosmossdk.io/x/bank/v2/testutil" + banktypes "cosmossdk.io/x/bank/v2/types" codectestutil "github.com/cosmos/cosmos-sdk/codec/testutil" "github.com/cosmos/cosmos-sdk/runtime" @@ -73,7 +74,6 @@ func (suite *KeeperTestSuite) mockMintCoins(moduleAcc *authtypes.ModuleAccount) func (suite *KeeperTestSuite) mockSendCoinsFromModuleToAccount(moduleAcc *authtypes.ModuleAccount, _ sdk.AccAddress) { suite.authKeeper.EXPECT().GetModuleAddress(moduleAcc.Name).Return(moduleAcc.GetAddress()) - suite.authKeeper.EXPECT().GetAccount(suite.ctx, moduleAcc.GetAddress()).Return(moduleAcc) } func (suite *KeeperTestSuite) mockFundAccount(receiver sdk.AccAddress) { @@ -81,16 +81,13 @@ func (suite *KeeperTestSuite) mockFundAccount(receiver sdk.AccAddress) { suite.mockSendCoinsFromModuleToAccount(mintAcc, receiver) } -func (suite *KeeperTestSuite) mockSendCoins(ctx context.Context, sender sdk.AccountI, _ sdk.AccAddress) { - suite.authKeeper.EXPECT().GetAccount(ctx, sender.GetAddress()).Return(sender) -} - type KeeperTestSuite struct { suite.Suite - ctx context.Context - bankKeeper keeper.Keeper - authKeeper *banktestutil.MockAccountKeeper + ctx context.Context + bankKeeper keeper.Keeper + authKeeper *banktestutil.MockAccountKeeper + addressCodec address.Codec } func TestKeeperTestSuite(t *testing.T) { @@ -121,26 +118,26 @@ func (suite *KeeperTestSuite) SetupTest() { encCfg.Codec, authKeeper, ) + suite.addressCodec = ac } -func (suite *KeeperTestSuite) TestSendCoins() { +func (suite *KeeperTestSuite) TestSendCoins_Acount_To_Account() { ctx := suite.ctx require := suite.Require() balances := sdk.NewCoins(newFooCoin(100), newBarCoin(50)) sendAmt := sdk.NewCoins(newFooCoin(10), newBarCoin(10)) - acc0 := authtypes.NewBaseAccountWithAddress(accAddrs[0]) + acc0Str, _ := suite.addressCodec.BytesToString(accAddrs[0]) + acc1Str, _ := suite.addressCodec.BytesToString(accAddrs[1]) // Try send with empty balances - suite.authKeeper.EXPECT().GetAccount(suite.ctx, accAddrs[0]).Return(acc0) - err := suite.bankKeeper.SendCoins(ctx, accAddrs[0], accAddrs[1], sendAmt) + err := suite.bankKeeper.SendCoins(ctx, acc0Str, acc1Str, sendAmt) require.Error(err) // Set balances for acc0 and then try send to acc1 suite.mockFundAccount(accAddrs[0]) - require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, accAddrs[0], balances)) - suite.mockSendCoins(ctx, acc0, accAddrs[1]) - require.NoError(suite.bankKeeper.SendCoins(ctx, accAddrs[0], accAddrs[1], sendAmt)) + require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, acc0Str, balances)) + require.NoError(suite.bankKeeper.SendCoins(ctx, acc0Str, acc1Str, sendAmt)) // Check balances acc0FooBalance := suite.bankKeeper.GetBalance(ctx, accAddrs[0], fooDenom) @@ -151,5 +148,66 @@ func (suite *KeeperTestSuite) TestSendCoins() { require.Equal(acc1FooBalance.Amount, math.NewInt(10)) acc1BarBalance := suite.bankKeeper.GetBalance(ctx, accAddrs[1], barDenom) require.Equal(acc1BarBalance.Amount, math.NewInt(10)) +} + +func (suite *KeeperTestSuite) TestSendCoins_Module_To_Account() { + ctx := suite.ctx + require := suite.Require() + balances := sdk.NewCoins(newFooCoin(100), newBarCoin(50)) + + acc4Str, _ := suite.addressCodec.BytesToString(accAddrs[4]) + + suite.mockMintCoins(mintAcc) + require.NoError(suite.bankKeeper.MintCoins(ctx, banktypes.MintModuleName, balances)) + + // Try send from invalid module + suite.authKeeper.EXPECT().GetModuleAddress("invalid").Return(nil) + err := suite.bankKeeper.SendCoins(ctx, "invalid", acc4Str, balances) + require.Error(err) + + // Send from mint module + suite.authKeeper.EXPECT().GetModuleAddress(mintAcc.Name).Return(mintAcc.GetAddress()) + err = suite.bankKeeper.SendCoins(ctx, banktypes.MintModuleName, acc4Str, balances) + require.NoError(err) + // Check balances + acc4FooBalance := suite.bankKeeper.GetBalance(ctx, accAddrs[4], fooDenom) + require.Equal(acc4FooBalance.Amount, math.NewInt(100)) + acc4BarBalance := suite.bankKeeper.GetBalance(ctx, accAddrs[4], barDenom) + require.Equal(acc4BarBalance.Amount, math.NewInt(50)) + mintFooBalance := suite.bankKeeper.GetBalance(ctx, mintAcc.GetAddress(), fooDenom) + require.Equal(mintFooBalance.Amount, math.NewInt(0)) + mintBarBalance := suite.bankKeeper.GetBalance(ctx, mintAcc.GetAddress(), barDenom) + require.Equal(mintBarBalance.Amount, math.NewInt(0)) +} + +func (suite *KeeperTestSuite) TestSendCoins_Module_To_Module() { + ctx := suite.ctx + require := suite.Require() + balances := sdk.NewCoins(newFooCoin(100), newBarCoin(50)) + + suite.mockMintCoins(mintAcc) + require.NoError(suite.bankKeeper.MintCoins(ctx, banktypes.MintModuleName, balances)) + + // Try send to invalid module + suite.authKeeper.EXPECT().GetModuleAddress(mintAcc.Name).Return(mintAcc.GetAddress()) + suite.authKeeper.EXPECT().GetModuleAddress("invalid").Return(nil) + err := suite.bankKeeper.SendCoins(ctx, mintAcc.Name, "invalid", balances) + require.Error(err) + + // Send from mint module to burn module + suite.authKeeper.EXPECT().GetModuleAddress(mintAcc.Name).Return(mintAcc.GetAddress()) + suite.authKeeper.EXPECT().GetModuleAddress(burnerAcc.Name).Return(burnerAcc.GetAddress()) + err = suite.bankKeeper.SendCoins(ctx, mintAcc.Name, burnerAcc.Name, balances) + require.NoError(err) + + // Check balances + burnerFooBalance := suite.bankKeeper.GetBalance(ctx, burnerAcc.GetAddress(), fooDenom) + require.Equal(burnerFooBalance.Amount, math.NewInt(100)) + burnerBarBalance := suite.bankKeeper.GetBalance(ctx, burnerAcc.GetAddress(), barDenom) + require.Equal(burnerBarBalance.Amount, math.NewInt(50)) + mintFooBalance := suite.bankKeeper.GetBalance(ctx, mintAcc.GetAddress(), fooDenom) + require.Equal(mintFooBalance.Amount, math.NewInt(0)) + mintBarBalance := suite.bankKeeper.GetBalance(ctx, mintAcc.GetAddress(), barDenom) + require.Equal(mintBarBalance.Amount, math.NewInt(0)) } diff --git a/x/bank/v2/testutil/helpers.go b/x/bank/v2/testutil/helpers.go index 879b3d00d9f8..8e9deed14a33 100644 --- a/x/bank/v2/testutil/helpers.go +++ b/x/bank/v2/testutil/helpers.go @@ -12,14 +12,11 @@ import ( // FundAccount is a utility function that funds an account by minting and // sending the coins to the address. This should be used for testing purposes // only! -// -// TODO: Instead of using the mint module account, which has the -// permission of minting, create a "faucet" account. (@fdymylja) -func FundAccount(ctx context.Context, bankKeeper bankkeeper.Keeper, addr sdk.AccAddress, amounts sdk.Coins) error { +func FundAccount(ctx context.Context, bankKeeper bankkeeper.Keeper, addr string, amounts sdk.Coins) error { if err := bankKeeper.MintCoins(ctx, types.MintModuleName, amounts); err != nil { return err } - return bankKeeper.SendCoinsFromModuleToAccount(ctx, types.MintModuleName, addr, amounts) + return bankKeeper.SendCoins(ctx, types.MintModuleName, addr, amounts) } From 660e601b74090bafefc618cf2fca736c47831631 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Mon, 9 Sep 2024 20:37:57 +0700 Subject: [PATCH 09/21] rename --- x/bank/v2/depinject.go | 4 ++-- x/bank/v2/keeper/keeper.go | 8 ++++---- x/bank/v2/types/expected_keepers.go | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/x/bank/v2/depinject.go b/x/bank/v2/depinject.go index a7df5a89afc0..2ba0128bc40d 100644 --- a/x/bank/v2/depinject.go +++ b/x/bank/v2/depinject.go @@ -32,7 +32,7 @@ type ModuleInputs struct { Cdc codec.Codec Environment appmodule.Environment AddressCodec address.Codec - AccountKeeper types.AccountKeeper + AuthKeeper types.AuthKeeper } type ModuleOutputs struct { @@ -54,7 +54,7 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { } } - k := keeper.NewKeeper(authority, in.AddressCodec, in.Environment, in.Cdc, in.AccountKeeper) + k := keeper.NewKeeper(authority, in.AddressCodec, in.Environment, in.Cdc, in.AuthKeeper) m := NewAppModule(in.Cdc, k) return ModuleOutputs{ diff --git a/x/bank/v2/keeper/keeper.go b/x/bank/v2/keeper/keeper.go index ab84d084d88d..8f2dcbf8c644 100644 --- a/x/bank/v2/keeper/keeper.go +++ b/x/bank/v2/keeper/keeper.go @@ -24,7 +24,7 @@ import ( type Keeper struct { appmodulev2.Environment - ak types.AccountKeeper + ak types.AuthKeeper authority []byte addressCodec address.Codec schema collections.Schema @@ -33,7 +33,7 @@ type Keeper struct { supply collections.Map[string, math.Int] } -func NewKeeper(authority []byte, addressCodec address.Codec, env appmodulev2.Environment, cdc codec.BinaryCodec, ak types.AccountKeeper) *Keeper { +func NewKeeper(authority []byte, addressCodec address.Codec, env appmodulev2.Environment, cdc codec.BinaryCodec, ak types.AuthKeeper) *Keeper { sb := collections.NewSchemaBuilder(env.KVStoreService) k := &Keeper{ @@ -58,7 +58,7 @@ func NewKeeper(authority []byte, addressCodec address.Codec, env appmodulev2.Env // MintCoins creates new coins from thin air and adds it to the module account. // An error is returned if the module account does not exist or is unauthorized. func (k Keeper) MintCoins(ctx context.Context, moduleName string, amounts sdk.Coins) error { - + // TODO: Mint restriction acc := k.ak.GetModuleAccount(ctx, moduleName) if acc == nil { @@ -129,7 +129,7 @@ func (k Keeper) SendCoins(ctx context.Context, from, to string, amt sdk.Coins) e return fmt.Errorf("%s is not an address or module name", to) } } - + // TODO: Send restriction err = k.subUnlockedCoins(ctx, fromAddr, amt) diff --git a/x/bank/v2/types/expected_keepers.go b/x/bank/v2/types/expected_keepers.go index 87a033a5e7fb..aecd62fd57d3 100644 --- a/x/bank/v2/types/expected_keepers.go +++ b/x/bank/v2/types/expected_keepers.go @@ -6,9 +6,9 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) -// AccountKeeper defines the account contract that must be fulfilled when +// AuthKeeper defines the account contract that must be fulfilled when // creating a x/bank keeper. -type AccountKeeper interface { +type AuthKeeper interface { GetAccount(ctx context.Context, addr sdk.AccAddress) sdk.AccountI GetModuleAccount(ctx context.Context, moduleName string) sdk.ModuleAccountI GetModuleAddress(moduleName string) sdk.AccAddress From c5e327f84b0124ed89785ce7dd9ed00f973aca90 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Mon, 9 Sep 2024 21:11:04 +0700 Subject: [PATCH 10/21] unused --- x/bank/v2/types/vesting.go | 32 -------------------------------- 1 file changed, 32 deletions(-) delete mode 100644 x/bank/v2/types/vesting.go diff --git a/x/bank/v2/types/vesting.go b/x/bank/v2/types/vesting.go deleted file mode 100644 index a288d4ec81da..000000000000 --- a/x/bank/v2/types/vesting.go +++ /dev/null @@ -1,32 +0,0 @@ -package types - -import ( - "time" - - sdk "github.com/cosmos/cosmos-sdk/types" -) - -// VestingAccount defines an interface used for account vesting. -type VestingAccount interface { - // LockedCoins returns the set of coins that are not spendable (i.e. locked), - // defined as the vesting coins that are not delegated. - // - // To get spendable coins of a vesting account, first the total balance must - // be retrieved and the locked tokens can be subtracted from the total balance. - // Note, the spendable balance can be negative. - LockedCoins(blockTime time.Time) sdk.Coins - - // TrackDelegation performs internal vesting accounting necessary when - // delegating from a vesting account. It accepts the current block time, the - // delegation amount and balance of all coins whose denomination exists in - // the account's original vesting balance. - TrackDelegation(blockTime time.Time, balance, amount sdk.Coins) - - // TrackUndelegation performs internal vesting accounting necessary when a - // vesting account performs an undelegation. - TrackUndelegation(amount sdk.Coins) - - GetOriginalVesting() sdk.Coins - GetDelegatedFree() sdk.Coins - GetDelegatedVesting() sdk.Coins -} From 279ddd2c3a2cabd25e3fb3eabd390fb07be40588 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Tue, 10 Sep 2024 15:55:01 +0700 Subject: [PATCH 11/21] take input as bytes --- x/bank/v2/keeper/keeper.go | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/x/bank/v2/keeper/keeper.go b/x/bank/v2/keeper/keeper.go index 8f2dcbf8c644..ff4e46988f99 100644 --- a/x/bank/v2/keeper/keeper.go +++ b/x/bank/v2/keeper/keeper.go @@ -2,7 +2,6 @@ package keeper import ( "context" - "fmt" "cosmossdk.io/collections" "cosmossdk.io/collections/indexes" @@ -103,50 +102,42 @@ func (k Keeper) MintCoins(ctx context.Context, moduleName string, amounts sdk.Co // Function take sender & receipient as string. // They can be sdk address or module name. // An error is returned upon failure. -func (k Keeper) SendCoins(ctx context.Context, from, to string, amt sdk.Coins) error { +func (k Keeper) SendCoins(ctx context.Context, from, to []byte, amt sdk.Coins) error { if !amt.IsValid() { return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, amt.String()) } - var fromAddr, toAddr sdk.AccAddress var err error - // Detect from & to is address format or module name - fromAddr, err = sdk.AccAddressFromBech32(from) - if err != nil { - // Check if is a module name - fromAddr = k.ak.GetModuleAddress(from) - if fromAddr == nil { - return fmt.Errorf("%s is not an address or module name", from) - } + // If from & to is []byte of module name + // Override with module address + fromAddr := k.ak.GetModuleAddress(string(from)) + if fromAddr != nil { + from = fromAddr } - toAddr, err = sdk.AccAddressFromBech32(to) - if err != nil { - // Check if is a module name - toAddr = k.ak.GetModuleAddress(to) - if toAddr == nil { - return fmt.Errorf("%s is not an address or module name", to) - } + toAddr := k.ak.GetModuleAddress(string(to)) + if toAddr != nil { + to = toAddr } // TODO: Send restriction - err = k.subUnlockedCoins(ctx, fromAddr, amt) + err = k.subUnlockedCoins(ctx, from, amt) if err != nil { return err } - err = k.addCoins(ctx, toAddr, amt) + err = k.addCoins(ctx, to, amt) if err != nil { return err } - fromAddrString, err := k.addressCodec.BytesToString(fromAddr) + fromAddrString, err := k.addressCodec.BytesToString(from) if err != nil { return err } - toAddrString, err := k.addressCodec.BytesToString(toAddr) + toAddrString, err := k.addressCodec.BytesToString(to) if err != nil { return err } From 392207f367c891756189735634651cb13c2e3232 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Tue, 10 Sep 2024 15:55:10 +0700 Subject: [PATCH 12/21] update tests --- x/bank/v2/keeper/keeper_test.go | 36 +++++++++++++++++++++------------ x/bank/v2/testutil/helpers.go | 5 ++--- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/x/bank/v2/keeper/keeper_test.go b/x/bank/v2/keeper/keeper_test.go index 17b494c3e495..92fde14676ec 100644 --- a/x/bank/v2/keeper/keeper_test.go +++ b/x/bank/v2/keeper/keeper_test.go @@ -72,7 +72,7 @@ func (suite *KeeperTestSuite) mockMintCoins(moduleAcc *authtypes.ModuleAccount) suite.authKeeper.EXPECT().GetModuleAccount(suite.ctx, moduleAcc.Name).Return(moduleAcc) } -func (suite *KeeperTestSuite) mockSendCoinsFromModuleToAccount(moduleAcc *authtypes.ModuleAccount, _ sdk.AccAddress) { +func (suite *KeeperTestSuite) mockSendCoinsFromModuleToAccount(moduleAcc *authtypes.ModuleAccount, recv sdk.AccAddress) { suite.authKeeper.EXPECT().GetModuleAddress(moduleAcc.Name).Return(moduleAcc.GetAddress()) } @@ -127,17 +127,20 @@ func (suite *KeeperTestSuite) TestSendCoins_Acount_To_Account() { balances := sdk.NewCoins(newFooCoin(100), newBarCoin(50)) sendAmt := sdk.NewCoins(newFooCoin(10), newBarCoin(10)) - acc0Str, _ := suite.addressCodec.BytesToString(accAddrs[0]) - acc1Str, _ := suite.addressCodec.BytesToString(accAddrs[1]) - // Try send with empty balances - err := suite.bankKeeper.SendCoins(ctx, acc0Str, acc1Str, sendAmt) + suite.authKeeper.EXPECT().GetModuleAddress(string(accAddrs[0])).Return(nil) + suite.authKeeper.EXPECT().GetModuleAddress(string(accAddrs[1])).Return(nil) + err := suite.bankKeeper.SendCoins(ctx, accAddrs[0], accAddrs[1], sendAmt) require.Error(err) // Set balances for acc0 and then try send to acc1 suite.mockFundAccount(accAddrs[0]) - require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, acc0Str, balances)) - require.NoError(suite.bankKeeper.SendCoins(ctx, acc0Str, acc1Str, sendAmt)) + suite.authKeeper.EXPECT().GetModuleAddress(string(accAddrs[0])).Return(nil) + require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, accAddrs[0], balances)) + + suite.authKeeper.EXPECT().GetModuleAddress(string(accAddrs[0])).Return(nil) + suite.authKeeper.EXPECT().GetModuleAddress(string(accAddrs[1])).Return(nil) + require.NoError(suite.bankKeeper.SendCoins(ctx, accAddrs[0], accAddrs[1], sendAmt)) // Check balances acc0FooBalance := suite.bankKeeper.GetBalance(ctx, accAddrs[0], fooDenom) @@ -162,12 +165,14 @@ func (suite *KeeperTestSuite) TestSendCoins_Module_To_Account() { // Try send from invalid module suite.authKeeper.EXPECT().GetModuleAddress("invalid").Return(nil) - err := suite.bankKeeper.SendCoins(ctx, "invalid", acc4Str, balances) + suite.authKeeper.EXPECT().GetModuleAddress(acc4Str).Return(nil) + err := suite.bankKeeper.SendCoins(ctx, []byte("invalid"), []byte(acc4Str), balances) require.Error(err) // Send from mint module suite.authKeeper.EXPECT().GetModuleAddress(mintAcc.Name).Return(mintAcc.GetAddress()) - err = suite.bankKeeper.SendCoins(ctx, banktypes.MintModuleName, acc4Str, balances) + suite.authKeeper.EXPECT().GetModuleAddress(string(accAddrs[4])).Return(nil) + err = suite.bankKeeper.SendCoins(ctx, []byte(banktypes.MintModuleName), accAddrs[4], balances) require.NoError(err) // Check balances @@ -184,21 +189,22 @@ func (suite *KeeperTestSuite) TestSendCoins_Module_To_Account() { func (suite *KeeperTestSuite) TestSendCoins_Module_To_Module() { ctx := suite.ctx require := suite.Require() - balances := sdk.NewCoins(newFooCoin(100), newBarCoin(50)) + balances := sdk.NewCoins(newFooCoin(200), newBarCoin(100)) suite.mockMintCoins(mintAcc) require.NoError(suite.bankKeeper.MintCoins(ctx, banktypes.MintModuleName, balances)) // Try send to invalid module + // In this case it will create a new account suite.authKeeper.EXPECT().GetModuleAddress(mintAcc.Name).Return(mintAcc.GetAddress()) suite.authKeeper.EXPECT().GetModuleAddress("invalid").Return(nil) - err := suite.bankKeeper.SendCoins(ctx, mintAcc.Name, "invalid", balances) - require.Error(err) + err := suite.bankKeeper.SendCoins(ctx, []byte(mintAcc.Name), []byte("invalid"), sdk.NewCoins(newFooCoin(100), newBarCoin(50))) + require.NoError(err) // Send from mint module to burn module suite.authKeeper.EXPECT().GetModuleAddress(mintAcc.Name).Return(mintAcc.GetAddress()) suite.authKeeper.EXPECT().GetModuleAddress(burnerAcc.Name).Return(burnerAcc.GetAddress()) - err = suite.bankKeeper.SendCoins(ctx, mintAcc.Name, burnerAcc.Name, balances) + err = suite.bankKeeper.SendCoins(ctx, []byte(mintAcc.Name), []byte(burnerAcc.Name), sdk.NewCoins(newFooCoin(100), newBarCoin(50))) require.NoError(err) // Check balances @@ -206,6 +212,10 @@ func (suite *KeeperTestSuite) TestSendCoins_Module_To_Module() { require.Equal(burnerFooBalance.Amount, math.NewInt(100)) burnerBarBalance := suite.bankKeeper.GetBalance(ctx, burnerAcc.GetAddress(), barDenom) require.Equal(burnerBarBalance.Amount, math.NewInt(50)) + invalidFooBalance := suite.bankKeeper.GetBalance(ctx, []byte("invalid"), fooDenom) + require.Equal(invalidFooBalance.Amount, math.NewInt(100)) + invalidBarBalance := suite.bankKeeper.GetBalance(ctx, []byte("invalid"), barDenom) + require.Equal(invalidBarBalance.Amount, math.NewInt(50)) mintFooBalance := suite.bankKeeper.GetBalance(ctx, mintAcc.GetAddress(), fooDenom) require.Equal(mintFooBalance.Amount, math.NewInt(0)) mintBarBalance := suite.bankKeeper.GetBalance(ctx, mintAcc.GetAddress(), barDenom) diff --git a/x/bank/v2/testutil/helpers.go b/x/bank/v2/testutil/helpers.go index 8e9deed14a33..37be81009095 100644 --- a/x/bank/v2/testutil/helpers.go +++ b/x/bank/v2/testutil/helpers.go @@ -12,11 +12,10 @@ import ( // FundAccount is a utility function that funds an account by minting and // sending the coins to the address. This should be used for testing purposes // only! -func FundAccount(ctx context.Context, bankKeeper bankkeeper.Keeper, addr string, amounts sdk.Coins) error { +func FundAccount(ctx context.Context, bankKeeper bankkeeper.Keeper, addr []byte, amounts sdk.Coins) error { if err := bankKeeper.MintCoins(ctx, types.MintModuleName, amounts); err != nil { return err } - return bankKeeper.SendCoins(ctx, types.MintModuleName, addr, amounts) + return bankKeeper.SendCoins(ctx, []byte(types.MintModuleName), addr, amounts) } - From 92e3631f2e7dc1f9e5b8bf5c1cde4ea8da527224 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Wed, 11 Sep 2024 19:48:18 +0700 Subject: [PATCH 13/21] simplify send --- x/bank/v2/keeper/keeper.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/x/bank/v2/keeper/keeper.go b/x/bank/v2/keeper/keeper.go index ff4e46988f99..433c90503342 100644 --- a/x/bank/v2/keeper/keeper.go +++ b/x/bank/v2/keeper/keeper.go @@ -108,19 +108,6 @@ func (k Keeper) SendCoins(ctx context.Context, from, to []byte, amt sdk.Coins) e } var err error - - // If from & to is []byte of module name - // Override with module address - fromAddr := k.ak.GetModuleAddress(string(from)) - if fromAddr != nil { - from = fromAddr - } - - toAddr := k.ak.GetModuleAddress(string(to)) - if toAddr != nil { - to = toAddr - } - // TODO: Send restriction err = k.subUnlockedCoins(ctx, from, amt) From afe58f77cc8066c2ace469cfd6504b8bcbd762e2 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Wed, 11 Sep 2024 19:48:29 +0700 Subject: [PATCH 14/21] tests --- x/bank/v2/keeper/keeper_test.go | 90 ++++++++++++++------------------- x/bank/v2/testutil/helpers.go | 6 +-- 2 files changed, 42 insertions(+), 54 deletions(-) diff --git a/x/bank/v2/keeper/keeper_test.go b/x/bank/v2/keeper/keeper_test.go index 92fde14676ec..6beb48062410 100644 --- a/x/bank/v2/keeper/keeper_test.go +++ b/x/bank/v2/keeper/keeper_test.go @@ -26,26 +26,13 @@ import ( ) const ( - fooDenom = "foo" - barDenom = "bar" - ibcPath = "transfer/channel-0" - ibcBaseDenom = "farboo" - metaDataDescription = "IBC Token from %s" - initialPower = int64(100) - holder = "holder" - multiPerm = "multiple permissions account" - randomPerm = "random permission" + fooDenom = "foo" + barDenom = "bar" ) var ( - holderAcc = authtypes.NewEmptyModuleAccount(holder) - randomAcc = authtypes.NewEmptyModuleAccount(randomPerm) - burnerAcc = authtypes.NewEmptyModuleAccount(authtypes.Burner, authtypes.Burner, authtypes.Staking) - minterAcc = authtypes.NewEmptyModuleAccount(authtypes.Minter, authtypes.Minter) - mintAcc = authtypes.NewEmptyModuleAccount(banktypes.MintModuleName, authtypes.Minter) - multiPermAcc = authtypes.NewEmptyModuleAccount(multiPerm, authtypes.Burner, authtypes.Minter, authtypes.Staking) - - baseAcc = authtypes.NewBaseAccountWithAddress(sdk.AccAddress([]byte("baseAcc"))) + burnerAcc = authtypes.NewEmptyModuleAccount(authtypes.Burner, authtypes.Burner, authtypes.Staking) + mintAcc = authtypes.NewEmptyModuleAccount(banktypes.MintModuleName, authtypes.Minter) accAddrs = []sdk.AccAddress{ sdk.AccAddress([]byte("addr1_______________")), @@ -54,10 +41,6 @@ var ( sdk.AccAddress([]byte("addr4_______________")), sdk.AccAddress([]byte("addr5_______________")), } - - // The default power validators are initialized to have within tests - initTokens = sdk.TokensFromConsensusPower(initialPower, sdk.DefaultPowerReduction) - initCoins = sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, initTokens)) ) func newFooCoin(amt int64) sdk.Coin { @@ -128,18 +111,12 @@ func (suite *KeeperTestSuite) TestSendCoins_Acount_To_Account() { sendAmt := sdk.NewCoins(newFooCoin(10), newBarCoin(10)) // Try send with empty balances - suite.authKeeper.EXPECT().GetModuleAddress(string(accAddrs[0])).Return(nil) - suite.authKeeper.EXPECT().GetModuleAddress(string(accAddrs[1])).Return(nil) err := suite.bankKeeper.SendCoins(ctx, accAddrs[0], accAddrs[1], sendAmt) require.Error(err) // Set balances for acc0 and then try send to acc1 suite.mockFundAccount(accAddrs[0]) - suite.authKeeper.EXPECT().GetModuleAddress(string(accAddrs[0])).Return(nil) - require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, accAddrs[0], balances)) - - suite.authKeeper.EXPECT().GetModuleAddress(string(accAddrs[0])).Return(nil) - suite.authKeeper.EXPECT().GetModuleAddress(string(accAddrs[1])).Return(nil) + require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, suite.authKeeper, accAddrs[0], balances)) require.NoError(suite.bankKeeper.SendCoins(ctx, accAddrs[0], accAddrs[1], sendAmt)) // Check balances @@ -153,26 +130,46 @@ func (suite *KeeperTestSuite) TestSendCoins_Acount_To_Account() { require.Equal(acc1BarBalance.Amount, math.NewInt(10)) } -func (suite *KeeperTestSuite) TestSendCoins_Module_To_Account() { +func (suite *KeeperTestSuite) TestSendCoins_Acount_To_Module() { ctx := suite.ctx require := suite.Require() balances := sdk.NewCoins(newFooCoin(100), newBarCoin(50)) + sendAmt := sdk.NewCoins(newFooCoin(10), newBarCoin(10)) + + // Try send with empty balances + err := suite.bankKeeper.SendCoins(ctx, accAddrs[0], burnerAcc.GetAddress(), sendAmt) + require.Error(err) - acc4Str, _ := suite.addressCodec.BytesToString(accAddrs[4]) + // Set balances for acc0 and then try send to acc1 + suite.mockFundAccount(accAddrs[0]) + require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, suite.authKeeper, accAddrs[0], balances)) + require.NoError(suite.bankKeeper.SendCoins(ctx, accAddrs[0], burnerAcc.GetAddress(), sendAmt)) + + // Check balances + acc0FooBalance := suite.bankKeeper.GetBalance(ctx, accAddrs[0], fooDenom) + require.Equal(acc0FooBalance.Amount, math.NewInt(90)) + acc0BarBalance := suite.bankKeeper.GetBalance(ctx, accAddrs[0], barDenom) + require.Equal(acc0BarBalance.Amount, math.NewInt(40)) + burnerFooBalance := suite.bankKeeper.GetBalance(ctx, burnerAcc.GetAddress(), fooDenom) + require.Equal(burnerFooBalance.Amount, math.NewInt(10)) + burnerBarBalance := suite.bankKeeper.GetBalance(ctx, burnerAcc.GetAddress(), barDenom) + require.Equal(burnerBarBalance.Amount, math.NewInt(10)) +} + +func (suite *KeeperTestSuite) TestSendCoins_Module_To_Account() { + ctx := suite.ctx + require := suite.Require() + balances := sdk.NewCoins(newFooCoin(100), newBarCoin(50)) suite.mockMintCoins(mintAcc) require.NoError(suite.bankKeeper.MintCoins(ctx, banktypes.MintModuleName, balances)) - // Try send from invalid module - suite.authKeeper.EXPECT().GetModuleAddress("invalid").Return(nil) - suite.authKeeper.EXPECT().GetModuleAddress(acc4Str).Return(nil) - err := suite.bankKeeper.SendCoins(ctx, []byte("invalid"), []byte(acc4Str), balances) + // Try send from burner module + err := suite.bankKeeper.SendCoins(ctx, burnerAcc.GetAddress(), accAddrs[4], balances) require.Error(err) // Send from mint module - suite.authKeeper.EXPECT().GetModuleAddress(mintAcc.Name).Return(mintAcc.GetAddress()) - suite.authKeeper.EXPECT().GetModuleAddress(string(accAddrs[4])).Return(nil) - err = suite.bankKeeper.SendCoins(ctx, []byte(banktypes.MintModuleName), accAddrs[4], balances) + err = suite.bankKeeper.SendCoins(ctx, mintAcc.GetAddress(), accAddrs[4], balances) require.NoError(err) // Check balances @@ -189,22 +186,17 @@ func (suite *KeeperTestSuite) TestSendCoins_Module_To_Account() { func (suite *KeeperTestSuite) TestSendCoins_Module_To_Module() { ctx := suite.ctx require := suite.Require() - balances := sdk.NewCoins(newFooCoin(200), newBarCoin(100)) + balances := sdk.NewCoins(newFooCoin(100), newBarCoin(50)) suite.mockMintCoins(mintAcc) require.NoError(suite.bankKeeper.MintCoins(ctx, banktypes.MintModuleName, balances)) - // Try send to invalid module - // In this case it will create a new account - suite.authKeeper.EXPECT().GetModuleAddress(mintAcc.Name).Return(mintAcc.GetAddress()) - suite.authKeeper.EXPECT().GetModuleAddress("invalid").Return(nil) - err := suite.bankKeeper.SendCoins(ctx, []byte(mintAcc.Name), []byte("invalid"), sdk.NewCoins(newFooCoin(100), newBarCoin(50))) - require.NoError(err) + // Try send from burner module + err := suite.bankKeeper.SendCoins(ctx, burnerAcc.GetAddress(), mintAcc.GetAddress(), sdk.NewCoins(newFooCoin(100), newBarCoin(50))) + require.Error(err) // Send from mint module to burn module - suite.authKeeper.EXPECT().GetModuleAddress(mintAcc.Name).Return(mintAcc.GetAddress()) - suite.authKeeper.EXPECT().GetModuleAddress(burnerAcc.Name).Return(burnerAcc.GetAddress()) - err = suite.bankKeeper.SendCoins(ctx, []byte(mintAcc.Name), []byte(burnerAcc.Name), sdk.NewCoins(newFooCoin(100), newBarCoin(50))) + err = suite.bankKeeper.SendCoins(ctx, mintAcc.GetAddress(), burnerAcc.GetAddress(), sdk.NewCoins(newFooCoin(100), newBarCoin(50))) require.NoError(err) // Check balances @@ -212,10 +204,6 @@ func (suite *KeeperTestSuite) TestSendCoins_Module_To_Module() { require.Equal(burnerFooBalance.Amount, math.NewInt(100)) burnerBarBalance := suite.bankKeeper.GetBalance(ctx, burnerAcc.GetAddress(), barDenom) require.Equal(burnerBarBalance.Amount, math.NewInt(50)) - invalidFooBalance := suite.bankKeeper.GetBalance(ctx, []byte("invalid"), fooDenom) - require.Equal(invalidFooBalance.Amount, math.NewInt(100)) - invalidBarBalance := suite.bankKeeper.GetBalance(ctx, []byte("invalid"), barDenom) - require.Equal(invalidBarBalance.Amount, math.NewInt(50)) mintFooBalance := suite.bankKeeper.GetBalance(ctx, mintAcc.GetAddress(), fooDenom) require.Equal(mintFooBalance.Amount, math.NewInt(0)) mintBarBalance := suite.bankKeeper.GetBalance(ctx, mintAcc.GetAddress(), barDenom) diff --git a/x/bank/v2/testutil/helpers.go b/x/bank/v2/testutil/helpers.go index 37be81009095..b65ece6bfcd4 100644 --- a/x/bank/v2/testutil/helpers.go +++ b/x/bank/v2/testutil/helpers.go @@ -12,10 +12,10 @@ import ( // FundAccount is a utility function that funds an account by minting and // sending the coins to the address. This should be used for testing purposes // only! -func FundAccount(ctx context.Context, bankKeeper bankkeeper.Keeper, addr []byte, amounts sdk.Coins) error { +func FundAccount(ctx context.Context, bankKeeper bankkeeper.Keeper, authKeeper types.AuthKeeper, addr []byte, amounts sdk.Coins) error { if err := bankKeeper.MintCoins(ctx, types.MintModuleName, amounts); err != nil { return err } - - return bankKeeper.SendCoins(ctx, []byte(types.MintModuleName), addr, amounts) + mintAddr := authKeeper.GetModuleAddress(types.MintModuleName) + return bankKeeper.SendCoins(ctx, mintAddr, addr, amounts) } From 8a61b147f31c231d8862e94f9294700e7f4270ca Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Wed, 11 Sep 2024 20:13:53 +0700 Subject: [PATCH 15/21] lint --- x/bank/v2/keeper/keeper.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/bank/v2/keeper/keeper.go b/x/bank/v2/keeper/keeper.go index 433c90503342..56325eddd35a 100644 --- a/x/bank/v2/keeper/keeper.go +++ b/x/bank/v2/keeper/keeper.go @@ -8,6 +8,7 @@ import ( "cosmossdk.io/core/address" appmodulev2 "cosmossdk.io/core/appmodule/v2" "cosmossdk.io/core/event" + "cosmossdk.io/math" "cosmossdk.io/x/bank/v2/types" From bf74c6b9a6135d427966179f0461465527ffd70a Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Wed, 11 Sep 2024 21:24:40 +0700 Subject: [PATCH 16/21] gofumpt --- x/bank/v2/keeper/keeper.go | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/x/bank/v2/keeper/keeper.go b/x/bank/v2/keeper/keeper.go index 56325eddd35a..1f3a0bb0927f 100644 --- a/x/bank/v2/keeper/keeper.go +++ b/x/bank/v2/keeper/keeper.go @@ -1,22 +1,21 @@ package keeper import ( - "context" - - "cosmossdk.io/collections" - "cosmossdk.io/collections/indexes" - "cosmossdk.io/core/address" - appmodulev2 "cosmossdk.io/core/appmodule/v2" - "cosmossdk.io/core/event" - - "cosmossdk.io/math" - "cosmossdk.io/x/bank/v2/types" - - errorsmod "cosmossdk.io/errors" - "github.com/cosmos/cosmos-sdk/codec" - sdk "github.com/cosmos/cosmos-sdk/types" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + "context" + + "cosmossdk.io/collections" + "cosmossdk.io/collections/indexes" + "cosmossdk.io/core/address" + appmodulev2 "cosmossdk.io/core/appmodule/v2" + "cosmossdk.io/core/event" + "cosmossdk.io/math" + "cosmossdk.io/x/bank/v2/types" + + errorsmod "cosmossdk.io/errors" + "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" ) // Keeper defines the bank/v2 module keeper. @@ -58,7 +57,6 @@ func NewKeeper(authority []byte, addressCodec address.Codec, env appmodulev2.Env // MintCoins creates new coins from thin air and adds it to the module account. // An error is returned if the module account does not exist or is unauthorized. func (k Keeper) MintCoins(ctx context.Context, moduleName string, amounts sdk.Coins) error { - // TODO: Mint restriction acc := k.ak.GetModuleAccount(ctx, moduleName) if acc == nil { From c36d49451a8981753e9060589d86f76c20c5f295 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Wed, 11 Sep 2024 21:58:04 +0700 Subject: [PATCH 17/21] feedback --- x/bank/v2/keeper/keeper.go | 48 +++++++++++++++++++------------------- x/bank/v2/types/keys.go | 15 ------------ 2 files changed, 24 insertions(+), 39 deletions(-) diff --git a/x/bank/v2/keeper/keeper.go b/x/bank/v2/keeper/keeper.go index 1f3a0bb0927f..03a8401d9a14 100644 --- a/x/bank/v2/keeper/keeper.go +++ b/x/bank/v2/keeper/keeper.go @@ -1,21 +1,21 @@ package keeper import ( - "context" - - "cosmossdk.io/collections" - "cosmossdk.io/collections/indexes" - "cosmossdk.io/core/address" - appmodulev2 "cosmossdk.io/core/appmodule/v2" - "cosmossdk.io/core/event" - "cosmossdk.io/math" - "cosmossdk.io/x/bank/v2/types" - - errorsmod "cosmossdk.io/errors" - "github.com/cosmos/cosmos-sdk/codec" - sdk "github.com/cosmos/cosmos-sdk/types" - sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" + "context" + + "cosmossdk.io/collections" + "cosmossdk.io/collections/indexes" + "cosmossdk.io/core/address" + appmodulev2 "cosmossdk.io/core/appmodule/v2" + "cosmossdk.io/core/event" + "cosmossdk.io/math" + "cosmossdk.io/x/bank/v2/types" + + errorsmod "cosmossdk.io/errors" + "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" ) // Keeper defines the bank/v2 module keeper. @@ -28,7 +28,7 @@ type Keeper struct { addressCodec address.Codec schema collections.Schema params collections.Item[types.Params] - balances *collections.IndexedMap[collections.Pair[sdk.AccAddress, string], math.Int, BalancesIndexes] + balances *collections.IndexedMap[collections.Pair[[]byte, string], math.Int, BalancesIndexes] supply collections.Map[string, math.Int] } @@ -41,7 +41,7 @@ func NewKeeper(authority []byte, addressCodec address.Codec, env appmodulev2.Env authority: authority, addressCodec: addressCodec, // TODO(@julienrbrt): Should we add address codec to the environment? params: collections.NewItem(sb, types.ParamsKey, "params", codec.CollValue[types.Params](cdc)), - balances: collections.NewIndexedMap(sb, types.BalancesPrefix, "balances", collections.PairKeyCodec(sdk.AccAddressKey, collections.StringKey), types.BalanceValueCodec, newBalancesIndexes(sb)), + balances: collections.NewIndexedMap(sb, types.BalancesPrefix, "balances", collections.PairKeyCodec(collections.BytesKey, collections.StringKey), sdk.IntValue, newBalancesIndexes(sb)), supply: collections.NewMap(sb, types.SupplyKey, "supply", collections.StringKey, sdk.IntValue), } @@ -147,7 +147,7 @@ func (k Keeper) GetSupply(ctx context.Context, denom string) sdk.Coin { // GetBalance returns the balance of a specific denomination for a given account // by address. -func (k Keeper) GetBalance(ctx context.Context, addr sdk.AccAddress, denom string) sdk.Coin { +func (k Keeper) GetBalance(ctx context.Context, addr []byte, denom string) sdk.Coin { amt, err := k.balances.Get(ctx, collections.Join(addr, denom)) if err != nil { return sdk.NewCoin(denom, math.ZeroInt()) @@ -161,7 +161,7 @@ func (k Keeper) GetBalance(ctx context.Context, addr sdk.AccAddress, denom strin // CONTRACT: The provided amount (amt) must be valid, non-negative coins. // // A coin_spent event is emitted after the operation. -func (k Keeper) subUnlockedCoins(ctx context.Context, addr sdk.AccAddress, amt sdk.Coins) error { +func (k Keeper) subUnlockedCoins(ctx context.Context, addr []byte, amt sdk.Coins) error { for _, coin := range amt { balance := k.GetBalance(ctx, addr, coin.Denom) spendable := sdk.Coins{balance} @@ -202,7 +202,7 @@ func (k Keeper) subUnlockedCoins(ctx context.Context, addr sdk.AccAddress, amt s // CONTRACT: The provided amount (amt) must be valid, non-negative coins. // // It emits a coin_received event after the operation. -func (k Keeper) addCoins(ctx context.Context, addr sdk.AccAddress, amt sdk.Coins) error { +func (k Keeper) addCoins(ctx context.Context, addr []byte, amt sdk.Coins) error { for _, coin := range amt { balance := k.GetBalance(ctx, addr, coin.Denom) newBalance := balance.Add(coin) @@ -236,7 +236,7 @@ func (k Keeper) setSupply(ctx context.Context, coin sdk.Coin) { } // setBalance sets the coin balance for an account by address. -func (k Keeper) setBalance(ctx context.Context, addr sdk.AccAddress, balance sdk.Coin) error { +func (k Keeper) setBalance(ctx context.Context, addr []byte, balance sdk.Coin) error { if !balance.IsValid() { return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, balance.String()) } @@ -256,12 +256,12 @@ func newBalancesIndexes(sb *collections.SchemaBuilder) BalancesIndexes { return BalancesIndexes{ Denom: indexes.NewReversePair[math.Int]( sb, types.DenomAddressPrefix, "address_by_denom_index", - collections.PairKeyCodec(sdk.LengthPrefixedAddressKey(sdk.AccAddressKey), collections.StringKey), //nolint:staticcheck // Note: refer to the LengthPrefixedAddressKey docs to understand why we do this. - indexes.WithReversePairUncheckedValue(), // denom to address indexes were stored as Key: Join(denom, address) Value: []byte{0}, this will migrate the value to []byte{} in a lazy way. + collections.PairKeyCodec(collections.BytesKey, collections.StringKey), //nolint:staticcheck // Note: refer to the LengthPrefixedAddressKey docs to understand why we do this. + indexes.WithReversePairUncheckedValue(), // denom to address indexes were stored as Key: Join(denom, address) Value: []byte{0}, this will migrate the value to []byte{} in a lazy way. ), } } type BalancesIndexes struct { - Denom *indexes.ReversePair[sdk.AccAddress, string, math.Int] + Denom *indexes.ReversePair[[]byte, string, math.Int] } diff --git a/x/bank/v2/types/keys.go b/x/bank/v2/types/keys.go index b235b1a0b821..79eff8ae1c3e 100644 --- a/x/bank/v2/types/keys.go +++ b/x/bank/v2/types/keys.go @@ -2,10 +2,6 @@ package types import ( "cosmossdk.io/collections" - collcodec "cosmossdk.io/collections/codec" - "cosmossdk.io/math" - - sdk "github.com/cosmos/cosmos-sdk/types" ) const ( @@ -36,14 +32,3 @@ var ( SupplyKey = collections.NewPrefix(5) ) - -// BalanceValueCodec is a codec for encoding bank balances in a backwards compatible way. -// Historically, balances were represented as Coin, now they're represented as a simple math.Int -var BalanceValueCodec = collcodec.NewAltValueCodec(sdk.IntValue, func(bytes []byte) (math.Int, error) { - c := new(sdk.Coin) - err := c.Unmarshal(bytes) - if err != nil { - return math.Int{}, err - } - return c.Amount, nil -}) From 4f5deceff6860084e16754d40b8defd7add73a21 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Wed, 11 Sep 2024 22:00:22 +0700 Subject: [PATCH 18/21] unused --- x/bank/v2/types/expected_keepers.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/bank/v2/types/expected_keepers.go b/x/bank/v2/types/expected_keepers.go index aecd62fd57d3..fae6e453d015 100644 --- a/x/bank/v2/types/expected_keepers.go +++ b/x/bank/v2/types/expected_keepers.go @@ -9,7 +9,6 @@ import ( // AuthKeeper defines the account contract that must be fulfilled when // creating a x/bank keeper. type AuthKeeper interface { - GetAccount(ctx context.Context, addr sdk.AccAddress) sdk.AccountI GetModuleAccount(ctx context.Context, moduleName string) sdk.ModuleAccountI GetModuleAddress(moduleName string) sdk.AccAddress } From 1c7db48d6b2d6aee880ac7c344439744b4827c73 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Wed, 11 Sep 2024 22:14:49 +0700 Subject: [PATCH 19/21] remove auth --- x/bank/v2/depinject.go | 3 +-- x/bank/v2/keeper/keeper.go | 25 ++++++---------------- x/bank/v2/keeper/keeper_test.go | 32 ++++------------------------- x/bank/v2/testutil/helpers.go | 9 ++------ x/bank/v2/types/expected_keepers.go | 13 ------------ 5 files changed, 13 insertions(+), 69 deletions(-) diff --git a/x/bank/v2/depinject.go b/x/bank/v2/depinject.go index 2ba0128bc40d..b5465d7e02df 100644 --- a/x/bank/v2/depinject.go +++ b/x/bank/v2/depinject.go @@ -32,7 +32,6 @@ type ModuleInputs struct { Cdc codec.Codec Environment appmodule.Environment AddressCodec address.Codec - AuthKeeper types.AuthKeeper } type ModuleOutputs struct { @@ -54,7 +53,7 @@ func ProvideModule(in ModuleInputs) ModuleOutputs { } } - k := keeper.NewKeeper(authority, in.AddressCodec, in.Environment, in.Cdc, in.AuthKeeper) + k := keeper.NewKeeper(authority, in.AddressCodec, in.Environment, in.Cdc) m := NewAppModule(in.Cdc, k) return ModuleOutputs{ diff --git a/x/bank/v2/keeper/keeper.go b/x/bank/v2/keeper/keeper.go index 03a8401d9a14..e057b8f56ef0 100644 --- a/x/bank/v2/keeper/keeper.go +++ b/x/bank/v2/keeper/keeper.go @@ -15,7 +15,6 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" ) // Keeper defines the bank/v2 module keeper. @@ -23,7 +22,6 @@ import ( type Keeper struct { appmodulev2.Environment - ak types.AuthKeeper authority []byte addressCodec address.Codec schema collections.Schema @@ -32,12 +30,11 @@ type Keeper struct { supply collections.Map[string, math.Int] } -func NewKeeper(authority []byte, addressCodec address.Codec, env appmodulev2.Environment, cdc codec.BinaryCodec, ak types.AuthKeeper) *Keeper { +func NewKeeper(authority []byte, addressCodec address.Codec, env appmodulev2.Environment, cdc codec.BinaryCodec) *Keeper { sb := collections.NewSchemaBuilder(env.KVStoreService) k := &Keeper{ Environment: env, - ak: ak, authority: authority, addressCodec: addressCodec, // TODO(@julienrbrt): Should we add address codec to the environment? params: collections.NewItem(sb, types.ParamsKey, "params", codec.CollValue[types.Params](cdc)), @@ -56,22 +53,14 @@ func NewKeeper(authority []byte, addressCodec address.Codec, env appmodulev2.Env // MintCoins creates new coins from thin air and adds it to the module account. // An error is returned if the module account does not exist or is unauthorized. -func (k Keeper) MintCoins(ctx context.Context, moduleName string, amounts sdk.Coins) error { - // TODO: Mint restriction - acc := k.ak.GetModuleAccount(ctx, moduleName) - if acc == nil { - return errorsmod.Wrapf(sdkerrors.ErrUnknownAddress, "module account %s does not exist", moduleName) - } - - if !acc.HasPermission(authtypes.Minter) { - return errorsmod.Wrapf(sdkerrors.ErrUnauthorized, "module account %s does not have permissions to mint tokens", moduleName) - } +func (k Keeper) MintCoins(ctx context.Context, addr []byte, amounts sdk.Coins) error { + // TODO: Mint restriction & permission if !amounts.IsValid() { return errorsmod.Wrap(sdkerrors.ErrInvalidCoins, amounts.String()) } - err := k.addCoins(ctx, acc.GetAddress(), amounts) + err := k.addCoins(ctx, addr, amounts) if err != nil { return err } @@ -82,9 +71,7 @@ func (k Keeper) MintCoins(ctx context.Context, moduleName string, amounts sdk.Co k.setSupply(ctx, supply) } - k.Logger.Debug("minted coins from module account", "amount", amounts.String(), "from", moduleName) - - addrStr, err := k.addressCodec.BytesToString(acc.GetAddress()) + addrStr, err := k.addressCodec.BytesToString(addr) if err != nil { return err } @@ -98,7 +85,7 @@ func (k Keeper) MintCoins(ctx context.Context, moduleName string, amounts sdk.Co } // SendCoins transfers amt coins from a sending account to a receiving account. -// Function take sender & receipient as string. +// Function take sender & receipient as []byte. // They can be sdk address or module name. // An error is returned upon failure. func (k Keeper) SendCoins(ctx context.Context, from, to []byte, amt sdk.Coins) error { diff --git a/x/bank/v2/keeper/keeper_test.go b/x/bank/v2/keeper/keeper_test.go index 6beb48062410..a0ae69b54571 100644 --- a/x/bank/v2/keeper/keeper_test.go +++ b/x/bank/v2/keeper/keeper_test.go @@ -5,7 +5,6 @@ import ( "testing" "time" - "github.com/golang/mock/gomock" "github.com/stretchr/testify/suite" "cosmossdk.io/core/address" @@ -51,25 +50,11 @@ func newBarCoin(amt int64) sdk.Coin { return sdk.NewInt64Coin(barDenom, amt) } -func (suite *KeeperTestSuite) mockMintCoins(moduleAcc *authtypes.ModuleAccount) { - suite.authKeeper.EXPECT().GetModuleAccount(suite.ctx, moduleAcc.Name).Return(moduleAcc) -} - -func (suite *KeeperTestSuite) mockSendCoinsFromModuleToAccount(moduleAcc *authtypes.ModuleAccount, recv sdk.AccAddress) { - suite.authKeeper.EXPECT().GetModuleAddress(moduleAcc.Name).Return(moduleAcc.GetAddress()) -} - -func (suite *KeeperTestSuite) mockFundAccount(receiver sdk.AccAddress) { - suite.mockMintCoins(mintAcc) - suite.mockSendCoinsFromModuleToAccount(mintAcc, receiver) -} - type KeeperTestSuite struct { suite.Suite ctx context.Context bankKeeper keeper.Keeper - authKeeper *banktestutil.MockAccountKeeper addressCodec address.Codec } @@ -89,17 +74,12 @@ func (suite *KeeperTestSuite) SetupTest() { authority, err := ac.BytesToString(authtypes.NewModuleAddress("gov")) suite.Require().NoError(err) - // gomock initializations - ctrl := gomock.NewController(suite.T()) - authKeeper := banktestutil.NewMockAccountKeeper(ctrl) suite.ctx = ctx - suite.authKeeper = authKeeper suite.bankKeeper = *keeper.NewKeeper( []byte(authority), ac, env, encCfg.Codec, - authKeeper, ) suite.addressCodec = ac } @@ -115,8 +95,7 @@ func (suite *KeeperTestSuite) TestSendCoins_Acount_To_Account() { require.Error(err) // Set balances for acc0 and then try send to acc1 - suite.mockFundAccount(accAddrs[0]) - require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, suite.authKeeper, accAddrs[0], balances)) + require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, accAddrs[0], balances)) require.NoError(suite.bankKeeper.SendCoins(ctx, accAddrs[0], accAddrs[1], sendAmt)) // Check balances @@ -141,8 +120,7 @@ func (suite *KeeperTestSuite) TestSendCoins_Acount_To_Module() { require.Error(err) // Set balances for acc0 and then try send to acc1 - suite.mockFundAccount(accAddrs[0]) - require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, suite.authKeeper, accAddrs[0], balances)) + require.NoError(banktestutil.FundAccount(ctx, suite.bankKeeper, accAddrs[0], balances)) require.NoError(suite.bankKeeper.SendCoins(ctx, accAddrs[0], burnerAcc.GetAddress(), sendAmt)) // Check balances @@ -161,8 +139,7 @@ func (suite *KeeperTestSuite) TestSendCoins_Module_To_Account() { require := suite.Require() balances := sdk.NewCoins(newFooCoin(100), newBarCoin(50)) - suite.mockMintCoins(mintAcc) - require.NoError(suite.bankKeeper.MintCoins(ctx, banktypes.MintModuleName, balances)) + require.NoError(suite.bankKeeper.MintCoins(ctx, mintAcc.GetAddress(), balances)) // Try send from burner module err := suite.bankKeeper.SendCoins(ctx, burnerAcc.GetAddress(), accAddrs[4], balances) @@ -188,8 +165,7 @@ func (suite *KeeperTestSuite) TestSendCoins_Module_To_Module() { require := suite.Require() balances := sdk.NewCoins(newFooCoin(100), newBarCoin(50)) - suite.mockMintCoins(mintAcc) - require.NoError(suite.bankKeeper.MintCoins(ctx, banktypes.MintModuleName, balances)) + require.NoError(suite.bankKeeper.MintCoins(ctx, mintAcc.GetAddress(), balances)) // Try send from burner module err := suite.bankKeeper.SendCoins(ctx, burnerAcc.GetAddress(), mintAcc.GetAddress(), sdk.NewCoins(newFooCoin(100), newBarCoin(50))) diff --git a/x/bank/v2/testutil/helpers.go b/x/bank/v2/testutil/helpers.go index b65ece6bfcd4..afb2d82bcea6 100644 --- a/x/bank/v2/testutil/helpers.go +++ b/x/bank/v2/testutil/helpers.go @@ -4,7 +4,6 @@ import ( "context" bankkeeper "cosmossdk.io/x/bank/v2/keeper" - "cosmossdk.io/x/bank/v2/types" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -12,10 +11,6 @@ import ( // FundAccount is a utility function that funds an account by minting and // sending the coins to the address. This should be used for testing purposes // only! -func FundAccount(ctx context.Context, bankKeeper bankkeeper.Keeper, authKeeper types.AuthKeeper, addr []byte, amounts sdk.Coins) error { - if err := bankKeeper.MintCoins(ctx, types.MintModuleName, amounts); err != nil { - return err - } - mintAddr := authKeeper.GetModuleAddress(types.MintModuleName) - return bankKeeper.SendCoins(ctx, mintAddr, addr, amounts) +func FundAccount(ctx context.Context, bankKeeper bankkeeper.Keeper, addr []byte, amounts sdk.Coins) error { + return bankKeeper.MintCoins(ctx, addr, amounts) } diff --git a/x/bank/v2/types/expected_keepers.go b/x/bank/v2/types/expected_keepers.go index fae6e453d015..ab1254f4c2be 100644 --- a/x/bank/v2/types/expected_keepers.go +++ b/x/bank/v2/types/expected_keepers.go @@ -1,14 +1 @@ package types - -import ( - "context" - - sdk "github.com/cosmos/cosmos-sdk/types" -) - -// AuthKeeper defines the account contract that must be fulfilled when -// creating a x/bank keeper. -type AuthKeeper interface { - GetModuleAccount(ctx context.Context, moduleName string) sdk.ModuleAccountI - GetModuleAddress(moduleName string) sdk.AccAddress -} From c4c284b95bde8884fe31f63143d7def942119882 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Thu, 12 Sep 2024 13:55:33 +0700 Subject: [PATCH 20/21] lint --- x/bank/v2/keeper/keeper.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/x/bank/v2/keeper/keeper.go b/x/bank/v2/keeper/keeper.go index e057b8f56ef0..f16982f168c4 100644 --- a/x/bank/v2/keeper/keeper.go +++ b/x/bank/v2/keeper/keeper.go @@ -8,10 +8,10 @@ import ( "cosmossdk.io/core/address" appmodulev2 "cosmossdk.io/core/appmodule/v2" "cosmossdk.io/core/event" + errorsmod "cosmossdk.io/errors" "cosmossdk.io/math" "cosmossdk.io/x/bank/v2/types" - errorsmod "cosmossdk.io/errors" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -243,8 +243,8 @@ func newBalancesIndexes(sb *collections.SchemaBuilder) BalancesIndexes { return BalancesIndexes{ Denom: indexes.NewReversePair[math.Int]( sb, types.DenomAddressPrefix, "address_by_denom_index", - collections.PairKeyCodec(collections.BytesKey, collections.StringKey), //nolint:staticcheck // Note: refer to the LengthPrefixedAddressKey docs to understand why we do this. - indexes.WithReversePairUncheckedValue(), // denom to address indexes were stored as Key: Join(denom, address) Value: []byte{0}, this will migrate the value to []byte{} in a lazy way. + collections.PairKeyCodec(collections.BytesKey, collections.StringKey), + indexes.WithReversePairUncheckedValue(), // denom to address indexes were stored as Key: Join(denom, address) Value: []byte{0}, this will migrate the value to []byte{} in a lazy way. ), } } From b927ae1eaa672bdd7260711df9c5cf780a76c227 Mon Sep 17 00:00:00 2001 From: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Date: Thu, 12 Sep 2024 18:23:44 +0700 Subject: [PATCH 21/21] update authority --- x/bank/v2/keeper/keeper_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/x/bank/v2/keeper/keeper_test.go b/x/bank/v2/keeper/keeper_test.go index a0ae69b54571..32ccbac4d2ef 100644 --- a/x/bank/v2/keeper/keeper_test.go +++ b/x/bank/v2/keeper/keeper_test.go @@ -71,12 +71,11 @@ func (suite *KeeperTestSuite) SetupTest() { env := runtime.NewEnvironment(runtime.NewKVStoreService(key), coretesting.NewNopLogger()) ac := codectestutil.CodecOptions{}.GetAddressCodec() - authority, err := ac.BytesToString(authtypes.NewModuleAddress("gov")) - suite.Require().NoError(err) + authority := authtypes.NewModuleAddress("gov") suite.ctx = ctx suite.bankKeeper = *keeper.NewKeeper( - []byte(authority), + authority, ac, env, encCfg.Codec,