From 7b7bcbaf98828f0337fa8f800652569aad5e2cc6 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Thu, 21 Sep 2023 18:32:16 +0800 Subject: [PATCH] chain: add verbose logging when error in `getRawTxIgnoreErr` This commit modifies the method `getRawTxIgnoreErr` to log more useful info. It also removes `*mempool` as the method reciever as it's unnecessary to put it on mempool. --- chain/mempool.go | 30 +++++++++++++++++++----------- chain/mempool_test.go | 21 ++++++++------------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/chain/mempool.go b/chain/mempool.go index e3a95997f6..3ca8a821b3 100644 --- a/chain/mempool.go +++ b/chain/mempool.go @@ -153,7 +153,7 @@ type mempoolConfig struct { // // TODO(yy): interface rpcclient.FutureGetRawTransactionResult so we // can remove this hack. - rawTxReceiver func(getRawTxReceiver) *btcutil.Tx + rawTxReceiver func(chainhash.Hash, getRawTxReceiver) *btcutil.Tx } // newMempool creates a new mempool object. @@ -168,7 +168,7 @@ func newMempool(cfg *mempoolConfig) *mempool { // Mount the default methods. m.cfg.rawMempoolGetter = m.getRawMempool - m.cfg.rawTxReceiver = m.getRawTxIgnoreErr + m.cfg.rawTxReceiver = getRawTxIgnoreErr return m } @@ -482,9 +482,13 @@ func (m *mempool) batchGetRawTxes(txids []*chainhash.Hash, len(txids)/m.cfg.getRawTxBatchSize+1) defer log.Debugf("Finished batch GetRawTransaction") + // txRecievers defines a map that has the txid as its key and the tx's + // response reciever as its value. + type txRecievers map[chainhash.Hash]getRawTxReceiver + // respReceivers stores a list of response receivers returned from // batch calling `GetRawTransactionAsync`. - respReceivers := make([]getRawTxReceiver, 0, m.cfg.getRawTxBatchSize) + respReceivers := make(txRecievers, m.cfg.getRawTxBatchSize) // Conditionally init a newTxes slice. var newTxes []*wire.MsgTx @@ -496,7 +500,7 @@ func (m *mempool) batchGetRawTxes(txids []*chainhash.Hash, // bitcoind and waits for all the responses to return. Each time a // response is received, it will be used to update the local mempool // state and conditionally saved to a slice that will be returned. - processBatch := func(results []getRawTxReceiver) error { + processBatch := func(results txRecievers) error { // Ask the client to send all the batched requests. err := m.cfg.client.Send() if err != nil { @@ -504,8 +508,8 @@ func (m *mempool) batchGetRawTxes(txids []*chainhash.Hash, } // Iterate the recievers and fetch the response. - for _, resp := range results { - tx := m.cfg.rawTxReceiver(resp) + for txid, resp := range results { + tx := m.cfg.rawTxReceiver(txid, resp) if tx == nil { continue } @@ -533,7 +537,7 @@ func (m *mempool) batchGetRawTxes(txids []*chainhash.Hash, // Create the async request and save it to txRespReceivers. resp := m.cfg.client.GetRawTransactionAsync(txHash) - respReceivers = append(respReceivers, resp) + respReceivers[*txHash] = resp // When getRawTxBatchSize is reached, we'd ask the batch client // to send the requests and process the responses. @@ -555,7 +559,7 @@ func (m *mempool) batchGetRawTxes(txids []*chainhash.Hash, // Empty the slice for next batch iteration. respReceivers = make( - []getRawTxReceiver, 0, m.cfg.getRawTxBatchSize, + txRecievers, m.cfg.getRawTxBatchSize, ) } } @@ -581,7 +585,9 @@ func (m *mempool) batchGetRawTxes(txids []*chainhash.Hash, // for the txid in bitcoind's mempool. If the tx is replaced, confirmed, or not // yet included in bitcoind's mempool, the error txNotFoundErr will be // returned. -func (m *mempool) getRawTxIgnoreErr(rawTx getRawTxReceiver) *btcutil.Tx { +func getRawTxIgnoreErr(txid chainhash.Hash, + rawTx getRawTxReceiver) *btcutil.Tx { + tx, err := rawTx.Receive() // Exit early if there's no error. @@ -593,12 +599,14 @@ func (m *mempool) getRawTxIgnoreErr(rawTx getRawTxReceiver) *btcutil.Tx { errStr := strings.ToLower(err.Error()) errExp := strings.ToLower(txNotFoundErr) if strings.Contains(errStr, errExp) { - log.Debugf("unable to fetch transaction from mempool: %v", err) + log.Debugf("unable to fetch transaction %s from mempool: %v", + txid, err) } else { // Otherwise, unexpected error is found, we'll create an error // log. - log.Errorf("unable to fetch transaction from mempool: %v", err) + log.Errorf("unable to fetch transaction %s from mempool: %v", + txid, err) } return nil diff --git a/chain/mempool_test.go b/chain/mempool_test.go index b88e9bdfa1..c9599deca4 100644 --- a/chain/mempool_test.go +++ b/chain/mempool_test.go @@ -470,7 +470,9 @@ func TestUpdateMempoolTxes(t *testing.T) { m.cfg.rawMempoolGetter = func() ([]*chainhash.Hash, error) { return mempool1, nil } - m.cfg.rawTxReceiver = func(reciever getRawTxReceiver) *btcutil.Tx { + m.cfg.rawTxReceiver = func(txid chainhash.Hash, + reciever getRawTxReceiver) *btcutil.Tx { + switch reciever { case mockTx1Receiver: return btctx1 @@ -526,7 +528,9 @@ func TestUpdateMempoolTxes(t *testing.T) { m.cfg.rawMempoolGetter = func() ([]*chainhash.Hash, error) { return mempool2, nil } - m.cfg.rawTxReceiver = func(reciever getRawTxReceiver) *btcutil.Tx { + m.cfg.rawTxReceiver = func(txid chainhash.Hash, + reciever getRawTxReceiver) *btcutil.Tx { + switch reciever { case mockTx3Receiver: return btctx3 @@ -624,14 +628,6 @@ func TestUpdateMempoolTxesOnShutdown(t *testing.T) { func TestGetRawTxIgnoreErr(t *testing.T) { require := require.New(t) - // Create a mock client and init our mempool. - mockRPC := &mockRPCClient{} - m := newMempool(&mempoolConfig{ - client: mockRPC, - batchWaitInterval: 0, - getRawTxBatchSize: 1, - }) - // Create a normal transaction that has two inputs. op := wire.OutPoint{Hash: chainhash.Hash{1}} tx := &wire.MsgTx{ @@ -645,7 +641,7 @@ func TestGetRawTxIgnoreErr(t *testing.T) { mockReceiver.On("Receive").Return(btctx, nil).Once() // Call the method and expect the tx to be returned. - resp := m.getRawTxIgnoreErr(mockReceiver) + resp := getRawTxIgnoreErr(tx.TxHash(), mockReceiver) require.Equal(btctx, resp) // Mock the reciever to return an error. @@ -653,10 +649,9 @@ func TestGetRawTxIgnoreErr(t *testing.T) { mockReceiver.On("Receive").Return(nil, dummyErr).Once() // Call the method again and expect nil response. - resp = m.getRawTxIgnoreErr(mockReceiver) + resp = getRawTxIgnoreErr(tx.TxHash(), mockReceiver) require.Nil(resp) // Assert the mock client was called as expected. - mockRPC.AssertExpectations(t) mockReceiver.AssertExpectations(t) }