Comments (9)
Concrete plan
Note: below text is written in a tree-like fashion, which helps me structure my writing and navigate the text. Hopefully it doesn't look too outlandish.
- Below algorithm describes a refactoring of BitcoinJ usage in Bisq
- the goal of the refactoring is to
- generally speaking, improve maintenance of Bisq's Bitcoin network code
- more specifically, lay the foundation for ongoing reduction of the BitcoinJ-Bisq coupling
- ongoing, because the coupling is too extensive to be safely undone at once
- the described refactoring aims to achieve that by
- introducing a "btc.low" layer that
- acts as a Bitcoin client API that is implementation agnostic
- in other words, it would be the only place in Bisq with Bitcoin-client-specific (BitcoinJ-specific) logic
- acts as a Bitcoin client API that is implementation agnostic
- doing it in a way that allows safe incremental changes
- what will make the process safe is the smallness of the refactoring steps
- the small steps facilitate audits
- further, being able to refactor in small steps will make opportunistic refactoring more common
- what will make the process safe is the smallness of the refactoring steps
- introducing a "btc.low" layer that
- the end product of this initial refactoring
- a place where we can move BitcoinJ-specific logic
- a place where we can see how exactly we're using BitcoinJ (in a much more centralized way)
- a clear and simple abstraction and refactoring system for ongoing improvement of our BitcoinJ usage
- the goal of the refactoring is to
Refactoring steps
For readability, I've bolded what you'd call the main variable names if this were code:
- Make an identical copy of the BitcoinJ API inside Bisq
- more specifically, of the subset of bitcoinj api that we use
- initially this copy will just redirect all calls to BitcoinJ
- nothing more than a fully transparent wrap
- you call this copy in the same way that you call the original
- and you get the same results
- Designate the copy as foundation for Bisq-BitcoinJ separation layer
- Bisq-BitcoinJ separation layer
- or, in other words, Bitcoin client agnostic layer
- the ultimate goal of this abstraction is to be the only repository of BitcoinJ-specific logic in Bisq
- Bisq-BitcoinJ separation layer
- Make this layer the (only) Bitcoin client used by Bisq
- as simple as
- search and replace
import org.bitcoinj.PeerGroup
forimport bisq.core.btc.low.PeerGroup
- search and replace
- will now refer to this layer as btc.low, as in the package bisq.core.btc.low.
- as simple as
- Incrementally mutate the btc.low abstraction
- by
- moving BitcoinJ-specific logic into it (from the rest of Bisq)
- generally refactoring it to better match the needs of Bisq
- this step is what makes the refactoring ongoing
- by
Illustrative code sketch
Below sketch illustrates what btc.core.btc.low.PeerGroup
could look like.
package bisq.core.btc.low;
import org.bitcoinj.core.BlockChain;
import org.bitcoinj.core.NetworkParameters;
import org.bitcoinj.net.BlockingClientManager;
import bisq.core.btc.nodes.ProxySocketFactory;
import bisq.core.btc.nodes.LocalBitcoinNode;
import java.net.Proxy;
import java.net.InetSocketAddress;
import com.runjva.sourceforge.jsocks.protocol.Socks5Proxy;
/* Below code is a sketch of what btc.low can look like.
* To make it short and simple, it presumes that our BitcoinJ usage is only
* what is in the bisq.core.btc.WalletConfig::createPeerGroup method.
* This sketch tries to demonstrate how logic is moved into btc.low: you start
* out with a lot of BitcoinJ-specific logic, and end up with a
* BitcoinJ-agnostic endpoint and a clear view of what BitcoinJ endpoints are
* used under the sheets.
* The code would definitely be simplified more after moving, but that's
* outside the scope of this sketch.
*/
public class PeerGroup {
private final org.bitcoinj.core.PeerGroup bitcoinjPeerGroup;
private PeerGroup(org.bitcoinj.core.PeerGroup bitcoinjPeerGroup) {
this.bitcoinjPeerGroup = bitcoinjPeerGroup;
}
/* This method is a copy-paste of
* bisq.core.btc.WalletConfig::createPeerGroup (except for the signature
* and using locally defined constructors). It serves only demo purposes
* and code quality inside it not representative of standards set for this
* project.
*/
private static PeerGroup createPeerGroup(
Socks5Proxy socks5Proxy,
NetworkParameters params,
BlockChain vChain,
LocalBitcoinNode localBitcoinNode
) {
PeerGroup peerGroup;
// no proxy case.
if (socks5Proxy == null) {
peerGroup = this(params, vChain);
} else {
// proxy case (tor).
Proxy proxy = new Proxy(Proxy.Type.SOCKS,
new InetSocketAddress(
socks5Proxy.getInetAddress().getHostName(),
socks5Proxy.getPort()));
ProxySocketFactory proxySocketFactory =
new ProxySocketFactory(proxy);
// We don't use tor mode if we have a local node running
BlockingClientManager blockingClientManager =
config.ignoreLocalBtcNode ?
new BlockingClientManager() :
new BlockingClientManager(proxySocketFactory);
peerGroup = this(params, vChain, blockingClientManager);
blockingClientManager.setConnectTimeoutMillis(TOR_SOCKET_TIMEOUT);
peerGroup.setConnectTimeoutMillis(TOR_VERSION_EXCHANGE_TIMEOUT);
}
if (!localBitcoinNode.shouldBeUsed())
peerGroup.setUseLocalhostPeerWhenPossible(false);
return peerGroup;
}
/* Proxy all PeerGroup methods that Bisq uses.
* These would probably be "protected" and be in a separate class, which
* would be subclassed by this one, for the sake of separating the used
* BitcoinJ endpoints and their usage.
*/
private PeerGroup(
NetworkParameters params,
BlockChain vChain
) {
org.bitcoinj.core.PeerGroup bitcoinjPeerGroup =
new org.bitcoinj.core.PeerGroup(params, vChain);
this(bitcoinjPeerGroup);
}
private PeerGroup(
NetworkParameters params,
BlockChain vChain,
BlockingClientManager
blockingClientManager
) {
org.bitcoinj.core.PeerGroup bitcoinjPeerGroup =
new org.bitcoinj.core.PeerGroup(
params, vChain, blockingClientManager);
this(bitcoinjPeerGroup);
}
private void setUseLocalhostPeerWhenPossible(boolean bool) {
this.bitcoinjPeerGroup.setUseLocalhostPeerWhenPossible(bool);
}
private void setConnectTimeoutMillis(int timeout) {
this.bitcoinjPeerGroup.setConnectTimeoutMillis(timeout);
}
}
from projects.
@dmos62 , Just want to express some enthusiasm for this project... Really, really big thumbs up from me.
The newly forming gRPC/Core API is going to help in terms of quick dev/test iterations -- faster than re-starting the UI after small changes. It's a little too early today, and the priority for the API is to ship the simplest reference impl asap, but even that will help speed up functional testing.
from projects.
@cbeams I agree a PR example is a good idea.
The 1:1 wrap part is just to get the foundation for where to move BitcoinJ-specific logic. I.e. the refactoring happens on top of that. You'd move the logic in one set of commits (like in the sketch) and then you'd most likely simplify it in another set of commits (not in the sketch).
At first I was thinking of calling it btc.client
, since it would handle all Bitcoin networking, but then realised that actually the bisq.core.btc
package is our Bitcoin client, and the proposed package would be the lowest-level of that client, hence the bisq.core.btc.low
. It's low-level in that it hides BitcoinJ logic that the rest of the btc
package shouldn't care about and exposes the basic interface that the rest of the btc
package will use. It makes sense to me at this point, but that might change and I don't have strong feelings either way.
from projects.
I like the idea of starting by just doing a transparent wrap and continue incrementally from there. One of the biggest issues will likely be keeping refactoring PRs small enough that they are easy enough to review and merge. I think the suggested route sounds good, but a sample PR will make it even clearer.
from projects.
Thanks for the update, @dmos62. I'm closing this as was:deferred
as that seems most fitting (definition). Let me know if that doesn't make sense to you.
from projects.
@dmos62, what do you think about putting together an initial PR that demonstrates a single such refactoring that you think is demonstrative of the value that can be provided? I'm generally for this effort, but I'd like to have a look at a working example before signing off on a larger scale effort. My main concern here is that I'm not sure the "fully transparent wrap" approach you detail above will actually add much value. If we do nothing more than a 1:1 wrap of a subset of the BitcoinJ API, what do we really gain? I'd rather see this play out in code than discuss it further here, so if you'd like to take a stab at it, please go ahead.
Also, what is the significance of "low" in your proposed bisq.core.btc.low
package? Do you mean "low-level bitcoin operations", perhaps? In any case, I don't think this is an intention-revealing name. Perhaps better naming will avail itself as we see and review an actual working example.
from projects.
- I think this proposal is independent of bisq-network/proposals#226. They don't compete with each other because they attack different problems.
- If both proposals are accepted I suggest not to code both projects in parallel because that would lead to merging hell. They could be implemented one after the other. If both proposals are accepted I suggest merging bisq-network/bisq#4270 as a starting point, which is already coded and only requires peer review.
- I did a research of bitcoinj alternatives back in Dec 2018 and found no good alternative (bisq-network/bisq#1062 (comment)). As of today I think there are 2 alternatives to bitcoinj that might make sense for bisq: a) bitcoin core and use its RPC interface or b) Electrum client for java (does not exist yet, but could be implemented). Every alternative has its pros and cons.
- Alternatives to bitcoinj might work very differently and would be difficult to abstract things without knowing the interfaces of the other libraries.
- This is a personal taste comment: bitcoinj usage in bisq could be subject to refactors and other code improvements, but I prefer to create an abstraction of a library once I am already implementing the usage of an alternative library. You guys know the bisq source code better than me, so this comment might not make sense for bisq.
from projects.
@oscarguindzberg I think there's potential for collaboration and see these projects working in complement to each other. More specifically, I can see this project facilitating the more interesting Bisq-side changes required for adopting SegWit and BitcoinJ 0.15.x in general.
Paving the way for an alternative library is more of a nice to have at this point. My priority with this project at this time is making our Bitcoin net code easier to maintain and audit. It just so happens that this is what's needed for switching out BitcoinJ as well.
from projects.
Status update:
While I believe that this project is very valuable in the medium to long term, and I very much enjoy working on it, I'm putting it on hold, because I feel there are short-term Bisq responsibilities that I should move my attention to. Follow a few notes I'd like to share.
This project is relatively slow to implement. To illustrate, I've spent 2.5 weeks refactoring logic surrounding PeerGroup, which is the simplest and least critical part of our Bitcoin code (that's also why I chose to start with it), and that's about 50% finished (intuition).
Whereas I started out thinking that this project would be low-hanging-fruit-only type of task, I soon realised that the really tangible increase in maintainability is from diving deep into Bisq code. Parts other than PeerGroup-related logic might be different, but probably not.
I've discovered a pretty complicated algorithm (which decides how we source peers) and I've not yet decided how to approach its audit and eventual refactor. I've created a TLA+ state machine of it (took about a week), but this algo has so many initial states that it's hard to define the properties that the algorithm should satisfy. For example, if a local bitcoin node is provided and custom nodes are provided too and we're in regtest, what sourcing method should be used? What about DAO regtest? I'm not really sure. And those are not all the input variables. Defining what to do for Mainnet is not that hard, but the rest is complicated, because it requires knowing a lot about Bisq. So that's the last thing I was working on.
Project's commit structure is such that it can be merged in installations and read one by one. I took care to have most if not all commits do only one thing, because for a reviewer 2 commits doing 2 simple things can be a lot simpler to review than those 2 commits squashed into 1.
I plan to now move to doing some PR reviewing and urgent bugfixing. I've considered continuing work on this refactor part-time, but I don't see myself being able to multitask this.
I'll PR the refactor to make my progress available.
from projects.
Related Issues (20)
- Improve support and mediation HOT 6
- Implement new-user-onboarding and new user interface design HOT 3
- Add Monero to fiat trading pairs using BTC as the security deposit (multi-sig) HOT 9
- Message board for multi-protocol project (working title Misq) HOT 53
- Specify interface and architecture for wallet and blockchain data modules HOT 9
- Research a solution for dynamically loading remote modules HOT 2
- Define architecture and interfaces for the protocol layer HOT 2
- Research on solutions for DIDs (decentralized IDs) in Bisq HOT 2
- Add Buy-Monero Keybase channel on Bisq for fiat trading pairs using BSQ bonds as the security deposit HOT 4
- Prototype for offer book and create offer UX for Bisq 2.0 (Misq) HOT 23
- Integrate wallet and blockchain data modules in Misq HOT 1
- bgmi bejjsjsj HOT 1
- Dev Call - Priorities HOT 20
- Investigate XMR-BTC atomic cross chain swap protocol options
- Integrate Bitcoind as a wallet backend into Misq
- Bisq2: Create Wallet Prototype UI To Test Wallet Functionalities HOT 5
- Bisq2: Liquid Wallet Integration (Elements) HOT 1
- Payment Methods - Plans for 2022
- Lightning node implementation for LN trades HOT 2
- Improve dispute resolution HOT 13
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from projects.