Merge pull request #635 from binance-chain/ante_panic

[R4R] hot fix panic is not recovered in prechecker

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 0.6.1-hf.1
+BUG FIXES
+* [\#635](https://github.com/binance-chain/node/pull/635) fix panic in pre-check is not recovered
+
 ## 0.6.1
 FEATURES
 * [\#605](https://github.com/binance-chain/node/pull/605) [Account] accounts can set flags to turn on memo validation

common/tx/ante.go

@@ -60,12 +60,19 @@ func InitSigCache(size int) {
 
 // this function is not implemented in AnteHandler in BaseApp.
 func NewTxPreChecker() sdk.PreChecker {
-	return func(ctx sdk.Context, txBytes []byte, tx sdk.Tx) sdk.Result {
+	return func(ctx sdk.Context, txBytes []byte, tx sdk.Tx) (res sdk.Result) {
 		stdTx, ok := tx.(auth.StdTx)
 		if !ok {
 			return sdk.ErrInternal("tx must be StdTx").Result()
 		}
 
+		defer func() {
+			if r := recover(); r != nil {
+				ctx.Logger().Error("failed to pre-check tx", "err", r)
+				res = sdk.ErrInternal("failed to pre-check tx").Result()
+			}
+		}()
+
 		err := validateBasic(stdTx)
 		if err != nil {
 			return err.Result()
@@ -108,6 +115,8 @@ func NewTxPreChecker() sdk.PreChecker {
 // and deducts fees from the first signer.
 // NOTE: Receiving the `NewOrder` dependency here avoids an import cycle.
 // nolint: gocyclo
+//
+// panic thrown in this function will be caught in RunTx
 func NewAnteHandler(am auth.AccountKeeper) sdk.AnteHandler {
 	return func(
 		ctx sdk.Context, tx sdk.Tx, mode sdk.RunTxMode,
@@ -205,6 +214,11 @@ func validateBasic(tx auth.StdTx) (err sdk.Error) {
 	}
 
 	// Assert that number of signatures is correct.
+	for _, msg := range tx.GetMsgs() {
+		if msg == nil {
+			return sdk.ErrUnknownRequest("msg should not be nil")
+		}
+	}
 	signerAddrs := tx.GetSigners()
 	if len(sigs) != len(signerAddrs) {
 		return sdk.ErrUnauthorized("wrong number of signers")

common/tx/ante_test.go

@@ -716,3 +716,36 @@ func Test_NewTxPreCheckerSignature(t *testing.T) {
 	res = prechecker(ctx, cdc.MustMarshalBinaryLengthPrefixed(txn), txn)
 	require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeInvalidPubKey), res.Code)
 }
+
+func Test_NewTxPreCheckerNilMsg(t *testing.T) {
+	ms, capKey, _ := testutils.SetupMultiStoreForUnitTest()
+	cdc := wire.NewCodec()
+	auth.RegisterBaseAccount(cdc)
+	sdk.RegisterCodec(cdc)
+	cdc.RegisterConcrete(sdk.TestMsg{}, "antetest/TestMsg", nil)
+	mapper := auth.NewAccountKeeper(cdc, capKey, auth.ProtoBaseAccount)
+	accountCache := getAccountCache(cdc, ms, capKey)
+
+	ctx := sdk.NewContext(ms, abci.Header{ChainID: "mychainid", Height: 1}, sdk.RunTxModeDeliver, log.NewNopLogger()).WithAccountCache(accountCache)
+
+	// keys and addresses
+	priv1, addr1 := testutils.PrivAndAddr()
+
+	// set the accounts
+	acc1 := mapper.NewAccountWithAddress(ctx, addr1)
+	acc1.SetPubKey(priv1.PubKey())
+	acc1.SetCoins(newCoins())
+	mapper.SetAccount(ctx, acc1)
+
+	var txn auth.StdTx
+	msg := newTestMsg(addr1)
+	msgs := []sdk.Msg{msg}
+
+	// test good tx and signBytes
+	privs, accnums, seqs := []crypto.PrivKey{priv1}, []int64{0}, []int64{0}
+	txn = newTestTx(ctx, msgs, privs, accnums, seqs)
+	txn.Msgs[0] = nil
+	prechecker := tx.NewTxPreChecker()
+	res := prechecker(ctx, cdc.MustMarshalBinaryLengthPrefixed(txn), txn)
+	require.Equal(t, sdk.ToABCICode(sdk.CodespaceRoot, sdk.CodeUnknownRequest), res.Code)
+}

version/version.go

@@ -12,7 +12,7 @@ var (
 	Version string
 )
 
-const NodeVersion = "0.6.1"
+const NodeVersion = "0.6.1-hf.1"
 
 func init() {
 	Version = fmt.Sprintf("Binance Chain Release: %s;", NodeVersion)