<p></p>
<h2>Description</h2>

<p>This is a proposal for introducing an explicit Java interface between Bisq and BitcoinJ, with the purpose of improving maintainability of our Bitcoin network code.</p>
<h2>Rationale</h2>

<h3>In summary</h3>
<h6>Why is it important?</h6>
<ul>
<li>
<p>Our Bitcoin network code is very difficult to maintain, which threatens reliability and blocks some features (SegWit). This would improve maintainability and be a stepping stone towards SegWit.</p>
</li>
<li>
<p>While our BitcoinJ fork is old, upstream BitcoinJ itself is widely regarded as outdated. It is possible that in the future we will want to make drastic changes, like switching Bitcoin clients. Reducing coupling between Bisq and BitcoinJ would improve our flexibility when making those kinds of changes.</p>
</li>
</ul>
<h6>Why now? What happens if we wait?</h6>
<ul>
<li>
<p>Our BitcoinJ code is not being audited (much less improved), because it is difficult to understand. The sooner we resume taking care of this code the better. At this time, reacting to a BitcoinJ vulnerability (salt over shoulder) would be very awkward.</p>
</li>
<li>
<p>Delaying work on our Bitcoin network code is also delaying SegWit.</p>
</li>
</ul>
<h3>In depth</h3>
<h4>Maintenance of Bitcoin network code</h4>
<p>Bisq is currently tightly coupled with BitcoinJ, and our usage of it, being scattered throughout the rest of the code, is difficult to audit and to maintain. Abstracting our usage of BitcoinJ into an explicit interface would facilitate audits, maintenance of Bisq, maintenance of our BitcoinJ fork, and general comprehension of Bisq's internals.</p>
<h4>Restoring maintainability of our BitcoinJ fork</h4>
<p>BitcoinJ is a Bitcoin network client that is used by and tightly coupled with Bisq. Maintaining the BitcoinJ part of our codebase has proven challenging. Notable illustrations being that we're using a two year-old release of BitcoinJ (0.14.7, Mar 30, 2018) and that we have a dedicated role for its maintenance (which has not been filled since May, 2019).</p>
<p>Among other things, returning to beIng able to maintain our BitcoinJ fork (and thus being able to upgrade it) will <a href="https://github.com/bisq-network/projects/issues/15" data-hovercard-type="issue" data-hovercard-url="/bisq-network/projects/issues/15/hovercard">allow us to support SegWit/Bech32 addresses</a>, which is an often asked for feature.</p>
<h4>Tight coupling and maintainability</h4>
<p>Our BitcoinJ fork's maintenance involves understanding how we use BitcoinJ, which is made difficult by tight coupling. To illustrate what is meant by tight coupling here is the long list of unique BitcoinJ imports we use in Bisq (excluding tests, minor formatting added):</p>
<pre><code>grep -r --include=\*\.java -E "import( static)* org.bitcoinj" ./*/src/main -h | sort -u

Output moved to pastebin.com for the sake of readability:
https://pastebin.com/raw/Jd0nA6Nk
</code></pre>
<p>And, below is the even longer list of (non-test) source files that import BitcoinJ classes (with minor formatting added):</p>
<pre><code>grep -r --include=\*\.java -E "import( static)* org.bitcoinj" ./*/src/main -l | sort -u

Output moved to pastebin.com for the sake of readability:
https://pastebin.com/raw/Ap0wf3Pd
</code></pre>
<p>To maintain our BitcoinJ fork you'd have to make at least some kind of sense of the above usages of it, which is (I think evidently) intimidating and difficult. The primary way that the proposed interface would help is by structuring our Bisq usage.</p>
<h4>Make sense of BitcoinJ usage by introducing an intermediary interface</h4>
<p>The proposed interface would aim to hide the bulk of BitcoinJ specific logic and to centralize essential Bisq Bitcoin network logic.</p>
<p>BitcoinJ imports can be put into two categories: essential and auxiliary. I define auxiliary imports as those that serve to handle output from and pass input to the essential imports. The essential imports (such as <code>Wallet</code>, <code>PeerGroup</code>, <code>BlockStore</code>, <code>net.discovery</code>) are the exclusive focus of this refactor effort, in part due to the sheer number of aux imports, and, at the same time, because hiding essential imports behind an interface should be enough to see an appreciable improvement in maintainability.</p>
<p>In other words, the proposed interface would not hide all of the above listed imports, but only the most important ones. This also means that this wouldn't be a complete decoupling of BitcoinJ, though it would pave the way for one, if it were to ever happen.</p>
<h2>Criteria for delivery</h2>

<ul class="contains-task-list">
<li class="task-list-item"><input type="checkbox" id="" disabled="" class="task-list-item-checkbox"> new intermediary interface abstracts away the most important BitcoinJ-specific logic;</li>
<li class="task-list-item"><input type="checkbox" id="" disabled="" class="task-list-item-checkbox"> there's a concensus that the new interface's clarity meets our standards.</li>
</ul>
<h2>Measures of success</h2>

<ul>
<li>positive difference in Bitcoin network code readability and maintainability;</li>
<li>reduced learning curve and complexity of resuming work on BitcoinJ fork.</li>
</ul>
<h2>Risks</h2>

<p>Our Bitcoin network code is one of the most sensitive areas of Bisq; however, the risk of introducing bad changes should be minimal since this is a refactoring effort (meaning rearranging code without changing algorithms).</p>
<h2>Tasks</h2>

<p>This refactor effort is exploratory by nature, so making a precise plan is not possible ahead of time. Currently the plan is to analyse BitcoinJ usage and abstract it away behind the new interface, in order of descending importance, until only the least critical BitcoinJ code is left.</p>
<h2>Estimates</h2>

<p>Budget not yet discussed.</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/projects/issues/30">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AJFFTNVVACFTJEC6GKSAZ5LROC5X3ANCNFSM4MPSVBDA">unsubscribe</a>.<img src="https://github.com/notifications/beacon/AJFFTNRCOBP2WDBGIDT26GTROC5X3A5CNFSM4MPSVBDKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4JA5P4AQ.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/projects/issues/30",
"url": "https://github.com/bisq-network/projects/issues/30",
"name": "View Issue"
},
"description": "View this Issue on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>