From 5f7060dadfa9c56fafe0d82242a04c6efef014a5 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 15 Aug 2018 17:22:32 -0700 Subject: [PATCH 1/3] chain: handle ZMQ timeout error correctly --- chain/bitcoind_conn.go | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/chain/bitcoind_conn.go b/chain/bitcoind_conn.go index 963afe3..ecc3049 100644 --- a/chain/bitcoind_conn.go +++ b/chain/bitcoind_conn.go @@ -168,7 +168,7 @@ func (c *BitcoindConn) blockEventHandler(conn *gozmq.Conn) { defer c.wg.Done() defer conn.Close() - log.Info("Started listening for bitcoind block notifications via ZMQ ", + log.Info("Started listening for bitcoind block notifications via ZMQ "+ "on", c.zmqBlockHost) for { @@ -180,16 +180,18 @@ func (c *BitcoindConn) blockEventHandler(conn *gozmq.Conn) { default: } - // Poll an event from the ZMQ socket. It's possible that the - // connection to the socket continuously times out, so we'll - // prevent logging this error to prevent spamming the logs. + // Poll an event from the ZMQ socket. msgBytes, err := conn.Receive() if err != nil { - err, ok := err.(net.Error) - if !ok || !err.Timeout() { - log.Error(err) + // It's possible that the connection to the socket + // continuously times out, so we'll prevent logging this + // error to prevent spamming the logs. + netErr, ok := err.(net.Error) + if ok && netErr.Timeout() { + continue } + log.Errorf("Unable to receive ZMQ message: %v", err) continue } @@ -234,7 +236,7 @@ func (c *BitcoindConn) txEventHandler(conn *gozmq.Conn) { defer conn.Close() log.Info("Started listening for bitcoind transaction notifications "+ - "via ZMQ on ", c.zmqTxHost) + "via ZMQ on", c.zmqTxHost) for { // Before attempting to read from the ZMQ socket, we'll make @@ -245,16 +247,18 @@ func (c *BitcoindConn) txEventHandler(conn *gozmq.Conn) { default: } - // Poll an event from the ZMQ socket. It's possible that the - // connection to the socket continuously times out, so we'll - // prevent logging this error to prevent spamming the logs. + // Poll an event from the ZMQ socket. msgBytes, err := conn.Receive() if err != nil { - err, ok := err.(net.Error) - if !ok || !err.Timeout() { - log.Error(err) + // It's possible that the connection to the socket + // continuously times out, so we'll prevent logging this + // error to prevent spamming the logs. + netErr, ok := err.(net.Error) + if ok && netErr.Timeout() { + continue } + log.Errorf("Unable to receive ZMQ message: %v", err) continue } From d6155a55e9654aec730ccfd0c5d03aa5131e0154 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 15 Aug 2018 17:22:50 -0700 Subject: [PATCH 2/3] chain: improve error when filtering blocks and transactions --- chain/bitcoind_client.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/chain/bitcoind_client.go b/chain/bitcoind_client.go index a136c00..75cc078 100644 --- a/chain/bitcoind_client.go +++ b/chain/bitcoind_client.go @@ -549,7 +549,8 @@ func (c *BitcoindClient) ntfnHandler() { select { case tx := <-c.zmqTxNtfns: if _, _, err := c.filterTx(tx, nil, true); err != nil { - log.Error(err) + log.Errorf("Unable to filter transaction %v: %v", + tx.TxHash(), err) } case newBlock := <-c.zmqBlockNtfns: // If the new block's previous hash matches the best @@ -566,7 +567,8 @@ func (c *BitcoindClient) ntfnHandler() { newBlock, newBlockHeight, true, ) if err != nil { - log.Error(err) + log.Errorf("Unable to filter block %v: %v", + newBlock.BlockHash(), err) continue } From 850ad0959ac63907361c28ab03a03236e0d343c1 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 15 Aug 2018 18:17:29 -0700 Subject: [PATCH 3/3] chain: ensure eventType from ZMQ response is human-readable In this commit, we fix a small issue where it's possible that we read a malformed message from the ZMQ connection to bitcoind due to it shutting down. To fix this, we ensure that the event type is human readable before attempting to log it. --- chain/bitcoind_conn.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/chain/bitcoind_conn.go b/chain/bitcoind_conn.go index ecc3049..8a90134 100644 --- a/chain/bitcoind_conn.go +++ b/chain/bitcoind_conn.go @@ -221,6 +221,14 @@ func (c *BitcoindConn) blockEventHandler(conn *gozmq.Conn) { } c.rescanClientsMtx.Unlock() default: + // It's possible that the message wasn't fully read if + // bitcoind shuts down, which will produce an unreadable + // event type. To prevent from logging it, we'll make + // sure it conforms to the ASCII standard. + if !isASCII(eventType) { + continue + } + log.Warnf("Received unexpected event type from "+ "rawblock subscription: %v", eventType) } @@ -364,3 +372,14 @@ func (c *BitcoindConn) RemoveClient(id uint64) { delete(c.rescanClients, id) } + +// isASCII is a helper method that checks whether all bytes in `data` would be +// printable ASCII characters if interpreted as a string. +func isASCII(s string) bool { + for _, c := range s { + if c < 32 || c > 126 { + return false + } + } + return true +}