LaconicNetwork/cosmos-sdk
7
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

refactor(bank): audit/QA changes (backport #21048) (#21187)

Browse Source 
Co-authored-by: Facundo Medica <14063057+facundomedica@users.noreply.github.com>
This commit is contained in:
mergify[bot] 2024-08-06 21:19:35 +02:00 committed by GitHub
parent c21b60638c
commit adcf958f34
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 136 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -67,7 +67,6 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
### Improvements
* (codec) [#20122](https://github.com/cosmos/cosmos-sdk/pull/20122) Added a cache to address codec.
* (bank) [#20354](https://github.com/cosmos/cosmos-sdk/pull/20354) Reduce the number of `ValidateDenom` calls in `bank.SendCoins`.
* (types) [#19869](https://github.com/cosmos/cosmos-sdk/pull/19869) Removed `Any` type from `codec/types` and replaced it with an alias for `cosmos/gogoproto/types/any`.
* (server) [#19854](https://github.com/cosmos/cosmos-sdk/pull/19854) Add customizability to start command.
* Add `StartCmdOptions` in `server.AddCommands` instead of `servertypes.ModuleInitFlags`. To set custom flags set them in the `StartCmdOptions` struct on the `AddFlags` field.

46
tests/integration/bank/app_test.go
View File

@ -118,7 +118,7 @@ func checkBalance(t *testing.T, baseApp *baseapp.BaseApp, addr sdk.AccAddress, b
t.Helper()
ctxCheck := baseApp.NewContext(true)
keeperBalances := keeper.GetAllBalances(ctxCheck, addr)
require.True(t, balances.Equal(keeperBalances))
require.True(t, balances.Equal(keeperBalances), balances.String(), keeperBalances.String())
}
func TestSendNotEnoughBalance(t *testing.T) {
@ -477,3 +477,47 @@ func TestMsgSetSendEnabled(t *testing.T) {
})
}
}
// TestSendToNonExistingAccount tests sending coins to an account that does not exist, and this account
// must not be created.
func TestSendToNonExistingAccount(t *testing.T) {
acc1 := authtypes.NewBaseAccountWithAddress(addr1)
genAccs := []authtypes.GenesisAccount{acc1}
s := createTestSuite(t, genAccs)
baseApp := s.App.BaseApp
ctx := baseApp.NewContext(false)
require.NoError(t, testutil.FundAccount(ctx, s.BankKeeper, addr1, sdk.NewCoins(sdk.NewInt64Coin("foocoin", 42))))
_, err := baseApp.FinalizeBlock(&abci.FinalizeBlockRequest{Height: baseApp.LastBlockHeight() + 1})
require.NoError(t, err)
_, err = baseApp.Commit()
require.NoError(t, err)
addr2Str, err := s.AccountKeeper.AddressCodec().BytesToString(addr2)
require.NoError(t, err)
sendMsg := types.NewMsgSend(addr1.String(), addr2Str, coins)
h := header.Info{Height: baseApp.LastBlockHeight() + 1}
txConfig := moduletestutil.MakeTestTxConfig(cdctestutil.CodecOptions{})
_, _, err = simtestutil.SignCheckDeliver(t, txConfig, baseApp, h, []sdk.Msg{sendMsg}, "", []uint64{0}, []uint64{0}, true, true, priv1)
require.NoError(t, err)
// Check that the account was not created
acc2 := s.AccountKeeper.GetAccount(baseApp.NewContext(true), addr2)
require.Nil(t, acc2)
// But it does have a balance
checkBalance(t, baseApp, addr2, coins, s.BankKeeper)
// Now we send coins back and the account should be created
sendMsg = types.NewMsgSend(addr2Str, addr1.String(), coins)
h = header.Info{Height: baseApp.LastBlockHeight() + 1}
_, _, err = simtestutil.SignCheckDeliver(t, txConfig, baseApp, h, []sdk.Msg{sendMsg}, "", []uint64{0}, []uint64{0}, true, true, priv2)
require.NoError(t, err)
// Balance has been reduced
checkBalance(t, baseApp, addr2, sdk.NewCoins(), s.BankKeeper)
// Check that the account was created
acc2 = s.AccountKeeper.GetAccount(baseApp.NewContext(true), addr2)
require.NotNil(t, acc2, "account should have been created %s", addr2.String())
}

1
x/bank/CHANGELOG.md
View File

@ -34,6 +34,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [#18636](https://github.com/cosmos/cosmos-sdk/pull/18636) `SendCoinsFromModuleToAccount`, `SendCoinsFromModuleToModule`, `SendCoinsFromAccountToModule`, `DelegateCoinsFromAccountToModule`, `UndelegateCoinsFromModuleToAccount`, `MintCoins` and `BurnCoins` methods now returns an error instead of panicking if any module accounts does not exist or unauthorized.
* [#20517](https://github.com/cosmos/cosmos-sdk/pull/20517) `SendCoins` now checks for `SendRestrictions` before instead of after deducting coins using `subUnlockedCoins`.
* [#20354](https://github.com/cosmos/cosmos-sdk/pull/20354) Reduce the number of `ValidateDenom` calls in `bank.SendCoins`.
### API Breaking Changes

2
x/bank/README.md
View File

@ -125,7 +125,7 @@ aforementioned state:
## Params
The bank module stores it's params in state with the prefix of `0x05`,
The bank module stores its params in state with the prefix of `0x05`,
it can be updated with governance or the address with authority.
* Params: `0x05 | ProtocolBuffer(Params)`

2
x/bank/autocli.go
View File

@ -95,7 +95,7 @@ To look up all denoms, do not provide any arguments.`,
RpcCommandOptions: []*autocliv1.RpcCommandOptions{
{
RpcMethod: "Send",
Use: "send [from_key_or_address] [to_address] [amount]",
Use: "send [from_key_or_address] [to_address] [amount ...]",
Short: "Send funds from one account to another.",
Long: `Send funds from one account to another.
Note, the '--from' flag is ignored as it is implied from [from_key_or_address].

5
x/bank/keeper/keeper.go
View File

@ -414,12 +414,13 @@ func (k BaseKeeper) BurnCoins(ctx context.Context, address []byte, amounts sdk.C
k.setSupply(ctx, supply)
}
k.Logger.Debug("burned tokens from account", "amount", amounts.String(), "from", address)
addrStr, err := k.ak.AddressCodec().BytesToString(acc.GetAddress())
if err != nil {
return err
}
k.Logger.Debug("burned tokens from account", "amount", amounts.String(), "from", addrStr)
// emit burn event
return k.EventService.EventManager(ctx).EmitKV(
types.EventTypeCoinBurn,

3
x/bank/keeper/keeper_test.go
View File

@ -579,6 +579,9 @@ func (suite *KeeperTestSuite) TestSupply_BurnCoins() {
suite.mockBurnCoins(burnerAcc)
require.NoError(keeper.BurnCoins(ctx, burnerAcc.GetAddress(), initCoins))
suite.mockBurnCoins(burnerAcc)
require.ErrorContains(keeper.BurnCoins(ctx, burnerAcc.GetAddress(), sdk.Coins{sdk.Coin{Denom: "asd", Amount: math.NewInt(-1)}}), "-1asd: invalid coins")
supplyAfterBurn, _, err := keeper.GetPaginatedTotalSupply(ctx, &query.PageRequest{})
require.NoError(err)
require.Equal(sdk.NewCoins(), keeper.GetAllBalances(ctx, burnerAcc.GetAddress()))

4
x/bank/keeper/msg_server.go
View File

@ -177,7 +177,7 @@ func (k msgServer) SetSendEnabled(ctx context.Context, msg *types.MsgSetSendEnab
return &types.MsgSetSendEnabledResponse{}, nil
}
func (k msgServer) Burn(goCtx context.Context, msg *types.MsgBurn) (*types.MsgBurnResponse, error) {
func (k msgServer) Burn(ctx context.Context, msg *types.MsgBurn) (*types.MsgBurnResponse, error) {
var (
from []byte
err error
@ -205,7 +205,7 @@ func (k msgServer) Burn(goCtx context.Context, msg *types.MsgBurn) (*types.MsgBu
return nil, errorsmod.Wrap(sdkerrors.ErrInvalidCoins, coins.String())
}
err = k.BurnCoins(goCtx, from, coins)
err = k.BurnCoins(ctx, from, coins)
if err != nil {
return nil, err
}

77
x/bank/types/inputs_outputs_test.go Normal file
View File

@ -0,0 +1,77 @@
package types_test
import (
"testing"
"github.com/stretchr/testify/require"
"cosmossdk.io/math"
"cosmossdk.io/x/bank/types"
sdk "github.com/cosmos/cosmos-sdk/types"
)
func TestValidateInputOutputs(t *testing.T) {
tests := []struct {
name string
input types.Input
outputs []types.Output
expErr bool
}{
{
"valid input and outputs",
types.NewInput("cosmos1yq8lgssgxlx9smjhes6ryjasmqmd3ts2559g0t", sdk.Coins{sdk.NewInt64Coin("uatom", 1)}),
[]types.Output{
{
Address: "cosmos1yq8lgssgxlx9smjhes6ryjasmqmd3ts2559g0t",
Coins: sdk.Coins{sdk.NewInt64Coin("uatom", 1)},
},
},
false,
},
{
"empty input",
types.Input{},
[]types.Output{
{
Address: "cosmos1yq8lgssgxlx9smjhes6ryjasmqmd3ts2559g0t",
Coins: sdk.Coins{sdk.NewInt64Coin("uatom", 1)},
},
},
true,
},
{
"input and output mismatch",
types.NewInput("cosmos1yq8lgssgxlx9smjhes6ryjasmqmd3ts2559g0t", sdk.Coins{sdk.NewInt64Coin("uatom", 1)}),
[]types.Output{
{
Address: "cosmos1yq8lgssgxlx9smjhes6ryjasmqmd3ts2559g0t",
Coins: sdk.Coins{sdk.NewInt64Coin("uatom", 2)},
},
},
true,
},
{
"invalid output",
types.NewInput("cosmos1yq8lgssgxlx9smjhes6ryjasmqmd3ts2559g0t", sdk.Coins{sdk.NewInt64Coin("uatom", 1)}),
[]types.Output{
{
Address: "cosmos1yq8lgssgxlx9smjhes6ryjasmqmd3ts2559g0t",
Coins: sdk.Coins{sdk.Coin{Denom: "", Amount: math.NewInt(1)}},
},
},
true,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
err := types.ValidateInputOutputs(tc.input, tc.outputs)
if tc.expErr {
require.Error(t, err)
} else {
require.NoError(t, err)
}
})
}
}

6
x/bank/types/msgs.go
View File

@ -12,17 +12,17 @@ var (
_ coretransaction.Msg = &MsgUpdateParams{}
)
// NewMsgSend - construct a msg to send coins from one account to another.
// NewMsgSend constructs a msg to send coins from one account to another.
func NewMsgSend(fromAddr, toAddr string, amount sdk.Coins) *MsgSend {
return &MsgSend{FromAddress: fromAddr, ToAddress: toAddr, Amount: amount}
}
// NewMsgMultiSend - construct arbitrary multi-in, multi-out send msg.
// NewMsgMultiSend constructs an arbitrary multi-in, multi-out send msg.
func NewMsgMultiSend(in Input, out []Output) *MsgMultiSend {
return &MsgMultiSend{Inputs: []Input{in}, Outputs: out}
}
// NewMsgSetSendEnabled Construct a message to set one or more SendEnabled entries.
// NewMsgSetSendEnabled constructs a message to set one or more SendEnabled entries.
func NewMsgSetSendEnabled(authority string, sendEnabled []*SendEnabled, useDefaultFor []string) *MsgSetSendEnabled {
return &MsgSetSendEnabled{
Authority: authority,