<p></p>
<p><b>@sqrrm</b> commented on this pull request.</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/4851#discussion_r536136447">core/src/main/java/bisq/core/dao/node/lite/LiteNode.java</a>:</p>
<pre style='color:#555'>> +        } else {
+            if (blockDownloadListener == null) {
+                blockDownloadListener = (observable, oldValue, newValue) -> {
+                    if ((double) newValue == 1) {
+                        setupWalletBestBlockListener();
+                        UserThread.execute(() -> walletsSetup.downloadPercentageProperty().removeListener(blockDownloadListener));
+                    }
+                };
+                walletsSetup.downloadPercentageProperty().addListener(blockDownloadListener);
+            }
+        }
</pre>

⬇️ Suggested change
<pre style="color: #555">-        } else {
-            if (blockDownloadListener == null) {
-                blockDownloadListener = (observable, oldValue, newValue) -> {
-                    if ((double) newValue == 1) {
-                        setupWalletBestBlockListener();
-                        UserThread.execute(() -> walletsSetup.downloadPercentageProperty().removeListener(blockDownloadListener));
-                    }
-                };
-                walletsSetup.downloadPercentageProperty().addListener(blockDownloadListener);
-            }
-        }
+        } else if (blockDownloadListener == null) {
+            blockDownloadListener = (observable, oldValue, newValue) -> {
+                if ((double) newValue == 1) {
+                    setupWalletBestBlockListener();
+                    UserThread.execute(() -> walletsSetup.downloadPercentageProperty().removeListener(blockDownloadListener));
+                }
+            };
+            walletsSetup.downloadPercentageProperty().addListener(blockDownloadListener);
+        }
</pre>


<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/4851#discussion_r536137387">core/src/main/java/bisq/core/dao/node/lite/LiteNode.java</a>:</p>
<pre style='color:#555'>> @@ -97,20 +122,20 @@ public void start() {
                 checkForBlockReceivedTimer.stop();
             }
 
-            int height = block.getHeight();
-            log.info("New block at height {} from bsqWalletService", height);
+            int btcWalletHeight = btcBlock.getHeight();
+            log.error("New block at height {} from bsqWalletService", btcWalletHeight);
</pre>
<p>Why error log level?</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/4851#discussion_r536138619">core/src/main/java/bisq/core/dao/node/lite/LiteNode.java</a>:</p>
<pre style='color:#555'>>                  }
-            }, CHECK_FOR_BLOCK_RECEIVED_DELAY_SEC);
+            }, CHECK_FOR_BLOCK_RECEIVED_DELAY_SEC, TimeUnit.MILLISECONDS);
</pre>
<p>Timeout unit should be sec, is this a test artefact together with the error log level?</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/4851#discussion_r536140901">core/src/main/java/bisq/core/dao/node/lite/LiteNode.java</a>:</p>
<pre style='color:#555'>> @@ -229,11 +257,12 @@ private void runDelayedBatchProcessing(List<RawBlock> blocks, Runnable resultHan
     // We received a new block
     private void onNewBlockReceived(RawBlock block) {
         int blockHeight = block.getHeight();
-        log.debug("onNewBlockReceived: block at height {}, hash={}", blockHeight, block.getHash());
+        log.info("onNewBlockReceived: block at height {}, hash={}. Our DAO chainHeight={}", blockHeight, block.getHash(), chainTipHeight);
</pre>
<p>That's quite a bit of logging but perhaps ok since it's once per received block</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/4851#discussion_r536156944">core/src/main/java/bisq/core/dao/node/lite/network/RequestBlocksHandler.java</a>:</p>
<pre style='color:#555'>>                      } else {
-                        log.trace("We have stopped already. We ignore that networkNode.sendMessage.onFailure call. " +
-                                "Might be caused by a previous timeout.");
+                        log.warn("We have stopped already. We ignore that timeoutTimer.run call. " +
+                                "Might be caused by an previous networkNode.sendMessage.onFailure.");
</pre>

⬇️ Suggested change
<pre style="color: #555">-                                "Might be caused by an previous networkNode.sendMessage.onFailure.");
+                                "Might be caused by a previous networkNode.sendMessage.onFailure.");
</pre>


<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/4851#discussion_r536159701">core/src/main/java/bisq/core/dao/node/lite/network/RequestBlocksHandler.java</a>:</p>
<pre style='color:#555'>> @@ -168,31 +167,36 @@ public void onFailure(@NotNull Throwable throwable) {
     @Override
     public void onMessage(NetworkEnvelope networkEnvelope, Connection connection) {
         if (networkEnvelope instanceof GetBlocksResponse) {
</pre>

⬇️ Suggested change
<pre style="color: #555">-        if (networkEnvelope instanceof GetBlocksResponse) {
+        if (!(networkEnvelope instanceof GetBlocksResponse)) return; // Also need to remove other brace
</pre>


<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/4851#discussion_r536164882">core/src/main/java/bisq/core/dao/node/lite/network/RequestBlocksHandler.java</a>:</p>
<pre style='color:#555'>>              }
+
+            GetBlocksResponse getBlocksResponse = (GetBlocksResponse) networkEnvelope;
+            if (getBlocksResponse.getRequestNonce() != nonce) {
+                log.warn("Nonce not matching. That can happen rarely if we get a response after a canceled " +
+                                "handshake (timeout causes connection close but peer might have sent a msg before " +
+                                "connection was closed).\n\t" +
+                                "We drop that message. nonce={} / requestNonce={}",
+                        nonce, getBlocksResponse.getRequestNonce());
+                return;
+            }
+
+            cleanup();
</pre>
<p>Doesn't timeoutTimer need to be stopped here like before?</p>
<pre><code>        stopTimeoutTimer();
</code></pre>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/bisq-network/bisq/pull/4851#pullrequestreview-545003484">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AJFFTNS6FMFSE7DJEOC5X6TSTD3MRANCNFSM4UD52FOA">unsubscribe</a>.<img src="https://github.com/notifications/beacon/AJFFTNSFSPJQYP6FOXYNREDSTD3MRA5CNFSM4UD52FOKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOEB6BPXA.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/bisq-network/bisq/pull/4851#pullrequestreview-545003484",
"url": "https://github.com/bisq-network/bisq/pull/4851#pullrequestreview-545003484",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>