refactor: x/bank audit changes (backport #16690)

proto/cosmos/bank/module/v1/module.proto

@@ -10,7 +10,7 @@ message Module {
     go_import: "github.com/cosmos/cosmos-sdk/x/bank"
   };
 
-  // blocked_module_accounts configures exceptional module accounts which should be blocked from receiving funds.
+  // blocked_module_accounts_override configures exceptional module accounts which should be blocked from receiving funds.
   // If left empty it defaults to the list of account names supplied in the auth module configuration as
   // module_account_permissions
   repeated string blocked_module_accounts_override = 1;

proto/cosmos/bank/v1beta1/query.proto

@@ -267,6 +267,7 @@ message QueryParamsRequest {}
 
 // QueryParamsResponse defines the response type for querying x/bank parameters.
 message QueryParamsResponse {
+  // params provides the parameters of the bank module.
   Params params = 1 [(gogoproto.nullable) = false, (amino.dont_omitempty) = true];
 }
 

proto/cosmos/bank/v1beta1/tx.proto

@@ -105,6 +105,7 @@ message MsgSetSendEnabled {
   option (cosmos.msg.v1.signer) = "authority";
   option (amino.name)           = "cosmos-sdk/MsgSetSendEnabled";
 
+  // authority is the address that controls the module.
   string authority = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
 
   // send_enabled is the list of entries to add or update.

x/bank/client/cli/suite_test.go

@@ -22,7 +22,7 @@ type CLITestSuite struct {
 	baseCtx client.Context
 }
 
-func TestMigrateTestSuite(t *testing.T) {
+func TestCLITestSuite(t *testing.T) {
 	suite.Run(t, new(CLITestSuite))
 }
 

x/bank/keeper/grpc_query_test.go

@@ -275,10 +275,10 @@ func (suite *KeeperTestSuite) TestQueryParams() {
 	suite.Require().Equal(suite.bankKeeper.GetParams(suite.ctx), res.GetParams())
 }
 
-func (suite *KeeperTestSuite) QueryDenomsMetadataRequest() {
+func (suite *KeeperTestSuite) TestQueryDenomsMetadataRequest() {
 	var (
 		req         *types.QueryDenomsMetadataRequest
-		expMetadata = []types.Metadata{}
+		expMetadata = []types.Metadata(nil)
 	)
 
 	testCases := []struct {
@@ -377,7 +377,7 @@ func (suite *KeeperTestSuite) QueryDenomsMetadataRequest() {
 	}
 }
 
-func (suite *KeeperTestSuite) QueryDenomMetadataRequest() {
+func (suite *KeeperTestSuite) TestQueryDenomMetadataRequest() {
 	var (
 		req         *types.QueryDenomMetadataRequest
 		expMetadata = types.Metadata{}
@@ -407,7 +407,7 @@ func (suite *KeeperTestSuite) QueryDenomMetadataRequest() {
 		{
 			"success",
 			func() {
-				expMetadata := types.Metadata{
+				expMetadata = types.Metadata{
 					Description: "The native staking token of the Cosmos Hub.",
 					DenomUnits: []*types.DenomUnit{
 						{

x/bank/keeper/invariants.go

@@ -17,6 +17,10 @@ func RegisterInvariants(ir sdk.InvariantRegistry, k Keeper) {
 // AllInvariants runs all invariants of the X/bank module.
 func AllInvariants(k Keeper) sdk.Invariant {
 	return func(ctx sdk.Context) (string, bool) {
+		res, stop := NonnegativeBalanceInvariant(k)(ctx)
+		if stop {
+			return res, stop
+		}
 		return TotalSupply(k)(ctx)
 	}
 }

x/bank/keeper/keeper_test.go

@@ -50,7 +50,7 @@ const (
 
 var (
 	holderAcc    = authtypes.NewEmptyModuleAccount(holder)
-	burnerAcc    = authtypes.NewEmptyModuleAccount(authtypes.Burner, authtypes.Burner)
+	burnerAcc    = authtypes.NewEmptyModuleAccount(authtypes.Burner, authtypes.Burner, authtypes.Staking)
 	minterAcc    = authtypes.NewEmptyModuleAccount(authtypes.Minter, authtypes.Minter)
 	mintAcc      = authtypes.NewEmptyModuleAccount(minttypes.ModuleName, authtypes.Minter)
 	multiPermAcc = authtypes.NewEmptyModuleAccount(multiPerm, authtypes.Burner, authtypes.Minter, authtypes.Staking)
@@ -220,6 +220,16 @@ func (suite *KeeperTestSuite) mockSpendableCoins(ctx sdk.Context, acc sdk.Accoun
 	suite.authKeeper.EXPECT().GetAccount(ctx, acc.GetAddress()).Return(acc)
 }
 
+func (suite *KeeperTestSuite) mockDelegateCoinsFromAccountToModule(acc *authtypes.BaseAccount, moduleAcc *authtypes.ModuleAccount) {
+	suite.authKeeper.EXPECT().GetModuleAccount(suite.ctx, moduleAcc.Name).Return(moduleAcc)
+	suite.mockDelegateCoins(suite.ctx, acc, moduleAcc)
+}
+
+func (suite *KeeperTestSuite) mockUndelegateCoinsFromModuleToAccount(moduleAcc *authtypes.ModuleAccount, accAddr *authtypes.BaseAccount) {
+	suite.authKeeper.EXPECT().GetModuleAccount(suite.ctx, moduleAcc.Name).Return(moduleAcc)
+	suite.mockUnDelegateCoins(suite.ctx, accAddr, moduleAcc)
+}
+
 func (suite *KeeperTestSuite) mockDelegateCoins(ctx context.Context, acc, mAcc sdk.AccountI) {
 	vacc, ok := acc.(banktypes.VestingAccount)
 	if ok {
@@ -316,6 +326,67 @@ func (suite *KeeperTestSuite) TestSendCoinsFromModuleToAccount_Blocklist() {
 	))
 }
 
+func (suite *KeeperTestSuite) TestSupply_DelegateUndelegateCoins() {
+	ctx := suite.ctx
+	require := suite.Require()
+	authKeeper, keeper := suite.authKeeper, suite.bankKeeper
+
+	// set initial balances
+	suite.mockMintCoins(mintAcc)
+	require.NoError(keeper.MintCoins(ctx, banktypes.MintModuleName, initCoins))
+
+	suite.mockSendCoinsFromModuleToAccount(mintAcc, holderAcc.GetAddress())
+	require.NoError(keeper.SendCoinsFromModuleToAccount(ctx, banktypes.MintModuleName, holderAcc.GetAddress(), initCoins))
+
+	authKeeper.EXPECT().GetModuleAddress("").Return(nil)
+	require.Panics(func() {
+		_ = keeper.SendCoinsFromModuleToAccount(ctx, "", holderAcc.GetAddress(), initCoins) //nolint:errcheck // we're testing for a panic, not an error
+	})
+
+	authKeeper.EXPECT().GetModuleAddress(burnerAcc.Name).Return(burnerAcc.GetAddress())
+	authKeeper.EXPECT().GetModuleAccount(ctx, "").Return(nil)
+	require.Panics(func() {
+		_ = keeper.SendCoinsFromModuleToModule(ctx, authtypes.Burner, "", initCoins) //nolint:errcheck // we're testing for a panic, not an error
+	})
+
+	authKeeper.EXPECT().GetModuleAddress("").Return(nil)
+	require.Panics(func() {
+		_ = keeper.SendCoinsFromModuleToAccount(ctx, "", baseAcc.GetAddress(), initCoins) //nolint:errcheck // we're testing for a panic, not an error
+	})
+
+	authKeeper.EXPECT().GetModuleAddress(holderAcc.Name).Return(holderAcc.GetAddress())
+	authKeeper.EXPECT().GetAccount(suite.ctx, holderAcc.GetAddress()).Return(holderAcc)
+	require.Error(
+		keeper.SendCoinsFromModuleToAccount(ctx, holderAcc.GetName(), baseAcc.GetAddress(), initCoins.Add(initCoins...)),
+	)
+	suite.mockSendCoinsFromModuleToModule(holderAcc, burnerAcc)
+	require.NoError(
+		keeper.SendCoinsFromModuleToModule(ctx, holderAcc.GetName(), authtypes.Burner, initCoins),
+	)
+
+	require.Equal(sdk.NewCoins(), keeper.GetAllBalances(ctx, holderAcc.GetAddress()))
+	require.Equal(initCoins, keeper.GetAllBalances(ctx, burnerAcc.GetAddress()))
+
+	suite.mockSendCoinsFromModuleToAccount(burnerAcc, baseAcc.GetAddress())
+	require.NoError(
+		keeper.SendCoinsFromModuleToAccount(ctx, authtypes.Burner, baseAcc.GetAddress(), initCoins),
+	)
+	require.Equal(sdk.NewCoins(), keeper.GetAllBalances(ctx, burnerAcc.GetAddress()))
+	require.Equal(initCoins, keeper.GetAllBalances(ctx, baseAcc.GetAddress()))
+
+	suite.mockDelegateCoinsFromAccountToModule(baseAcc, burnerAcc)
+
+	sdkCtx := sdk.UnwrapSDKContext(ctx)
+	require.NoError(keeper.DelegateCoinsFromAccountToModule(sdkCtx, baseAcc.GetAddress(), authtypes.Burner, initCoins))
+	require.Equal(sdk.NewCoins(), keeper.GetAllBalances(ctx, baseAcc.GetAddress()))
+	require.Equal(initCoins, keeper.GetAllBalances(ctx, burnerAcc.GetAddress()))
+
+	suite.mockUndelegateCoinsFromModuleToAccount(burnerAcc, baseAcc)
+	require.NoError(keeper.UndelegateCoinsFromModuleToAccount(sdkCtx, authtypes.Burner, baseAcc.GetAddress(), initCoins))
+	require.Equal(sdk.NewCoins(), keeper.GetAllBalances(ctx, burnerAcc.GetAddress()))
+	require.Equal(initCoins, keeper.GetAllBalances(ctx, baseAcc.GetAddress()))
+}
+
 func (suite *KeeperTestSuite) TestSupply_SendCoins() {
 	ctx := suite.ctx
 	require := suite.Require()

x/bank/types/msgs.go

@@ -26,24 +26,6 @@ func NewMsgMultiSend(in Input, out []Output) *MsgMultiSend {
 	return &MsgMultiSend{Inputs: []Input{in}, Outputs: out}
 }
 
-// GetSigners Implements Msg.
-func (msg MsgMultiSend) GetSigners() []sdk.AccAddress {
-	// should not happen as ValidateBasic would have failed
-	if len(msg.Inputs) == 0 {
-		return nil
-	}
-
-	addrs, _ := sdk.AccAddressFromBech32(msg.Inputs[0].Address)
-	return []sdk.AccAddress{addrs}
-}
-
-// GetSigners returns the signer addresses that are expected to sign the result
-// of GetSignBytes.
-func (msg MsgUpdateParams) GetSigners() []sdk.AccAddress {
-	authority, _ := sdk.AccAddressFromBech32(msg.Authority)
-	return []sdk.AccAddress{authority}
-}
-
 // NewMsgSetSendEnabled Construct a message to set one or more SendEnabled entries.
 func NewMsgSetSendEnabled(authority string, sendEnabled []*SendEnabled, useDefaultFor []string) *MsgSetSendEnabled {
 	return &MsgSetSendEnabled{
@@ -52,9 +34,3 @@ func NewMsgSetSendEnabled(authority string, sendEnabled []*SendEnabled, useDefau
 		UseDefaultFor: useDefaultFor,
 	}
 }
-
-// GetSigners returns the expected signers for MsgSoftwareUpgrade.
-func (msg MsgSetSendEnabled) GetSigners() []sdk.AccAddress {
-	addr, _ := sdk.AccAddressFromBech32(msg.Authority)
-	return []sdk.AccAddress{addr}
-}

x/bank/types/msgs_test.go

@@ -9,8 +9,11 @@
 	"github.com/cosmos/cosmos-sdk/codec"
 	"github.com/cosmos/cosmos-sdk/codec/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+<<<<<<< HEAD
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
+=======
+>>>>>>> 7c8236d2e (refactor: x/bank audit changes (#16690))
 )
 
 func TestMsgSendGetSignBytes(t *testing.T) {
@@ -122,16 +125,6 @@
 	require.Equal(t, expected, string(res))
 }
 
-func TestMsgMultiSendGetSigners(t *testing.T) {
-	addr := sdk.AccAddress([]byte("input111111111111111"))
-	input := NewInput(addr, nil)
-	msg := NewMsgMultiSend(input, nil)
-
-	res := msg.GetSigners()
-	require.Equal(t, 1, len(res))
-	require.True(t, addr.Equals(res[0]))
-}
-
 func TestNewMsgSetSendEnabled(t *testing.T) {
 	// Punt. Just setting one to all non-default values and making sure they're as expected.
 	msg := NewMsgSetSendEnabled("milton", []*SendEnabled{{"barrycoin", true}}, []string{"billcoin"})
@@ -161,6 +154,7 @@
 	actual := string(actualBz)
 	assert.Equal(t, expected, actual)
 }
+<<<<<<< HEAD
 
 func TestMsgSetSendEnabledGetSigners(t *testing.T) {
 	govModuleAddr := authtypes.NewModuleAddress(govtypes.ModuleName)
@@ -169,3 +163,5 @@
 	actual := msg.GetSigners()
 	assert.Equal(t, expected, actual)
 }
+=======
+>>>>>>> 7c8236d2e (refactor: x/bank audit changes (#16690))