diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index f4a10102f..a1a121164 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -20,6 +20,7 @@ program](https://hackerone.com/tendermint). ### IMPROVEMENTS: +- [mempool] [\#4057](https://github.com/tendermint/tendermint/issues/4057) Include peer ID when logging rejected txns (@erikgrinaker) - [tools] [\#4023](https://github.com/tendermint/tendermint/issues/4023) Improved `tm-monitor` formatting of start time and avg tx throughput (@erikgrinaker) ### BUG FIXES: diff --git a/mempool/clist_mempool.go b/mempool/clist_mempool.go index 554839f1a..50a37e5aa 100644 --- a/mempool/clist_mempool.go +++ b/mempool/clist_mempool.go @@ -17,6 +17,7 @@ import ( "github.com/tendermint/tendermint/libs/clist" cmn "github.com/tendermint/tendermint/libs/common" "github.com/tendermint/tendermint/libs/log" + "github.com/tendermint/tendermint/p2p" "github.com/tendermint/tendermint/proxy" "github.com/tendermint/tendermint/types" ) @@ -281,7 +282,7 @@ func (mem *CListMempool) CheckTxWithInfo(tx types.Tx, cb func(*abci.Response), t } reqRes := mem.proxyAppConn.CheckTxAsync(abci.RequestCheckTx{Tx: tx}) - reqRes.SetCallback(mem.reqResCb(tx, txInfo.SenderID, cb)) + reqRes.SetCallback(mem.reqResCb(tx, txInfo.SenderID, txInfo.SenderP2PID, cb)) return nil } @@ -314,14 +315,19 @@ func (mem *CListMempool) globalCb(req *abci.Request, res *abci.Response) { // when all other response processing is complete. // // Used in CheckTxWithInfo to record PeerID who sent us the tx. -func (mem *CListMempool) reqResCb(tx []byte, peerID uint16, externalCb func(*abci.Response)) func(res *abci.Response) { +func (mem *CListMempool) reqResCb( + tx []byte, + peerID uint16, + peerP2PID p2p.ID, + externalCb func(*abci.Response), +) func(res *abci.Response) { return func(res *abci.Response) { if mem.recheckCursor != nil { // this should never happen panic("recheck cursor is not nil in reqResCb") } - mem.resCbFirstTime(tx, peerID, res) + mem.resCbFirstTime(tx, peerID, peerP2PID, res) // update metrics mem.metrics.Size.Set(float64(mem.Size())) @@ -360,7 +366,12 @@ func (mem *CListMempool) removeTx(tx types.Tx, elem *clist.CElement, removeFromC // // The case where the app checks the tx for the second and subsequent times is // handled by the resCbRecheck callback. -func (mem *CListMempool) resCbFirstTime(tx []byte, peerID uint16, res *abci.Response) { +func (mem *CListMempool) resCbFirstTime( + tx []byte, + peerID uint16, + peerP2PID p2p.ID, + res *abci.Response, +) { switch r := res.Value.(type) { case *abci.Response_CheckTx: var postCheckErr error @@ -384,7 +395,8 @@ func (mem *CListMempool) resCbFirstTime(tx []byte, peerID uint16, res *abci.Resp mem.notifyTxsAvailable() } else { // ignore bad transaction - mem.logger.Info("Rejected bad transaction", "tx", txID(tx), "res", r, "err", postCheckErr) + mem.logger.Info("Rejected bad transaction", + "tx", txID(tx), "peerID", peerP2PID, "res", r, "err", postCheckErr) mem.metrics.FailedTxs.Add(1) // remove from cache (it might be good later) mem.cache.Remove(tx) diff --git a/mempool/mempool.go b/mempool/mempool.go index 0995c734f..a979515ef 100644 --- a/mempool/mempool.go +++ b/mempool/mempool.go @@ -4,6 +4,7 @@ import ( "fmt" abci "github.com/tendermint/tendermint/abci/types" + "github.com/tendermint/tendermint/p2p" "github.com/tendermint/tendermint/types" ) @@ -90,9 +91,11 @@ type PostCheckFunc func(types.Tx, *abci.ResponseCheckTx) error // TxInfo are parameters that get passed when attempting to add a tx to the // mempool. type TxInfo struct { - // We don't use p2p.ID here because it's too big. The gain is to store max 2 - // bytes with each tx to identify the sender rather than 20 bytes. + // SenderID is the internal peer ID used in the mempool to identify the + // sender, storing 2 bytes with each tx instead of 20 bytes for the p2p.ID. SenderID uint16 + // SenderP2PID is the actual p2p.ID of the sender, used e.g. for logging. + SenderP2PID p2p.ID } //-------------------------------------------------------------------------------- diff --git a/mempool/reactor.go b/mempool/reactor.go index 349ed7ea5..35797b6c5 100644 --- a/mempool/reactor.go +++ b/mempool/reactor.go @@ -165,8 +165,11 @@ func (memR *Reactor) Receive(chID byte, src p2p.Peer, msgBytes []byte) { switch msg := msg.(type) { case *TxMessage: - peerID := memR.ids.GetForPeer(src) - err := memR.mempool.CheckTxWithInfo(msg.Tx, nil, TxInfo{SenderID: peerID}) + txInfo := TxInfo{SenderID: memR.ids.GetForPeer(src)} + if src != nil { + txInfo.SenderP2PID = src.ID() + } + err := memR.mempool.CheckTxWithInfo(msg.Tx, nil, txInfo) if err != nil { memR.Logger.Info("Could not check tx", "tx", txID(msg.Tx), "err", err) }