diff --git a/x/bank/app_test.go b/x/bank/app_test.go index b81cfdbaa..fb4f6a74f 100644 --- a/x/bank/app_test.go +++ b/x/bank/app_test.go @@ -2,6 +2,7 @@ package bank_test import ( "context" + "fmt" "testing" "github.com/stretchr/testify/require" @@ -121,6 +122,142 @@ func TestSendNotEnoughBalance(t *testing.T) { require.Equal(t, res2.GetSequence(), origSeq+1) } +func TestSendReceiverNotInAllowList(t *testing.T) { + acc := &authtypes.BaseAccount{ + Address: addr1.String(), + } + + genAccs := []authtypes.GenesisAccount{acc} + app := simapp.SetupWithGenesisAccounts(genAccs) + ctx := app.BaseApp.NewContext(false, tmproto.Header{}) + testDenom := "testDenom" + factoryDenom := fmt.Sprintf("factory/%s/%s", addr1.String(), testDenom) + + require.NoError(t, simapp.FundAccount(app.BankKeeper, ctx, addr1, sdk.NewCoins(sdk.NewInt64Coin(factoryDenom, 100)))) + app.BankKeeper.SetDenomAllowList(ctx, factoryDenom, + types.AllowList{Addresses: []string{addr1.String()}}) + + app.Commit(context.Background()) + + res1 := app.AccountKeeper.GetAccount(ctx, addr1) + require.NotNil(t, res1) + require.Equal(t, acc, res1.(*authtypes.BaseAccount)) + + origAccNum := res1.GetAccountNumber() + origSeq := res1.GetSequence() + + sendMsg := types.NewMsgSend(addr1, addr2, sdk.Coins{sdk.NewInt64Coin(factoryDenom, 10)}) + header := tmproto.Header{Height: app.LastBlockHeight() + 1} + txGen := simapp.MakeTestEncodingConfig().TxConfig + _, _, err := simapp.SignCheckDeliver(t, txGen, app.BaseApp, header, []sdk.Msg{sendMsg}, "", []uint64{origAccNum}, []uint64{origSeq}, false, false, priv1) + require.Error(t, err) + require.Contains(t, err.Error(), fmt.Sprintf("%s is not allowed to receive funds", addr2)) + + simapp.CheckBalance(t, app, addr1, sdk.Coins{sdk.NewInt64Coin(factoryDenom, 100)}) +} + +func TestSendSenderAndReceiverInAllowList(t *testing.T) { + acc := &authtypes.BaseAccount{ + Address: addr1.String(), + } + + genAccs := []authtypes.GenesisAccount{acc} + app := simapp.SetupWithGenesisAccounts(genAccs) + ctx := app.BaseApp.NewContext(false, tmproto.Header{}) + testDenom := "testDenom" + factoryDenom := fmt.Sprintf("factory/%s/%s", addr1.String(), testDenom) + + require.NoError(t, simapp.FundAccount(app.BankKeeper, ctx, addr1, sdk.NewCoins(sdk.NewInt64Coin(factoryDenom, 100)))) + app.BankKeeper.SetDenomAllowList(ctx, factoryDenom, + types.AllowList{Addresses: []string{addr1.String(), addr2.String()}}) + + app.Commit(context.Background()) + + res1 := app.AccountKeeper.GetAccount(ctx, addr1) + require.NotNil(t, res1) + require.Equal(t, acc, res1.(*authtypes.BaseAccount)) + + origAccNum := res1.GetAccountNumber() + origSeq := res1.GetSequence() + + sendMsg := types.NewMsgSend(addr1, addr2, sdk.Coins{sdk.NewInt64Coin(factoryDenom, 10)}) + header := tmproto.Header{Height: app.LastBlockHeight() + 1} + txGen := simapp.MakeTestEncodingConfig().TxConfig + _, _, err := simapp.SignCheckDeliver(t, txGen, app.BaseApp, header, []sdk.Msg{sendMsg}, "", []uint64{origAccNum}, []uint64{origSeq}, true, true, priv1) + require.NoError(t, err) + + simapp.CheckBalance(t, app, addr1, sdk.Coins{sdk.NewInt64Coin(factoryDenom, 90)}) + simapp.CheckBalance(t, app, addr2, sdk.Coins{sdk.NewInt64Coin(factoryDenom, 10)}) +} + +func TestSendWithEmptyAllowList(t *testing.T) { + acc := &authtypes.BaseAccount{ + Address: addr1.String(), + } + + genAccs := []authtypes.GenesisAccount{acc} + app := simapp.SetupWithGenesisAccounts(genAccs) + ctx := app.BaseApp.NewContext(false, tmproto.Header{}) + testDenom := "testDenom" + factoryDenom := fmt.Sprintf("factory/%s/%s", addr1.String(), testDenom) + + require.NoError(t, simapp.FundAccount(app.BankKeeper, ctx, addr1, sdk.NewCoins(sdk.NewInt64Coin(factoryDenom, 100)))) + app.BankKeeper.SetDenomAllowList(ctx, factoryDenom, + types.AllowList{Addresses: []string{}}) + + app.Commit(context.Background()) + + res1 := app.AccountKeeper.GetAccount(ctx, addr1) + require.NotNil(t, res1) + require.Equal(t, acc, res1.(*authtypes.BaseAccount)) + + origAccNum := res1.GetAccountNumber() + origSeq := res1.GetSequence() + + sendMsg := types.NewMsgSend(addr1, addr2, sdk.Coins{sdk.NewInt64Coin(factoryDenom, 10)}) + header := tmproto.Header{Height: app.LastBlockHeight() + 1} + txGen := simapp.MakeTestEncodingConfig().TxConfig + _, _, err := simapp.SignCheckDeliver(t, txGen, app.BaseApp, header, []sdk.Msg{sendMsg}, "", []uint64{origAccNum}, []uint64{origSeq}, true, true, priv1) + require.NoError(t, err) + + simapp.CheckBalance(t, app, addr1, sdk.Coins{sdk.NewInt64Coin(factoryDenom, 90)}) + simapp.CheckBalance(t, app, addr2, sdk.Coins{sdk.NewInt64Coin(factoryDenom, 10)}) +} + +func TestSendSenderNotInAllowList(t *testing.T) { + acc := &authtypes.BaseAccount{ + Address: addr1.String(), + } + + genAccs := []authtypes.GenesisAccount{acc} + app := simapp.SetupWithGenesisAccounts(genAccs) + ctx := app.BaseApp.NewContext(false, tmproto.Header{}) + testDenom := "testDenom" + factoryDenom := fmt.Sprintf("factory/%s/%s", addr1.String(), testDenom) + + require.NoError(t, simapp.FundAccount(app.BankKeeper, ctx, addr1, sdk.NewCoins(sdk.NewInt64Coin(factoryDenom, 100)))) + app.BankKeeper.SetDenomAllowList(ctx, factoryDenom, + types.AllowList{Addresses: []string{addr2.String()}}) + + app.Commit(context.Background()) + + res1 := app.AccountKeeper.GetAccount(ctx, addr1) + require.NotNil(t, res1) + require.Equal(t, acc, res1.(*authtypes.BaseAccount)) + + origAccNum := res1.GetAccountNumber() + origSeq := res1.GetSequence() + + sendMsg := types.NewMsgSend(addr1, addr2, sdk.Coins{sdk.NewInt64Coin(factoryDenom, 10)}) + header := tmproto.Header{Height: app.LastBlockHeight() + 1} + txGen := simapp.MakeTestEncodingConfig().TxConfig + _, _, err := simapp.SignCheckDeliver(t, txGen, app.BaseApp, header, []sdk.Msg{sendMsg}, "", []uint64{origAccNum}, []uint64{origSeq}, false, false, priv1) + require.Error(t, err) + require.Contains(t, err.Error(), fmt.Sprintf("%s is not allowed to send funds", addr1)) + + simapp.CheckBalance(t, app, addr1, sdk.Coins{sdk.NewInt64Coin(factoryDenom, 100)}) +} + func TestMsgMultiSendWithAccounts(t *testing.T) { acc := &authtypes.BaseAccount{ Address: addr1.String(), @@ -337,3 +474,240 @@ func TestMsgMultiSendDependent(t *testing.T) { } } } + +func TestMultiSendAllowList(t *testing.T) { + // CoinToAllowList defines a struct to map coins to their allow lists. + type CoinToAllowList struct { + fundAmount sdk.Coin + sendAmount sdk.Coin + allowList types.AllowList + } + + type testCase struct { + name string + coinsToAllowList []CoinToAllowList + sender sdk.AccAddress + receiver sdk.AccAddress + accNums []uint64 + accSeqs []uint64 + privKeys []cryptotypes.PrivKey + expectedSenderBal sdk.Coins + expectedReceiverBal sdk.Coins + expectedError bool + expectedErrorMsg string + } + + senderAcc := sdk.AccAddress(priv1.PubKey().Address()) + receiverAcc := sdk.AccAddress(priv2.PubKey().Address()) + testDenom := fmt.Sprintf("factory/%s/test", senderAcc.String()) + testDenom1 := fmt.Sprintf("factory/%s/test1", senderAcc.String()) + // Define test cases + testCases := []testCase{ + + { + name: "sender not allowed to send coins", + coinsToAllowList: []CoinToAllowList{ + { + fundAmount: sdk.NewInt64Coin(testDenom, 100), + sendAmount: sdk.NewInt64Coin(testDenom, 20), + allowList: types.AllowList{ + Addresses: []string{ + receiverAcc.String(), + }, + }, + }, + }, + accNums: []uint64{0, 2}, + accSeqs: []uint64{0, 0}, + sender: senderAcc, + receiver: receiverAcc, + privKeys: []cryptotypes.PrivKey{priv1}, + expectedSenderBal: sdk.NewCoins(sdk.NewInt64Coin(testDenom, 100)), + expectedReceiverBal: sdk.NewCoins(sdk.NewInt64Coin(testDenom, 0)), + expectedError: true, + expectedErrorMsg: fmt.Sprintf("%s is not allowed to send funds", senderAcc), + }, + { + name: "receiver not allowed to receive coins", + coinsToAllowList: []CoinToAllowList{ + { + fundAmount: sdk.NewInt64Coin(testDenom, 100), + sendAmount: sdk.NewInt64Coin(testDenom, 20), + allowList: types.AllowList{ + Addresses: []string{ + senderAcc.String(), + }, + }, + }, + }, + accNums: []uint64{0, 2}, + accSeqs: []uint64{0, 0}, + sender: senderAcc, + receiver: receiverAcc, + privKeys: []cryptotypes.PrivKey{priv1}, + expectedSenderBal: sdk.NewCoins(sdk.NewInt64Coin(testDenom, 100)), + expectedReceiverBal: sdk.NewCoins(sdk.NewInt64Coin(testDenom, 0)), + expectedError: true, + expectedErrorMsg: fmt.Sprintf("%s is not allowed to receive funds", receiverAcc), + }, + { + name: "allow list is empty (no restrictions)", + coinsToAllowList: []CoinToAllowList{ + { + fundAmount: sdk.NewInt64Coin(testDenom, 100), + sendAmount: sdk.NewInt64Coin(testDenom, 20), + allowList: types.AllowList{ + Addresses: []string{}, + }, + }, + }, + accNums: []uint64{0, 2}, + accSeqs: []uint64{0, 0}, + sender: senderAcc, + receiver: receiverAcc, + expectedSenderBal: sdk.NewCoins(sdk.NewInt64Coin(testDenom, 80)), + expectedReceiverBal: sdk.NewCoins(sdk.NewInt64Coin(testDenom, 20)), + privKeys: []cryptotypes.PrivKey{priv1}, + expectedError: false, + }, + { + name: "both sender and receiver are allowed to send and receive coins", + coinsToAllowList: []CoinToAllowList{ + { + fundAmount: sdk.NewInt64Coin(testDenom, 100), + sendAmount: sdk.NewInt64Coin(testDenom, 25), + allowList: types.AllowList{ + Addresses: []string{ + senderAcc.String(), receiverAcc.String(), + }, + }, + }, + }, + accNums: []uint64{0, 2}, + accSeqs: []uint64{0, 0}, + sender: senderAcc, + receiver: receiverAcc, + privKeys: []cryptotypes.PrivKey{priv1}, + expectedSenderBal: sdk.NewCoins(sdk.NewInt64Coin(testDenom, 75)), + expectedReceiverBal: sdk.NewCoins(sdk.NewInt64Coin(testDenom, 25)), + expectedError: false, + }, + { + name: "both are allowed for first coin, but only sender is allowed for second coin", + coinsToAllowList: []CoinToAllowList{ + { + fundAmount: sdk.NewInt64Coin(testDenom, 100), + sendAmount: sdk.NewInt64Coin(testDenom, 20), + allowList: types.AllowList{ + Addresses: []string{ + senderAcc.String(), receiverAcc.String(), + }, + }, + }, + { + fundAmount: sdk.NewInt64Coin(testDenom1, 200), + sendAmount: sdk.NewInt64Coin(testDenom1, 20), + allowList: types.AllowList{ + Addresses: []string{ + senderAcc.String(), + }, + }, + }, + }, + accNums: []uint64{0, 2}, + accSeqs: []uint64{0, 0}, + sender: senderAcc, + receiver: receiverAcc, + privKeys: []cryptotypes.PrivKey{priv1}, + expectedError: true, + expectedSenderBal: sdk.NewCoins(sdk.NewInt64Coin(testDenom, 100), sdk.NewInt64Coin(testDenom1, 200)), + expectedReceiverBal: sdk.NewCoins(sdk.NewInt64Coin(testDenom, 0), sdk.NewInt64Coin(testDenom1, 0)), + expectedErrorMsg: fmt.Sprintf("%s is not allowed to receive funds", receiverAcc), + }, + { + name: "both sender and receiver are allowed to send and receive 2 coins", + coinsToAllowList: []CoinToAllowList{ + { + fundAmount: sdk.NewInt64Coin(testDenom, 100), + sendAmount: sdk.NewInt64Coin(testDenom, 25), + allowList: types.AllowList{ + Addresses: []string{ + senderAcc.String(), receiverAcc.String(), + }, + }, + }, + { + fundAmount: sdk.NewInt64Coin(testDenom1, 200), + sendAmount: sdk.NewInt64Coin(testDenom1, 50), + allowList: types.AllowList{ + Addresses: []string{ + senderAcc.String(), receiverAcc.String(), + }, + }, + }, + }, + accNums: []uint64{0, 2}, + accSeqs: []uint64{0, 0}, + sender: senderAcc, + receiver: receiverAcc, + expectedSenderBal: sdk.NewCoins(sdk.NewInt64Coin(testDenom, 75), sdk.NewInt64Coin(testDenom1, 150)), + expectedReceiverBal: sdk.NewCoins(sdk.NewInt64Coin(testDenom, 25), sdk.NewInt64Coin(testDenom1, 50)), + privKeys: []cryptotypes.PrivKey{priv1}, + expectedError: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Setup genesis accounts + sender := &authtypes.BaseAccount{ + Address: tc.sender.String(), + } + receiver := &authtypes.BaseAccount{ + Address: tc.receiver.String(), + } + + genAccs := []authtypes.GenesisAccount{sender, receiver} + app := simapp.SetupWithGenesisAccounts(genAccs) + ctx := app.BaseApp.NewContext(false, tmproto.Header{}) + + msgs := make([]sdk.Msg, 0) + + for _, coinToAllowList := range tc.coinsToAllowList { + app.BankKeeper.SetDenomAllowList(ctx, coinToAllowList.fundAmount.Denom, coinToAllowList.allowList) + require.NoError(t, simapp.FundAccount(app.BankKeeper, ctx, sender.GetAddress(), sdk.NewCoins(coinToAllowList.fundAmount))) + multiSendMsg := &types.MsgMultiSend{ + Inputs: []types.Input{ + types.NewInput(sender.GetAddress(), sdk.Coins{coinToAllowList.sendAmount}), + }, + Outputs: []types.Output{ + types.NewOutput(receiver.GetAddress(), sdk.Coins{coinToAllowList.sendAmount}), + }, + } + msgs = append(msgs, multiSendMsg) + } + + app.Commit(context.Background()) + + header := tmproto.Header{Height: app.LastBlockHeight() + 1} + txGen := simapp.MakeTestEncodingConfig().TxConfig + _, _, err := simapp.SignCheckDeliver(t, txGen, app.BaseApp, header, msgs, "", tc.accNums, tc.accSeqs, !tc.expectedError, !tc.expectedError, tc.privKeys...) + + if tc.expectedError { + require.Error(t, err, "expected an error but got none") + require.Contains(t, err.Error(), tc.expectedErrorMsg) + } else { + require.NoError(t, err, "did not expect an error but got one") + } + + // Validate balances + // Sender's balance after send should be as expected + senderBal := app.BankKeeper.GetAllBalances(ctx, tc.sender) + require.Equal(t, tc.expectedSenderBal, senderBal, "sender balance mismatch") + + // Receiver's balance after receive should be as expected + receiverBal := app.BankKeeper.GetAllBalances(ctx, tc.receiver) + require.Equal(t, tc.expectedReceiverBal, receiverBal, "receiver balance mismatch") + }) + } +} diff --git a/x/bank/keeper/keeper_test.go b/x/bank/keeper/keeper_test.go index ea6a77ea3..1c5ffd2ea 100644 --- a/x/bank/keeper/keeper_test.go +++ b/x/bank/keeper/keeper_test.go @@ -52,10 +52,6 @@ func newFactoryFooCoin(address sdk.AccAddress, amt int64) sdk.Coin { return sdk.NewInt64Coin(fmt.Sprintf("%s/%s/%s", factoryDenomPrefix, address, fooDenom), amt) } -func newFactoryBarCoin(address sdk.AccAddress, amt int64) sdk.Coin { - return sdk.NewInt64Coin(fmt.Sprintf("%s/%s/%s", factoryDenomPrefix, address, barDenom), amt) -} - func newBarCoin(amt int64) sdk.Coin { return sdk.NewInt64Coin(barDenom, amt) } @@ -494,108 +490,6 @@ func (suite *IntegrationTestSuite) TestInputOutputCoins() { suite.Require().Equal(expected, acc3Balances) } -func (suite *IntegrationTestSuite) TestInputOutputCoinsWithAllowList() { - app, ctx := suite.app, suite.ctx - - addr1 := sdk.AccAddress("addr1_______________") - acc1 := app.AccountKeeper.NewAccountWithAddress(ctx, addr1) - app.AccountKeeper.SetAccount(ctx, acc1) - factoryCoin := newFactoryFooCoin(addr1, 100) - balances := sdk.NewCoins(factoryCoin) - - addr2 := sdk.AccAddress("addr2_______________") - acc2 := app.AccountKeeper.NewAccountWithAddress(ctx, addr2) - app.AccountKeeper.SetAccount(ctx, acc2) - - addr3 := sdk.AccAddress("addr3_______________") - acc3 := app.AccountKeeper.NewAccountWithAddress(ctx, addr3) - app.AccountKeeper.SetAccount(ctx, acc3) - - addr4 := sdk.AccAddress("addr4_______________") - acc4 := app.AccountKeeper.NewAccountWithAddress(ctx, addr4) - app.AccountKeeper.SetAccount(ctx, acc4) - - suite.Require().NoError(simapp.FundAccount(app.BankKeeper, ctx, addr1, balances)) - app.BankKeeper.SetDenomAllowList(ctx, factoryCoin.Denom, - types.AllowList{ - Addresses: []string{addr1.String(), addr2.String(), addr3.String()}}) - - inputs := []types.Input{ - {Address: addr1.String(), Coins: sdk.NewCoins(newFactoryFooCoin(addr1, 30))}, - {Address: addr1.String(), Coins: sdk.NewCoins(newFactoryFooCoin(addr1, 30))}, - } - outputs := []types.Output{ - {Address: addr2.String(), Coins: sdk.NewCoins(newFactoryFooCoin(addr1, 30))}, - {Address: addr3.String(), Coins: sdk.NewCoins(newFactoryFooCoin(addr1, 30))}, - } - suite.Require().NoError(app.BankKeeper.InputOutputCoins(ctx, inputs, outputs)) - - acc1Balances := app.BankKeeper.GetAllBalances(ctx, addr1) - expected := sdk.NewCoins(newFactoryFooCoin(addr1, 40)) - suite.Require().Equal(expected, acc1Balances) - - acc2Balances := app.BankKeeper.GetAllBalances(ctx, addr2) - expected = sdk.NewCoins(newFactoryFooCoin(addr1, 30)) - suite.Require().Equal(expected, acc2Balances) - - acc3Balances := app.BankKeeper.GetAllBalances(ctx, addr3) - expected = sdk.NewCoins(newFactoryFooCoin(addr1, 30)) - suite.Require().Equal(expected, acc3Balances) - - inputs1 := []types.Input{ - {Address: addr1.String(), Coins: sdk.NewCoins(newFactoryFooCoin(addr1, 5))}, - {Address: addr1.String(), Coins: sdk.NewCoins(newFactoryFooCoin(addr1, 10))}, - } - outputs1 := []types.Output{ - {Address: addr2.String(), Coins: sdk.NewCoins(newFactoryFooCoin(addr1, 5))}, - {Address: addr4.String(), Coins: sdk.NewCoins(newFactoryFooCoin(addr1, 10))}, - } - - err := app.BankKeeper.InputOutputCoins(ctx, inputs1, outputs1) - suite.Require().Error(err) - suite.Require().Equal( - fmt.Sprintf("%s is not allowed to receive funds: unauthorized", addr4.String()), err.Error()) - - acc1Balances = app.BankKeeper.GetAllBalances(ctx, addr1) - expected = sdk.NewCoins(newFactoryFooCoin(addr1, 40)) - suite.Require().Equal(expected, acc1Balances) - - acc2Balances = app.BankKeeper.GetAllBalances(ctx, addr2) - expected = sdk.NewCoins(newFactoryFooCoin(addr1, 30)) - suite.Require().Equal(expected, acc2Balances) - - acc3Balances = app.BankKeeper.GetAllBalances(ctx, addr3) - expected = sdk.NewCoins(newFactoryFooCoin(addr1, 30)) - suite.Require().Equal(expected, acc3Balances) - - inputs2 := []types.Input{ - {Address: addr4.String(), Coins: sdk.NewCoins(newFactoryFooCoin(addr1, 5))}, - {Address: addr4.String(), Coins: sdk.NewCoins(newFactoryFooCoin(addr1, 10))}, - } - outputs2 := []types.Output{ - {Address: addr1.String(), Coins: sdk.NewCoins(newFactoryFooCoin(addr1, 5))}, - {Address: addr2.String(), Coins: sdk.NewCoins(newFactoryFooCoin(addr1, 10))}, - } - - err = app.BankKeeper.InputOutputCoins(ctx, inputs2, outputs2) - suite.Require().Error(err) - suite.Require().Equal( - fmt.Sprintf("%s is not allowed to send funds: unauthorized", addr4.String()), err.Error()) - - acc1Balances = app.BankKeeper.GetAllBalances(ctx, addr1) - expected = sdk.NewCoins(newFactoryFooCoin(addr1, 40)) - suite.Require().Equal(expected, acc1Balances) - - acc2Balances = app.BankKeeper.GetAllBalances(ctx, addr2) - expected = sdk.NewCoins(newFactoryFooCoin(addr1, 30)) - suite.Require().Equal(expected, acc2Balances) - - acc3Balances = app.BankKeeper.GetAllBalances(ctx, addr3) - expected = sdk.NewCoins(newFactoryFooCoin(addr1, 30)) - suite.Require().Equal(expected, acc3Balances) - -} - func (suite *IntegrationTestSuite) TestSendCoins() { app, ctx := suite.app, suite.ctx balances := sdk.NewCoins(newFooCoin(100), newBarCoin(50)) @@ -661,68 +555,136 @@ func (suite *IntegrationTestSuite) TestSendCoinsWithAllowList() { suite.Require().Equal(expected, acc2Balances) } -func (suite *IntegrationTestSuite) TestSendCoinsWithSenderNotInAllowList() { - app, ctx := suite.app, suite.ctx +// Test that creating allowlist does not block module to module transfers +func (suite *IntegrationTestSuite) TestSendCoinsModuleToModuleWithAllowList() { + // add module accounts to supply keeper + ctx := suite.ctx + _, keeper := suite.initKeepersWithmAccPerms(make(map[string]bool)) + app := suite.app + app.BankKeeper = keeper + addr1 := sdk.AccAddress("addr1_______________") acc1 := app.AccountKeeper.NewAccountWithAddress(ctx, addr1) app.AccountKeeper.SetAccount(ctx, acc1) factoryCoin := newFactoryFooCoin(addr1, 100) balances := sdk.NewCoins(factoryCoin, newBarCoin(50)) - suite.Require().NoError(simapp.FundAccount(app.BankKeeper, ctx, addr1, balances)) + app.BankKeeper.SetDenomAllowList(ctx, factoryCoin.Denom, types.AllowList{ + Addresses: []string{addr1.String()}}) - addr2 := sdk.AccAddress("addr2_______________") - acc2 := app.AccountKeeper.NewAccountWithAddress(ctx, addr2) - app.AccountKeeper.SetAccount(ctx, acc2) + // set up bank balances + suite.Require().NoError(simapp.FundAccount(app.BankKeeper, ctx, multiPermAcc.GetAddress(), balances)) - app.BankKeeper.SetDenomAllowList(ctx, factoryCoin.Denom, - types.AllowList{Addresses: []string{addr2.String()}}) + sendCoins := sdk.NewCoins(newFactoryFooCoin(addr1, 50), newBarCoin(20)) + suite.Require().NoError(app.BankKeeper.SendCoinsFromModuleToModule(ctx, multiPerm, randomPerm, sendCoins)) + expectedBankBalances := sdk.NewCoins(newFactoryFooCoin(addr1, 50), newBarCoin(30)) + // assert module balances correct + bals := app.BankKeeper.GetAllBalances(ctx, multiPermAcc.GetAddress()) + suite.Require().Equal(expectedBankBalances, bals) + // assert receiver balances correct + userBals := app.BankKeeper.GetAllBalances(ctx, randomPermAcc.GetAddress()) + suite.Require().Equal(sendCoins, userBals) +} - sendAmt := sdk.NewCoins(newFactoryFooCoin(addr1, 5), newBarCoin(5)) - err := app.BankKeeper.SendCoins(ctx, addr1, addr2, sendAmt) - suite.Require().Error(err) - suite.Require().Equal( - fmt.Sprintf("%s is not allowed to send funds: unauthorized", addr1.String()), err.Error()) +// Test that creating allowlist does not block sending from module to account even though we are not explicitly adding +// the module account to the allowlist +func (suite *IntegrationTestSuite) TestSendCoinsModuleToAccountWithAllowList() { + // add module accounts to supply keeper + ctx := suite.ctx + moduleAddresses := make(map[string]bool) + moduleAddresses[multiPermAcc.GetAddress().String()] = true + moduleAddresses[suite.app.AccountKeeper.GetModuleAddress("mint").String()] = true + _, keeper := suite.initKeepersWithmAccPerms(make(map[string]bool)) + app := suite.app + app.BankKeeper = keeper - // Balances should remain the same - acc1Balances := app.BankKeeper.GetAllBalances(ctx, addr1) - expected := sdk.NewCoins(newFactoryFooCoin(addr1, 100), newBarCoin(50)) - suite.Require().Equal(expected, acc1Balances) + addr1 := sdk.AccAddress("addr1_______________") + acc1 := app.AccountKeeper.NewAccountWithAddress(ctx, addr1) + app.AccountKeeper.SetAccount(ctx, acc1) + factoryCoin := newFactoryFooCoin(addr1, 100) + balances := sdk.NewCoins(factoryCoin, newBarCoin(50)) + app.BankKeeper.SetDenomAllowList(ctx, factoryCoin.Denom, types.AllowList{ + Addresses: []string{addr1.String()}}) - acc2Balances := app.BankKeeper.GetAllBalances(ctx, addr2) - expected = sdk.NewCoins(newFactoryFooCoin(addr1, 0), newBarCoin(0)) - suite.Require().Equal(expected, acc2Balances) + // set up bank balances + suite.Require().NoError(simapp.FundAccount(app.BankKeeper, ctx, multiPermAcc.GetAddress(), balances)) + + sendCoins := sdk.NewCoins(newFactoryFooCoin(addr1, 50), newBarCoin(20)) + suite.Require().NoError(app.BankKeeper.SendCoinsFromModuleToAccount(ctx, multiPerm, addr1, sendCoins)) + + expectedBankBalances := sdk.NewCoins(newFactoryFooCoin(addr1, 50), newBarCoin(30)) + // assert module balances correct + bals := app.BankKeeper.GetAllBalances(ctx, multiPermAcc.GetAddress()) + suite.Require().Equal(expectedBankBalances, bals) + // assert receiver balances correct + userBals := app.BankKeeper.GetAllBalances(ctx, addr1) + suite.Require().Equal(sendCoins, userBals) } -func (suite *IntegrationTestSuite) TestSendCoinsWithReceiverNotInAllowList() { - app, ctx := suite.app, suite.ctx +// Test that creating allowlist does not block sending from account to module even though we are not explicitly adding +// the module account to the allowlist +func (suite *IntegrationTestSuite) TestSendCoinsAccountToModuleWithAllowList() { + // add module accounts to supply keeper + ctx := suite.ctx + moduleAddresses := make(map[string]bool) + moduleAddresses[multiPermAcc.GetAddress().String()] = true + moduleAddresses[suite.app.AccountKeeper.GetModuleAddress("mint").String()] = true + _, keeper := suite.initKeepersWithmAccPerms(make(map[string]bool)) + app := suite.app + app.BankKeeper = keeper + addr1 := sdk.AccAddress("addr1_______________") acc1 := app.AccountKeeper.NewAccountWithAddress(ctx, addr1) app.AccountKeeper.SetAccount(ctx, acc1) factoryCoin := newFactoryFooCoin(addr1, 100) balances := sdk.NewCoins(factoryCoin, newBarCoin(50)) + app.BankKeeper.SetDenomAllowList(ctx, factoryCoin.Denom, types.AllowList{ + Addresses: []string{addr1.String()}}) + + // set up bank balances suite.Require().NoError(simapp.FundAccount(app.BankKeeper, ctx, addr1, balances)) - addr2 := sdk.AccAddress("addr2_______________") - acc2 := app.AccountKeeper.NewAccountWithAddress(ctx, addr2) - app.AccountKeeper.SetAccount(ctx, acc2) + sendCoins := sdk.NewCoins(newFactoryFooCoin(addr1, 50), newBarCoin(20)) + suite.Require().NoError(app.BankKeeper.SendCoinsFromAccountToModule(ctx, addr1, multiPerm, sendCoins)) + expectedBankBalances := sdk.NewCoins(newFactoryFooCoin(addr1, 50), newBarCoin(30)) + // assert account balances correct + bals := app.BankKeeper.GetAllBalances(ctx, addr1) + suite.Require().Equal(expectedBankBalances, bals) + // assert module balances correct + userBals := app.BankKeeper.GetAllBalances(ctx, multiPermAcc.GetAddress()) + suite.Require().Equal(sendCoins, userBals) +} - app.BankKeeper.SetDenomAllowList(ctx, factoryCoin.Denom, - types.AllowList{Addresses: []string{addr1.String()}}) +// Test that creating allowlist does not block sending from account to module even though we are not explicitly adding +// the module account to the allowlist +func (suite *IntegrationTestSuite) TestDeferredSendCoinsAccountToModuleWithAllowList() { + // add module accounts to supply keeper + ctx := suite.ctx + _, keeper := suite.initKeepersWithmAccPerms(make(map[string]bool)) + app := suite.app + app.BankKeeper = keeper - sendAmt := sdk.NewCoins(newFactoryFooCoin(addr1, 5), newBarCoin(5)) - err := app.BankKeeper.SendCoins(ctx, addr1, addr2, sendAmt) - suite.Require().Error(err) - suite.Require().Equal( - fmt.Sprintf("%s is not allowed to receive funds: unauthorized", addr2.String()), err.Error()) + addr1 := sdk.AccAddress("addr1_______________") + acc1 := app.AccountKeeper.NewAccountWithAddress(ctx, addr1) + app.AccountKeeper.SetAccount(ctx, acc1) + factoryCoin := newFactoryFooCoin(addr1, 100) + balances := sdk.NewCoins(factoryCoin, newBarCoin(50)) + app.BankKeeper.SetDenomAllowList(ctx, factoryCoin.Denom, types.AllowList{ + Addresses: []string{addr1.String()}}) - // Balances should remain the same - acc1Balances := app.BankKeeper.GetAllBalances(ctx, addr1) - expected := sdk.NewCoins(newFactoryFooCoin(addr1, 100), newBarCoin(50)) - suite.Require().Equal(expected, acc1Balances) + // set up bank balances + suite.Require().NoError(simapp.FundAccount(app.BankKeeper, ctx, addr1, balances)) - acc2Balances := app.BankKeeper.GetAllBalances(ctx, addr2) - expected = sdk.NewCoins(newFactoryFooCoin(addr1, 0), newBarCoin(0)) - suite.Require().Equal(expected, acc2Balances) + sendCoins := sdk.NewCoins(newFactoryFooCoin(addr1, 50), newBarCoin(20)) + suite.Require().NoError(app.BankKeeper.DeferredSendCoinsFromAccountToModule(ctx, addr1, multiPerm, sendCoins)) + app.BankKeeper.WriteDeferredBalances(ctx) + + expectedBankBalances := sdk.NewCoins(newFactoryFooCoin(addr1, 50), newBarCoin(30)) + // assert account balances correct + bals := app.BankKeeper.GetAllBalances(ctx, addr1) + suite.Require().Equal(expectedBankBalances, bals) + // assert module balances correct + userBals := app.BankKeeper.GetAllBalances(ctx, multiPermAcc.GetAddress()) + suite.Require().Equal(sendCoins, userBals) } func (suite *IntegrationTestSuite) TestSendCoinsModuleToAccount() { @@ -1728,11 +1690,11 @@ func (suite *IntegrationTestSuite) TestBaseKeeper_IsAllowedToSendCoins() { } denomToAllowedAddressesCache := make(map[string]keeper.AllowedAddresses) isAllowed := - app.BankKeeper.IsAllowedToSendCoins(ctx, tt.args.addr, coins, denomToAllowedAddressesCache) + app.BankKeeper.IsInDenomAllowList(ctx, tt.args.addr, coins, denomToAllowedAddressesCache) // Use suite.Require to assert the results suite.Require().Equal(tt.isAllowed, isAllowed, - fmt.Errorf("IsAllowedToSendCoins() isAllowed = %v, want %v", + fmt.Errorf("IsInDenomAllowList() isAllowed = %v, want %v", isAllowed, tt.isAllowed)) }) } diff --git a/x/bank/keeper/msg_server.go b/x/bank/keeper/msg_server.go index 4e9237631..946d4e0c4 100644 --- a/x/bank/keeper/msg_server.go +++ b/x/bank/keeper/msg_server.go @@ -39,7 +39,12 @@ func (k msgServer) Send(goCtx context.Context, msg *types.MsgSend) (*types.MsgSe return nil, err } - if k.BlockedAddr(to) { + allowListCache := make(map[string]AllowedAddresses) + if !k.IsInDenomAllowList(ctx, from, msg.Amount, allowListCache) { + return nil, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to send funds", msg.FromAddress) + } + + if k.BlockedAddr(to) || !k.IsInDenomAllowList(ctx, to, msg.Amount, allowListCache) { return nil, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to receive funds", msg.ToAddress) } @@ -72,19 +77,23 @@ func (k msgServer) Send(goCtx context.Context, msg *types.MsgSend) (*types.MsgSe func (k msgServer) MultiSend(goCtx context.Context, msg *types.MsgMultiSend) (*types.MsgMultiSendResponse, error) { ctx := sdk.UnwrapSDKContext(goCtx) - + denomToAllowListCache := make(map[string]AllowedAddresses) // NOTE: totalIn == totalOut should already have been checked for _, in := range msg.Inputs { if err := k.IsSendEnabledCoins(ctx, in.Coins...); err != nil { return nil, err } + accAddr := sdk.MustAccAddressFromBech32(in.Address) + if !k.IsInDenomAllowList(ctx, accAddr, in.Coins, denomToAllowListCache) { + return nil, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to send funds", accAddr) + } } for _, out := range msg.Outputs { accAddr := sdk.MustAccAddressFromBech32(out.Address) - if k.BlockedAddr(accAddr) { - return nil, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to receive transactions", out.Address) + if k.BlockedAddr(accAddr) || !k.IsInDenomAllowList(ctx, accAddr, out.Coins, denomToAllowListCache) { + return nil, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to receive funds", out.Address) } } diff --git a/x/bank/keeper/send.go b/x/bank/keeper/send.go index d62829afb..2825039cc 100644 --- a/x/bank/keeper/send.go +++ b/x/bank/keeper/send.go @@ -13,8 +13,7 @@ import ( ) const ( - TokenFactoryPrefix = "factory" - TokenFactoryModuleName = "tokenfactory" + TokenFactoryPrefix = "factory" ) // SendKeeper defines a module interface that facilitates the transfer of coins @@ -38,7 +37,7 @@ type SendKeeper interface { IsSendEnabledCoins(ctx sdk.Context, coins ...sdk.Coin) error SetDenomAllowList(ctx sdk.Context, denom string, allowList types.AllowList) GetDenomAllowList(ctx sdk.Context, denom string) types.AllowList - IsAllowedToSendCoins(ctx sdk.Context, addr sdk.AccAddress, coins sdk.Coins, cache map[string]AllowedAddresses) bool + IsInDenomAllowList(ctx sdk.Context, addr sdk.AccAddress, coins sdk.Coins, cache map[string]AllowedAddresses) bool BlockedAddr(addr sdk.AccAddress) bool RegisterRecipientChecker(RecipientChecker) @@ -99,12 +98,6 @@ func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, inputs []types.Input, if err := types.ValidateInputsOutputs(inputs, outputs); err != nil { return err } - - err := k.validateInputOutputAddressesAllowedToSendCoins(ctx, inputs, outputs) - if err != nil { - return err - } - for _, in := range inputs { inAddress, err := sdk.AccAddressFromBech32(in.Address) if err != nil { @@ -156,45 +149,9 @@ func (k BaseSendKeeper) InputOutputCoins(ctx sdk.Context, inputs []types.Input, return nil } -func (k BaseSendKeeper) validateInputOutputAddressesAllowedToSendCoins(ctx sdk.Context, inputs []types.Input, outputs []types.Output) error { - denomToAllowListCache := make(map[string]AllowedAddresses) - for _, in := range inputs { - inAddress, err := sdk.AccAddressFromBech32(in.Address) - if err != nil { - return err - } - allowedToSend := k.IsAllowedToSendCoins(ctx, inAddress, in.Coins, denomToAllowListCache) - if !allowedToSend { - return sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to send funds", inAddress) - } - } - for _, out := range outputs { - outAddress, err := sdk.AccAddressFromBech32(out.Address) - if err != nil { - return err - } - allowedToReceive := k.IsAllowedToSendCoins(ctx, outAddress, out.Coins, denomToAllowListCache) - if !allowedToReceive { - return sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to receive funds", outAddress) - } - } - return nil -} - // SendCoins transfers amt coins from a sending account to a receiving account. // An error is returned upon failure. func (k BaseSendKeeper) SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error { - denomToAllowedAddressesCache := make(map[string]AllowedAddresses) - fromAllowed := k.IsAllowedToSendCoins(ctx, fromAddr, amt, denomToAllowedAddressesCache) - if !fromAllowed { - return sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to send funds", fromAddr) - } - - toAllowed := k.IsAllowedToSendCoins(ctx, toAddr, amt, denomToAllowedAddressesCache) - if !toAllowed { - return sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is not allowed to receive funds", toAddr) - } - if err := k.SendCoinsWithoutAccCreation(ctx, fromAddr, toAddr, amt); err != nil { return err } @@ -507,27 +464,27 @@ func (k BaseSendKeeper) GetDenomAllowList(ctx sdk.Context, denom string) types.A return allowList } -// IsAllowedToSendCoins checks if the given address is allowed to send the given coins. +// IsInDenomAllowList checks if the given address is allowed to send the given coins. // The check is performed only fot token factory denoms. For each token factory denom, // it checks if there is allow list for the given denom. If there is no allow list, // the address is allowed to send the coins. If there is an allow list, the address is // allowed to send the coins only if it is in the allow list. -func (k BaseSendKeeper) IsAllowedToSendCoins(ctx sdk.Context, addr sdk.AccAddress, coins sdk.Coins, cache map[string]AllowedAddresses) bool { +func (k BaseSendKeeper) IsInDenomAllowList(ctx sdk.Context, addr sdk.AccAddress, coins sdk.Coins, cache map[string]AllowedAddresses) bool { for _, coin := range coins { - // process only if denom does contain token factory prefix - if strings.HasPrefix(coin.Denom, TokenFactoryPrefix) { - allowedAddresses := k.getAllowedAddresses(ctx, cache, coin.Denom) - if len(allowedAddresses.set) > 0 { - // Add token factory module address to allowlist for minting tokens if allowlist is not empty - tokenFactoryAddr := k.ak.GetModuleAddress(TokenFactoryModuleName) - if tokenFactoryAddr != nil { - allowedAddresses.set[tokenFactoryAddr.String()] = struct{}{} - } - - if !allowedAddresses.contains(addr) { - return false - } - } + // Skip if denom does not contain the token factory prefix + if !strings.HasPrefix(coin.Denom, TokenFactoryPrefix) { + continue + } + + allowedAddresses := k.getAllowedAddresses(ctx, cache, coin.Denom) + // skip if there is no allow list for the denom + if len(allowedAddresses.set) == 0 { + continue + } + + // Return false if the address is neither allowed nor a module address + if !allowedAddresses.contains(addr) { + return false } } return true