[bisq-network/projects] Introduce interface between Bisq and BitcoinJ (#30)

[dmos62]

## Description
<!-- Briefly summarize the proposed project. Strive for one or two sentences of plain language that any user, contributor or stakeholder will understand. -->

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.

## Rationale
<!-- Make the case for the the project. Why is it important? Why should it be done now? What will happen if we don't do it or delay doing it? -->

### In summary

###### Why is it important? 

- 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.

- 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.

###### Why now? What happens if we wait?

- 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.

- Delaying work on our Bitcoin network code is also delaying SegWit.

### In depth

#### Maintenance of Bitcoin network code
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.

#### Restoring maintainability of our BitcoinJ fork

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).

Among other things, returning to beIng able to maintain our BitcoinJ fork (and thus being able to upgrade it) will [allow us to support SegWit/Bech32 addresses](https://github.com/bisq-network/projects/issues/15), which is an often asked for feature.

#### Tight coupling and maintainability

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):

```
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
```

And, below is the even longer list of (non-test) source files that import BitcoinJ classes (with minor formatting added):

```
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
```

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.

#### Make sense of BitcoinJ usage by introducing an intermediary interface

The proposed interface would aim to hide the bulk of BitcoinJ specific logic and to centralize essential Bisq Bitcoin network logic.

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 `Wallet`, `PeerGroup`, `BlockStore`, `net.discovery`) 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.

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.

## Criteria for delivery
<!-- Make a checklist defining the end state of the project. How will we know that the project is complete, i.e. delivered? What will exist at the completion of this project that does not exist now? What will have changed? What communications, promotions and/or documentation will exist to ensure people know about these changes? -->

- [ ] new intermediary interface abstracts away the most important BitcoinJ-specific logic;
- [ ] there's a concensus that the new interface's clarity meets our standards.

## Measures of success
<!-- After this project has been delivered, how will we know whether it was a success? What can be measured to indicate that this project was worth the time, effort and compensation it cost? -->

- positive difference in Bitcoin network code readability and maintainability;
- reduced learning curve and complexity of resuming work on BitcoinJ fork.

## Risks
<!-- What risks do we run by undertaking the project? What sensitive areas of code are to be modified? What negative implications might this project have on privacy, security, performance, compatibility, maintainability, user experience, etc? -->

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).

## Tasks
<!-- Make a checklist defining in as much detail as is foreseeable who will need to do what in order to deliver the project. The checklist may be modified throughout the course of the project as new tasks emerge. Alternatively, once the project proposal is approved, you may choose to migrate the task checklist to a dedicated GitHub project board. -->

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.

## Estimates
<!-- Estimate the cost in USD of delivering this project. Indicate which teams are involved and provide subtotal estimates per team. This section need not be complete for the project proposal to be approved but must be complete for budget to be allocated. -->

Budget not yet discussed.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/projects/issues/30
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20200423/fe16422b/attachment-0001.html>