Skip to content

Commit

Permalink
hot fix panic is not recovered in prechecker
Browse files Browse the repository at this point in the history
  • Loading branch information
ackratos committed Aug 1, 2019
1 parent 5ce5c59 commit ede2029
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 2 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
16 changes: 15 additions & 1 deletion common/tx/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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")
Expand Down
33 changes: 33 additions & 0 deletions common/tx/ante_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit ede2029

Please sign in to comment.