diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f07f38a4..9b751d706 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 0.6.2 +BUG FIXES +* [\#609](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 diff --git a/common/tx/ante.go b/common/tx/ante.go index 2ba97c1df..6e41faf33 100644 --- a/common/tx/ante.go +++ b/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("wrong number of messages") + } + } signerAddrs := tx.GetSigners() if len(sigs) != len(signerAddrs) { return sdk.ErrUnauthorized("wrong number of signers") diff --git a/common/tx/ante_test.go b/common/tx/ante_test.go index d38b9caaa..d5a7e3c5e 100644 --- a/common/tx/ante_test.go +++ b/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) +} diff --git a/version/version.go b/version/version.go index 9dca5d082..02c374777 100644 --- a/version/version.go +++ b/version/version.go @@ -12,7 +12,7 @@ var ( Version string ) -const NodeVersion = "0.6.1" +const NodeVersion = "0.6.2" func init() { Version = fmt.Sprintf("Binance Chain Release: %s;", NodeVersion)