<p><b>@cbeams</b> requested changes on this pull request.</p>

<p>NACK on formatting and style issues. There are more than just a couple minor nits here, stuff that's worth getting right with a view toward future PRs being correct from the start.</p>
<p>This is by no means a complete review. I just took a quick look to understand what's changed with the new <code>LocalBitcoinNode</code> class and saw these superficial issues and commented on them. Unfortunately I don't have time to do more than that now.</p><hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/3982#discussion_r381308857">core/src/main/java/bisq/core/btc/nodes/LocalBitcoinNode.java</a>:</p>
<pre style='color:#555'>> +    private static NioClient createClient(
+            Peer peer, int port, int connectionTimeout
+    ) throws IOException {
</pre>
<p>To follow convention, formatting here should be:</p>
<div class="highlight highlight-source-java"><pre><span class="pl-k">private</span> <span class="pl-k">static</span> <span class="pl-smi">NioClient</span> createClient(<span class="pl-smi">Peer</span> peer, <span class="pl-k">int</span> port, <span class="pl-k">int</span> connectionTimeout) throws <span class="pl-smi">IOException</span> {</pre></div>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/3982#discussion_r381310516">core/src/main/java/bisq/core/btc/nodes/LocalBitcoinNode.java</a>:</p>
<pre style='color:#555'>>       */
-    public void detectAndRun(Runnable callback) {
-        try (Socket socket = new Socket()) {
-            socket.connect(new InetSocketAddress("127.0.0.1", port), CONNECTION_TIMEOUT);
+
+    private static Peer createLocalPeer(
+            int port
+    ) throws UnknownHostException {
+        // TODO: what's the effect of NetworkParameters on handshake?
+        // i.e. is it fine to just always use MainNetParams?
+        var networkParameters = new MainNetParams();
+
</pre>
<p>Yes, please apply same formatting advice here as I mentioned above on lines 65–67. And apply it consistently to any other similar changes in this PR as well. The general rule in effect here is to keep things on one line up to the 120-char right margin.</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/3982#discussion_r381311510">core/src/main/java/bisq/core/btc/nodes/LocalBitcoinNode.java</a>:</p>
<pre style='color:#555'>> +    private static boolean checkWellConfigured(VersionMessage versionMessage) {
+        var notPruning = versionMessage.hasBlockChain();
+        var supportsAndAllowsBloomFilters =
+                isBloomFilteringSupportedAndEnabled(versionMessage);
+        return notPruning && supportsAndAllowsBloomFilters;
+    }
+
+    /**
+     * Method backported from upstream bitcoinj: at the time of writing, our version is
+     * not BIP111-aware.
+     * Source routines and data can be found in Bitcoinj under:
+     * core/src/main/java/org/bitcoinj/core/VersionMessage.java
+     * and
+     * core/src/main/java/org/bitcoinj/core/NetworkParameters.java
+     */
+
</pre>
<p>Nit: unnecessary newline here, and this need not be a (<code>/**</code>) Javadoc comment as it's on a private method and will never be published. Wrap comments at 90-char right margin, thanks.</p>

<hr>

<p>In <a href="https://github.com/bisq-network/bisq/pull/3982#discussion_r381311985">core/src/main/java/bisq/core/btc/nodes/LocalBitcoinNode.java</a>:</p>
<pre style='color:#555'>> +
+    /**
+     * Method backported from upstream bitcoinj: at the time of writing, our version is
+     * not BIP111-aware.
+     * Source routines and data can be found in Bitcoinj under:
+     * core/src/main/java/org/bitcoinj/core/VersionMessage.java
+     * and
+     * core/src/main/java/org/bitcoinj/core/NetworkParameters.java
+     */
+
+    private static boolean isBloomFilteringSupportedAndEnabled(VersionMessage versionMessage) {
+        // A service bit that denotes whether the peer supports BIP37 bloom filters or not.
+        // The service bit is defined in BIP111.
+        int NODE_BLOOM = 1 << 2;
+
+        int BLOOM_FILTERS_BIP37_PROTOCOL_VERSION = 70000;
</pre>
<p>Local variables should be camelCase.</p>

<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/3982?email_source=notifications&email_token=AJFFTNQDXLZOUJQMFRNCBALRDU4ZVA5CNFSM4KWZVT6KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCWDHKWI#pullrequestreview-361133401">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AJFFTNR3FTGKA2A3BGSXEETRDU4ZVANCNFSM4KWZVT6A">unsubscribe</a>.<img src="https://github.com/notifications/beacon/AJFFTNQDMIO22RCA5CN6773RDU4ZVA5CNFSM4KWZVT6KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCWDHKWI.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/3982?email_source=notifications\u0026email_token=AJFFTNQDXLZOUJQMFRNCBALRDU4ZVA5CNFSM4KWZVT6KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCWDHKWI#pullrequestreview-361133401",
"url": "https://github.com/bisq-network/bisq/pull/3982?email_source=notifications\u0026email_token=AJFFTNQDXLZOUJQMFRNCBALRDU4ZVA5CNFSM4KWZVT6KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCWDHKWI#pullrequestreview-361133401",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>