From f95360f8ba281bfb4e109ee32192bfade762f3ca Mon Sep 17 00:00:00 2001 From: Chris Stewart Date: Sat, 8 Apr 2023 16:12:03 -0500 Subject: [PATCH] Add logic to fetch block headers after compact filters are synced in IBD to avoid a stale tip (#5037) * Add logic to fetch block headers after compact filters are synced in IBD to avoid a stale tip * Restore logging, remove println * Add comment linking to issue * Cleanup logging * Only send getheaders message after filter sync if we are in IBD --- .../org/bitcoins/node/NeutrinoNodeTest.scala | 32 +++++++++++++++++++ .../networking/peer/DataMessageHandler.scala | 21 +++++++++--- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/node-test/src/test/scala/org/bitcoins/node/NeutrinoNodeTest.scala b/node-test/src/test/scala/org/bitcoins/node/NeutrinoNodeTest.scala index 1f9306b110..7cab5d4eec 100644 --- a/node-test/src/test/scala/org/bitcoins/node/NeutrinoNodeTest.scala +++ b/node-test/src/test/scala/org/bitcoins/node/NeutrinoNodeTest.scala @@ -306,6 +306,38 @@ class NeutrinoNodeTest extends NodeTestWithCachedBitcoindPair { } yield { succeed } + } + it must "sync block headers that occurred while were syncing compact filters during IBD" in { + nodeConnectedWithBitcoind: NeutrinoNodeConnectedWithBitcoinds => + //see: https://github.com/bitcoin-s/bitcoin-s/issues/5017 + val node = nodeConnectedWithBitcoind.node + val bitcoind = nodeConnectedWithBitcoind.bitcoinds(0) + + //start syncing node + val numBlocks = 5 + val startSyncF = node.sync() + val genBlocksF = { + for { + _ <- startSyncF + //give a little time for the sync to start + _ <- AsyncUtil.nonBlockingSleep(500.milliseconds) + //generate blocks while sync is ongoing + _ <- bitcoind.generate(numBlocks) + } yield { + () + } + } + + for { + _ <- genBlocksF + //wait for sync to complete + _ <- NodeTestUtil.awaitAllSync(node, bitcoind) + //generate another block and make sure it syncs it + _ <- bitcoind.generate(1) + _ <- NodeTestUtil.awaitAllSync(node, bitcoind) + } yield { + succeed + } } } diff --git a/node/src/main/scala/org/bitcoins/node/networking/peer/DataMessageHandler.scala b/node/src/main/scala/org/bitcoins/node/networking/peer/DataMessageHandler.scala index 8abc20c482..f70f4f429c 100644 --- a/node/src/main/scala/org/bitcoins/node/networking/peer/DataMessageHandler.scala +++ b/node/src/main/scala/org/bitcoins/node/networking/peer/DataMessageHandler.scala @@ -136,7 +136,7 @@ case class DataMessageHandler( newChainApi <- chainApi.processFilterHeaders( filterHeaders, filterHeader.stopHash.flip) - (newSyncing, startFilterHeightOpt) <- + (newSyncing, _) <- if (filterHeaders.size == chainConfig.filterHeaderBatchSize) { logger.debug( s"Received maximum amount of filter headers in one header message. This means we are not synced, requesting more") @@ -150,7 +150,8 @@ case class DataMessageHandler( syncing <- sendFirstGetCompactFilterCommand( peerMsgSender, startHeightOpt).map { syncing => - if (!syncing) logger.info("We are synced") + if (!syncing) + logger.info("Compact filters are already synced") syncing } } yield (syncing, startHeightOpt) @@ -499,7 +500,7 @@ case class DataMessageHandler( } /** syncs filter headers in case the header chain is still ahead post filter sync */ - def syncIfHeadersAhead( + private def syncIfHeadersAhead( peerMessageSender: PeerMessageSender): Future[Boolean] = { for { headerHeight <- chainApi.getBestHashBlockHeight() @@ -523,7 +524,19 @@ case class DataMessageHandler( s"headerHeight=$headerHeight filterCount=$filterCount") logger.info(s"We are synced") Try(initialSyncDone.map(_.success(Done))) - Future.successful(false) + //check to see if we had blocks mined while IBD + //was ongoing, see: https://github.com/bitcoin-s/bitcoin-s/issues/5036 + for { + bestBlockHash <- chainApi.getBestBlockHash() + isIBD <- chainApi.isIBD() + _ <- { + if (isIBD) { + peerMessageSender.sendGetHeadersMessage(bestBlockHash.flip) + } else { + Future.unit + } + } + } yield false } } } yield syncing