Merge PR: add check to send/multi-send in token module (#307)

* FIX: add check of blocked addr and send disable to token send/multisend * FIX: backend ut * FINISH: BlockedAddrSend ut * OPTIMIZE: annotation in ut * OPTIMIZE: strengthen SendCoinsFromAccountToAccount in token Co-authored-by: MengXiangJian <[email protected]>
okx · Aug 3, 2020 · 0fa8a15 · 0fa8a15
1 parent 0982393
commit 0fa8a15
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 7 deletions.
diff --git a/app/protocol/protocol_v0.go b/app/protocol/protocol_v0.go
@@ -343,7 +343,7 @@ func (p *ProtocolV0) produceKeepers() {
 func (p *ProtocolV0) moduleAccountAddrs() map[string]bool {
 	modAccAddrs := make(map[string]bool)
 	for acc := range maccPerms {
-		modAccAddrs[p.supplyKeeper.GetModuleAddress(acc).String()] = true
+		modAccAddrs[supply.NewModuleAddress(acc).String()] = true
 	}
 
 	return modAccAddrs

diff --git a/x/backend/mock_test.go b/x/backend/mock_test.go
@@ -160,7 +160,7 @@ func getMockApp(t *testing.T, numGenAccs int, enableBackend bool, dbDir string)
 	mockApp.QueryRouter().AddRoute(token.QuerierRoute, token.NewQuerier(mockApp.tokenKeeper))
 
 	mockApp.SetEndBlocker(getEndBlocker(mockApp.orderKeeper, mockApp.backendKeeper))
-	mockApp.SetInitChainer(getInitChainer(mockApp.App, mockApp.supplyKeeper,
+	mockApp.SetInitChainer(getInitChainer(mockApp.App, mockApp.bankKeeper, mockApp.supplyKeeper,
 		[]exported.ModuleAccountI{feeCollector}))
 
 	intQuantity := 100
@@ -200,14 +200,15 @@ func getEndBlocker(orderKeeper keeper.Keeper, backendKeeper Keeper) sdk.EndBlock
 	}
 }
 
-func getInitChainer(mapp *mock.App, supplyKeeper supply.Keeper,
+func getInitChainer(mapp *mock.App, bankKeeper bank.Keeper, supplyKeeper supply.Keeper,
 	blacklistedAddrs []exported.ModuleAccountI) sdk.InitChainer {
 	return func(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain {
 		mapp.InitChainer(ctx, req)
 		// set module accounts
 		for _, macc := range blacklistedAddrs {
 			supplyKeeper.SetModuleAccount(ctx, macc)
 		}
+		bankKeeper.SetSendEnabled(ctx, true)
 		supplyKeeper.SetSupply(ctx, supply.NewSupply(sdk.Coins{}))
 		return abci.ResponseInitChain{}
 	}

diff --git a/x/token/handler.go b/x/token/handler.go
@@ -252,6 +252,10 @@ func handleMsgTokenMint(ctx sdk.Context, keeper Keeper, msg types.MsgTokenMint,
 }
 
 func handleMsgMultiSend(ctx sdk.Context, keeper Keeper, msg types.MsgMultiSend, logger log.Logger) sdk.Result {
+	if !keeper.bankKeeper.GetSendEnabled(ctx) {
+		return types.ErrSendDisabled(DefaultCodespace).Result()
+	}
+
 	var transfers string
 	var coinNum int
 	for _, transferUnit := range msg.Transfers {
@@ -282,6 +286,9 @@ func handleMsgMultiSend(ctx sdk.Context, keeper Keeper, msg types.MsgMultiSend,
 }
 
 func handleMsgSend(ctx sdk.Context, keeper Keeper, msg types.MsgSend, logger log.Logger) sdk.Result {
+	if !keeper.bankKeeper.GetSendEnabled(ctx) {
+		return types.ErrSendDisabled(DefaultCodespace).Result()
+	}
 
 	err := keeper.SendCoinsFromAccountToAccount(ctx, msg.FromAddress, msg.ToAddress, msg.Amount)
 	if err != nil {

diff --git a/x/token/keeper.go b/x/token/keeper.go
@@ -164,6 +164,10 @@ func (k Keeper) NewToken(ctx sdk.Context, token types.Token) {
 
 // send tokens(one or more coins) from one account to another
 func (k Keeper) SendCoinsFromAccountToAccount(ctx sdk.Context, from, to sdk.AccAddress, amt sdk.DecCoins) error {
+	if k.bankKeeper.BlacklistedAddr(to) {
+		return types.ErrBlockedRecipient(DefaultCodespace, to.String())
+	}
+
 	return k.bankKeeper.SendCoins(ctx, from, to, amt)
 }
 

diff --git a/x/token/token_test.go b/x/token/token_test.go
@@ -74,7 +74,7 @@ func getMockDexApp(t *testing.T, numGenAccs int) (mockDexApp *MockDexApp, keeper
 
 	feeCollectorAcc := supply.NewEmptyModuleAccount(auth.FeeCollectorName)
 	blacklistedAddrs := make(map[string]bool)
-	blacklistedAddrs[feeCollectorAcc.String()] = true
+	blacklistedAddrs[feeCollectorAcc.Address.String()] = true
 
 	mockDexApp.bankKeeper = bank.NewBaseKeeper(
 		mockDexApp.AccountKeeper,
@@ -105,7 +105,7 @@ func getMockDexApp(t *testing.T, numGenAccs int) (mockDexApp *MockDexApp, keeper
 	mockDexApp.QueryRouter().AddRoute(QuerierRoute, NewQuerier(mockDexApp.tokenKeeper))
 
 	mockDexApp.SetEndBlocker(getEndBlocker(mockDexApp.tokenKeeper))
-	mockDexApp.SetInitChainer(getInitChainer(mockDexApp.App, mockDexApp.supplyKeeper, []exported.ModuleAccountI{feeCollectorAcc}))
+	mockDexApp.SetInitChainer(getInitChainer(mockDexApp.App, mockDexApp.bankKeeper, mockDexApp.supplyKeeper, []exported.ModuleAccountI{feeCollectorAcc}))
 
 	intQuantity := int64(100)
 	valTokens := sdk.NewDec(intQuantity)
@@ -223,7 +223,7 @@ func getMockDexAppEx(t *testing.T, numGenAccs int) (mockDexApp *MockDexApp, keep
 	mockDexApp.QueryRouter().AddRoute(QuerierRoute, NewQuerier(mockDexApp.tokenKeeper))
 
 	mockDexApp.SetEndBlocker(getEndBlocker(mockDexApp.tokenKeeper))
-	mockDexApp.SetInitChainer(getInitChainer(mockDexApp.App, mockDexApp.supplyKeeper, []exported.ModuleAccountI{feeCollectorAcc}))
+	mockDexApp.SetInitChainer(getInitChainer(mockDexApp.App, mockDexApp.bankKeeper, mockDexApp.supplyKeeper, []exported.ModuleAccountI{feeCollectorAcc}))
 
 	intQuantity := int64(10000000)
 	valTokens := sdk.NewDec(intQuantity)
@@ -253,14 +253,15 @@ func getMockDexAppEx(t *testing.T, numGenAccs int) (mockDexApp *MockDexApp, keep
 	return mockDexApp, mockDexApp.tokenKeeper, handler
 }
 
-func getInitChainer(mapp *mock.App, supplyKeeper supply.Keeper,
+func getInitChainer(mapp *mock.App, bankKeeper bank.Keeper, supplyKeeper supply.Keeper,
 	blacklistedAddrs []exported.ModuleAccountI) sdk.InitChainer {
 	return func(ctx sdk.Context, req abci.RequestInitChain) abci.ResponseInitChain {
 		mapp.InitChainer(ctx, req)
 		// set module accounts
 		for _, macc := range blacklistedAddrs {
 			supplyKeeper.SetModuleAccount(ctx, macc)
 		}
+		bankKeeper.SetSendEnabled(ctx, true)
 		supplyKeeper.SetSupply(ctx, supply.NewSupply(sdk.Coins{}))
 		return abci.ResponseInitChain{}
 	}
@@ -1077,3 +1078,51 @@ func TestTxSuccessFeeTable(t *testing.T) {
 		})
 	}
 }
+
+func TestBlockedAddrSend(t *testing.T) {
+	app, testAccounts := getMockAppToHandleFee(t, 30000, 1)
+	ctx := app.BaseApp.NewContext(true, abci.Header{})
+	// blocked addr
+	blockedAddr := supply.NewModuleAddress(auth.FeeCollectorName)
+	// unblocked addr
+	toPriKey := secp256k1.GenPrivKey()
+	toPubKey := toPriKey.PubKey()
+	validAddr := sdk.AccAddress(toPubKey.Address())
+
+	// build send msg
+	decCoin := sdk.NewDecCoinFromDec(common.NativeToken, sdk.NewDec(50))
+	successfulSendMsg := types.NewMsgTokenSend(testAccounts[0].baseAccount.Address, validAddr, sdk.DecCoins{decCoin})
+	failedSendMsg := types.NewMsgTokenSend(testAccounts[0].baseAccount.Address, blockedAddr, sdk.DecCoins{decCoin})
+
+	// build multi-send msg
+	multiSendStr := `[{"to":"` + validAddr.String() + `","amount":" 100` + common.NativeToken + `"}]`
+	transfers, err := types.StrToTransfers(multiSendStr)
+	require.NoError(t, err)
+	successfulMultiSendMsg := types.NewMsgMultiSend(testAccounts[0].baseAccount.Address, transfers)
+
+	multiSendStr = `[{"to":"` + blockedAddr.String() + `","amount":" 100` + common.NativeToken + `"}]`
+	transfers, err = types.StrToTransfers(multiSendStr)
+	require.NoError(t, err)
+	failedMultiSendMsg := types.NewMsgMultiSend(testAccounts[0].baseAccount.Address, transfers)
+
+	successfulTestSets := []struct {
+		description string
+		balance     string
+		msg         sdk.Msg
+		account     *testAccount
+	}{
+		// 0.01okt as fixed fee in each stdTx
+		{"success to send  : 50+0.01", "29949.99000000", successfulSendMsg, testAccounts[0]},
+		{"fail to send  : 0.01", "29949.98000000", failedSendMsg, testAccounts[0]},
+		{"success to multi-send  : 100+0.01", "29849.97000000", successfulMultiSendMsg, testAccounts[0]},
+		{"fail to multi-send  : 0.01", "29849.96000000", failedMultiSendMsg, testAccounts[0]},
+	}
+	for i, tt := range successfulTestSets {
+		t.Run(tt.description, func(t *testing.T) {
+			stdTx := createTokenMsg(t, app, ctx, tt.account, tt.msg)
+			ctx = mockApplyBlock(t, app, []auth.StdTx{stdTx}, int64(i+3))
+			require.Equal(t, tt.balance, app.AccountKeeper.GetAccount(ctx, testAccounts[0].addrKeys.Address).GetCoins().AmountOf(common.NativeToken).String())
+		})
+	}
+
+}
diff --git a/x/token/types/errors.go b/x/token/types/errors.go
@@ -14,8 +14,20 @@ const (
 	CodeInvalidHeight           sdk.CodeType = 5
 	CodeInvalidAsset            sdk.CodeType = 6
 	CodeInvalidCommon           sdk.CodeType = 7
+	CodeBlockedRecipient        sdk.CodeType = 8
+	CodeSendDisabled            sdk.CodeType = 9
 )
 
+// ErrBlockedRecipient returns an error when a transfer is tried on a blocked recipient
+func ErrBlockedRecipient(codespace sdk.CodespaceType, blockedAddr string) sdk.Error {
+	return sdk.NewError(codespace, CodeBlockedRecipient, "failed. %s is not allowed to receive transactions", blockedAddr)
+}
+
+// ErrSendDisabled returns an error when the transaction sending is disabled in bank module
+func ErrSendDisabled(codespace sdk.CodespaceType) sdk.Error {
+	return sdk.NewError(codespace, CodeSendDisabled, "failed. send transactions are currently disabled")
+}
+
 func ErrInvalidDexList(codespace sdk.CodespaceType, message string) sdk.Error {
 	return sdk.NewError(codespace, CodeInvalidDexList, message)
 }