[R4R] Blacklist module accounts (#517)

* implement module account blacklist

* update cdp getModuleAccountCoins method

app/app.go

@@ -98,6 +98,11 @@ var (
 		bep3.ModuleName:             nil,
 		kavadist.ModuleName:         {supply.Minter},
 	}
+
+	// module accounts that are allowed to receive tokens
+	allowedReceivingModAcc = map[string]bool{
+		distr.ModuleName: true,
+	}
 )
 
 // Verify app interface at compile time
@@ -199,7 +204,7 @@ func NewApp(logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest bool,
 	app.bankKeeper = bank.NewBaseKeeper(
 		app.accountKeeper,
 		bankSubspace,
-		app.ModuleAccountAddrs(),
+		app.BlacklistedAccAddrs(),
 	)
 	app.supplyKeeper = supply.NewKeeper(
 		app.cdc,
@@ -273,7 +278,7 @@ func NewApp(logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest bool,
 	app.committeeKeeper = committee.NewKeeper(
 		app.cdc,
 		keys[committee.StoreKey],
-		committeeGovRouter, // TODO blacklist module addresses?
+		committeeGovRouter,
 		app.paramsKeeper,
 	)
 
@@ -321,6 +326,7 @@ func NewApp(logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest bool,
 		app.auctionKeeper,
 		app.supplyKeeper,
 		app.accountKeeper,
+		mAccPerms,
 	)
 	app.bep3Keeper = bep3.NewKeeper(
 		app.cdc,
@@ -328,6 +334,7 @@ func NewApp(logger log.Logger, db dbm.DB, traceStore io.Writer, loadLatest bool,
 		app.supplyKeeper,
 		app.accountKeeper,
 		bep3Subspace,
+		app.ModuleAccountAddrs(),
 	)
 	app.kavadistKeeper = kavadist.NewKeeper(
 		app.cdc,
@@ -505,6 +512,16 @@ func (app *App) ModuleAccountAddrs() map[string]bool {
 	return modAccAddrs
 }
 
+// BlacklistedAccAddrs returns all the app's module account addresses black listed for receiving tokens.
+func (app *App) BlacklistedAccAddrs() map[string]bool {
+	blacklistedAddrs := make(map[string]bool)
+	for acc := range mAccPerms {
+		blacklistedAddrs[supply.NewModuleAddress(acc).String()] = !allowedReceivingModAcc[acc]
+	}
+
+	return blacklistedAddrs
+}
+
 // Codec returns the application's sealed codec.
 func (app *App) Codec() *codec.Codec {
 	return app.cdc

app/app_test.go

@@ -31,7 +31,7 @@ func TestBlackListedAddrs(t *testing.T) {
 	app := NewApp(log.NewTMLogger(log.NewSyncWriter(os.Stdout)), db, nil, true, map[int64]bool{}, 0)
 
 	for acc := range mAccPerms {
-		require.True(t, app.bankKeeper.BlacklistedAddr(app.supplyKeeper.GetModuleAddress(acc)))
+		require.Equal(t, !allowedReceivingModAcc[acc], app.bankKeeper.BlacklistedAddr(app.supplyKeeper.GetModuleAddress(acc)))
 	}
 }
 

x/bep3/handler.go

@@ -24,9 +24,8 @@ func NewHandler(k Keeper) sdk.Handler {
 
 // handleMsgCreateAtomicSwap handles requests to create a new AtomicSwap
 func handleMsgCreateAtomicSwap(ctx sdk.Context, k Keeper, msg MsgCreateAtomicSwap) (*sdk.Result, error) {
-	err := k.CreateAtomicSwap(ctx, msg.RandomNumberHash, msg.Timestamp, msg.HeightSpan, msg.From, msg.To,
-		msg.SenderOtherChain, msg.RecipientOtherChain, msg.Amount, true)
-
+	err := k.CreateAtomicSwap(ctx, msg.RandomNumberHash, msg.Timestamp, msg.HeightSpan,
+		msg.From, msg.To, msg.SenderOtherChain, msg.RecipientOtherChain, msg.Amount, true)
 	if err != nil {
 		return nil, err
 	}

x/bep3/keeper/keeper.go

@@ -7,7 +7,6 @@ import (
 	"github.com/cosmos/cosmos-sdk/store/prefix"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/params/subspace"
-
 	"github.com/tendermint/tendermint/libs/log"
 
 	"github.com/kava-labs/kava/x/bep3/types"
@@ -20,13 +19,12 @@ type Keeper struct {
 	paramSubspace subspace.Subspace
 	supplyKeeper  types.SupplyKeeper
 	accountKeeper types.AccountKeeper
+	Maccs         map[string]bool
 }
 
 // NewKeeper creates a bep3 keeper
-func NewKeeper(cdc *codec.Codec, key sdk.StoreKey,
-	sk types.SupplyKeeper, ak types.AccountKeeper,
-	paramstore subspace.Subspace,
-) Keeper {
+func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, sk types.SupplyKeeper, ak types.AccountKeeper,
+	paramstore subspace.Subspace, maccs map[string]bool) Keeper {
 	if !paramstore.HasKeyTable() {
 		paramstore = paramstore.WithKeyTable(types.ParamKeyTable())
 	}
@@ -37,6 +35,7 @@ func NewKeeper(cdc *codec.Codec, key sdk.StoreKey,
 		paramSubspace: paramstore,
 		supplyKeeper:  sk,
 		accountKeeper: ak,
+		Maccs:         maccs,
 	}
 	return keeper
 }

x/bep3/keeper/swap.go

@@ -23,6 +23,11 @@ func (k Keeper) CreateAtomicSwap(ctx sdk.Context, randomNumberHash []byte, times
 		return sdkerrors.Wrap(types.ErrAtomicSwapAlreadyExists, hex.EncodeToString(swapID))
 	}
 
+	// Cannot send coins to a module account
+	if k.Maccs[recipient.String()] {
+		return sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "%s is a module account", recipient)
+	}
+
 	// The heightSpan period should be more than 10 minutes and less than one week
 	// Assume average block time interval is 10 second. 10 mins = 60 blocks, 1 week = 60480 blocks
 	if heightSpan < k.GetMinBlockLock(ctx) || heightSpan > k.GetMaxBlockLock(ctx) {

x/bep3/keeper/swap_test.go

@@ -24,6 +24,7 @@ type AtomicSwapTestSuite struct {
 	keeper             keeper.Keeper
 	app                app.TestApp
 	ctx                sdk.Context
+	randMacc           sdk.AccAddress
 	deputy             sdk.AccAddress
 	addrs              []sdk.AccAddress
 	timestamps         []int64
@@ -62,6 +63,20 @@ func (suite *AtomicSwapTestSuite) SetupTest() {
 	suite.deputy = deputy
 	suite.addrs = addrs
 	suite.keeper = suite.app.GetBep3Keeper()
+
+	// Load a random module account to test blacklisting
+	i := 0
+	var randModuleAcc sdk.AccAddress
+	for macc := range suite.keeper.Maccs {
+		if i == len(suite.keeper.Maccs)/2 {
+			acc, err := sdk.AccAddressFromBech32(macc)
+			suite.Nil(err)
+			randModuleAcc = acc
+		}
+		i = i + 1
+	}
+	suite.randMacc = randModuleAcc
+
 	suite.GenerateSwapDetails()
 }
 
@@ -286,6 +301,24 @@ func (suite *AtomicSwapTestSuite) TestCreateAtomicSwap() {
 			false,
 			true,
 		},
+		{
+			"recipient is module account",
+			currentTmTime,
+			args{
+				randomNumberHash:    suite.randomNumberHashes[8],
+				timestamp:           suite.timestamps[8],
+				heightSpan:          uint64(360),
+				sender:              suite.deputy,
+				recipient:           suite.randMacc,
+				senderOtherChain:    TestSenderOtherChain,
+				recipientOtherChain: TestRecipientOtherChain,
+				coins:               cs(c(BNB_DENOM, 5000)),
+				crossChain:          true,
+				direction:           types.Incoming,
+			},
+			false,
+			false,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -624,7 +657,7 @@ func (suite *AtomicSwapTestSuite) TestRefundAtomicSwap() {
 			}
 
 			err := suite.keeper.CreateAtomicSwap(suite.ctx, suite.randomNumberHashes[i], suite.timestamps[i],
-				uint64(360), sender, suite.addrs[8], TestSenderOtherChain, TestRecipientOtherChain,
+				uint64(360), sender, suite.addrs[9], TestSenderOtherChain, TestRecipientOtherChain,
 				expectedRefundAmount, true)
 			suite.NoError(err)
 

x/cdp/keeper/keeper.go

@@ -20,10 +20,12 @@ type Keeper struct {
 	supplyKeeper    types.SupplyKeeper
 	auctionKeeper   types.AuctionKeeper
 	accountKeeper   types.AccountKeeper
+	maccPerms       map[string][]string
 }
 
 // NewKeeper creates a new keeper
-func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, paramstore subspace.Subspace, pfk types.PricefeedKeeper, ak types.AuctionKeeper, sk types.SupplyKeeper, ack types.AccountKeeper) Keeper {
+func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, paramstore subspace.Subspace, pfk types.PricefeedKeeper,
+	ak types.AuctionKeeper, sk types.SupplyKeeper, ack types.AccountKeeper, maccs map[string][]string) Keeper {
 	if !paramstore.HasKeyTable() {
 		paramstore = paramstore.WithKeyTable(types.ParamKeyTable())
 	}
@@ -36,6 +38,7 @@ func NewKeeper(cdc *codec.Codec, key sdk.StoreKey, paramstore subspace.Subspace,
 		auctionKeeper:   ak,
 		supplyKeeper:    sk,
 		accountKeeper:   ack,
+		maccPerms:       maccs,
 	}
 }
 

x/cdp/keeper/savings.go

@@ -7,12 +7,9 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	authexported "github.com/cosmos/cosmos-sdk/x/auth/exported"
-	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	supplyexported "github.com/cosmos/cosmos-sdk/x/supply/exported"
 
-	auctiontypes "github.com/kava-labs/kava/x/auction/types"
 	"github.com/kava-labs/kava/x/cdp/types"
-	kdtypes "github.com/kava-labs/kava/x/kavadist/types"
 )
 
 // DistributeSavingsRate distributes surplus that has accumulated in the liquidator account to address holding stable coins according the the savings rate
@@ -81,16 +78,14 @@ func (k Keeper) SetPreviousSavingsDistribution(ctx sdk.Context, distTime time.Ti
 	store.Set([]byte{}, k.cdc.MustMarshalBinaryLengthPrefixed(distTime))
 }
 
+// getModuleAccountCoins gets the total coin balance of this coin currently held by module accounts
 func (k Keeper) getModuleAccountCoins(ctx sdk.Context, denom string) sdk.Coins {
-	// NOTE: these are the module accounts that could end up holding stable denoms at some point.
-	// Since there are currently no api methods to 'GetAllModuleAccounts', this function will need to be updated if a
-	// new module account is added which can hold stable denoms.
-	savingsRateMaccCoinAmount := k.supplyKeeper.GetModuleAccount(ctx, types.SavingsRateMacc).GetCoins().AmountOf(denom)
-	cdpMaccCoinAmount := k.supplyKeeper.GetModuleAccount(ctx, types.ModuleName).GetCoins().AmountOf(denom)
-	auctionMaccCoinAmount := k.supplyKeeper.GetModuleAccount(ctx, auctiontypes.ModuleName).GetCoins().AmountOf(denom)
-	liquidatorMaccCoinAmount := k.supplyKeeper.GetModuleAccount(ctx, types.LiquidatorMacc).GetCoins().AmountOf(denom)
-	feeMaccCoinAmount := k.supplyKeeper.GetModuleAccount(ctx, authtypes.FeeCollectorName).GetCoins().AmountOf(denom)
-	kavadistMaccCoinAmount := k.supplyKeeper.GetModuleAccount(ctx, kdtypes.ModuleName).GetCoins().AmountOf(denom)
-	totalModAccountAmount := savingsRateMaccCoinAmount.Add(cdpMaccCoinAmount).Add(auctionMaccCoinAmount).Add(liquidatorMaccCoinAmount).Add(feeMaccCoinAmount).Add(kavadistMaccCoinAmount)
-	return sdk.NewCoins(sdk.NewCoin(denom, totalModAccountAmount))
+	totalModCoinBalance := sdk.NewCoins(sdk.NewCoin(denom, sdk.ZeroInt()))
+	for macc := range k.maccPerms {
+		modCoinBalance := k.supplyKeeper.GetModuleAccount(ctx, macc).GetCoins().AmountOf(denom)
+		if modCoinBalance.IsPositive() {
+			totalModCoinBalance = totalModCoinBalance.Add(sdk.NewCoin(denom, modCoinBalance))
+		}
+	}
+	return totalModCoinBalance
 }