bisq-network / proposals

@bisq-network improvement proposals
https://bisq.wiki/Proposals
43 stars 16 forks source link

Investigate alternative implementation for bisq (as basis for V2 / off-chain trading) #125

Closed bodymindarts closed 4 years ago

bodymindarts commented 4 years ago

This is a Bisq Network proposal. Please familiarize yourself with the submission and review process.

Suggestion

I would like to put forward the possibility of implementing Bisq v2 from scratch in a different language.

Major Triggers

The following major issues triggered me to even consider this:

There are many other minor things that led me to consider this but they are dwarfed by the above mentioned major ones.

Feasibility

Since a rewrite in a different language is a major undertaking that shouldn't be taken on lightly (even with the presence of the major triggers) much more information is required to analyse the trade-offs. For this purpose I have done a feasibility study to shed some light on the following questions:

You can find the code for the proof-of-concept of re-implementing bisq in rust here: https://github.com/bodymindarts/risq

POC findings

As a result of the proof-of-concept I have come to the conclusion that there are indeed many potential benefits by re-implementing bisq in rust (as part of V2). Enough that it is worth putting forward this proposal for serious consideration.

Next steps

With more information regarding feasibility available due to the proof-of-concept I believe an informed public discussion is now possible. Obviously how to move forward will depend a lot on the outcome of this discussion so the next steps I am laying out are not to be taken as recommendation. Rather a starting point for consideration.

To move forward with the (hypothetical) project Bisq V2 in rust (pending community discussions) the following steps would likely need to occur:

mpolavieja commented 4 years ago

As I see it, any decision about Bisq v2 should depend on the feasibility of Multisig on top of Lightning Network. If this is possible, then I think it could be a better approach than the BSQ approach.

I know that at this moment there is not a solution for this in LN, but what if on-chain fees stay low for 2 more years, and then after 2 years there is a solution on top of LN?

sqrrm commented 4 years ago

I don't see the benefit of rewriting for the sake of rewriting. It might be that rust is a better language for some reason or other, and there is indeed a lot of support in the Bitcoin community. We are however tied to using java for at least the DAO as you mention. Throwing out all the other java code and start from scratch would also mean we still need an interface to the DAO, and all the current developers that are not used to rust would have to learn a new language, or they might leave.

What might make more sense is to define the v2 protocol strictly and allow for different clients using something like a bisqd with the api. Still, we first need to know what v2 entails as @mpolavieja mentions.

bodymindarts commented 4 years ago

I don't see the benefit of rewriting for the sake of rewriting.

Completely agree. I'm proposing a rewrite for the sake of:

And only based on the assumption that the overlap between v1 and v2 is not very large / easy to re-implement. ie. it will be a hard-fork with completely new code for handling trading - which means a rewrite of some portion of the code anyway.

What might make more sense is to define the v2 protocol strictly and allow for different clients using something like a bisqd with the api.

A well defined protocol contract for v2 would certainly be good.

leshik commented 4 years ago

I believe @bodymindarts is doing a great job rewriting the Bisq protocol in Rust. Don't get me wrong, Bisq is awesome and the current Java implementation was great to launch the project and to onboard new users.

But there are issues. It has highly coupled components, making it difficult to extend, refactor or test. I'm following threads here on API implementation for more than a year already and I see how difficult is to make it work with current architecture. There were already several PRs that were rejected due to the number of changes needed to be done. I can imagine this might demotivate developers to continue the work and makes it difficult to attract new ones. Also, the current implementation is unsuitable for constrained hardware, such as Raspberry Pi, as it consumes a lot of resources. Some dependencies are outdated and unmaintained, the aforementioned BitcoinJ is just the most critical example. Not to say we have to stick with JDK10 which is not even LTS and might have security issues.

Besides the technical part, things like "Everyone should upgrade to version X before date Y because trading protocol will stop working" don't sound like decentralized governance to me. The modular, independent implementation (or better several implementations) will allow people to use only those features that they really need. For example, to me altcoins trading and GUI make little sense, and I know that I'm not alone. :) Also, not locking the project to the sole ecosystem and porting it to different architectures would help the protocol maturity IMHO.

We should encourage more independent implementations!

chimp1984 commented 4 years ago

Besides the technical part, things like "Everyone should upgrade to version X before date Y because trading protocol will stop working" don't sound like decentralized governance to me.

Those were exceptional cases due securitity issues (stolen bank account scammer in april) and now due the trade protocol update and the trotection tool which would be otherwise a hard fork. Bisq had only 1 real hard fork ever (in 3.5 years). To support multiple trade protocols in a backward compatible and interoperatible way would cause 10x more effort...

We should encourage more independent implementations!

This is wisdom which is true for "normal" engineering but not for concensus based projects. Even Satoshi warned of having multiple Bitcoin implementations and Ethereum showed why he was right with his warning. In a perfect world, yes it would be good but we are in a fast moving under-resourced imperfect world.

Not to say we have to stick with JDK10 which is not even LTS and might have security issues.

Tell Oracle to fix their mess with their new release cycles. Forcing people to a new version which is not sufficiently supported by the surrounding infrastructure (java packager is still missing) is not sign of good company strategy.

bodymindarts commented 4 years ago

To support multiple trade protocols in a backward compatible and interoperatible way would cause 10x more effort...

Good argument for having a clean seperation between v1 and v2 protocols. (ie. different apps / code bases)

This is wisdom which is true for "normal" engineering but not for concensus based projects. Even Satoshi warned of having multiple Bitcoin implementations and Ethereum showed why he was right with his warning. In a perfect world, yes it would be good but we are in a fast moving under-resourced imperfect world.

This is why I'm not proposing to re-implement the current protocol, rather focus on the v2 protocol.

Tell Oracle to fix their mess with their new release cycles. Forcing people to a new version which is not sufficiently supported by the surrounding infrastructure (java packager is still missing) is not sign of good company strategy.

Imo another good argument for moving away from the java ecosystem.

niyid commented 4 years ago

@bodymindarts

You made some great points. I think whatever chosen direction should be the one that will provide the fastest path to producing mobile versions of the Bisq Exchange app itself (Android and iOS).

To further explain: I will favor an architecture that creates a clear separation between the domain layer and the view such that the code for the domain layer essentially remains the same for different devices but only the view varies.

I expect this has been taken into consideration with the V2 protocol - which necessarily should provide the communication interface between the domain and the view with the assumption that they are separate modules; and possibly running on separate devices. For instance I can have my domain running on a PC while my view (GUI) is on an Android device.

Regards.

leshik commented 4 years ago

Those were exceptional cases due securitity issues (stolen bank account scammer in april) and now due the trade protocol update and the trotection tool which would be otherwise a hard fork.

You mean, as exceptional as Ethereum DAO was?

Bisq had only 1 real hard fork ever (in 3.5 years). To support multiple trade protocols in a backward compatible and interoperatible way would cause 10x more effort...

You don't have to. Let multiple implementations exist, then let the market decide.

We should encourage more independent implementations!

This is wisdom which is true for "normal" engineering but not for concensus based projects. Even Satoshi warned of having multiple Bitcoin implementations and Ethereum showed why he was right with his warning. In a perfect world, yes it would be good but we are in a fast moving under-resourced imperfect world.

Bitcoin has several implementations besides Bitcoin Core, this includes libbitcoin, ABCore and btcd to name a few.

Not to say we have to stick with JDK10 which is not even LTS and might have security issues.

Tell Oracle to fix their mess with their new release cycles. Forcing people to a new version which is not sufficiently supported by the surrounding infrastructure (java packager is still missing) is not sign of good company strategy.

That's what I'm talking about – JDK is a weird choice for Bitcoin project, and this highly coupled architecture makes it impossible to reuse components. For example, I don't need jpackager, really, nor a bundled tor.

chimp1984 commented 4 years ago

You mean, as exceptional as Ethereum DAO was?

I don't see how Bisq compares to Ethereum or the Ethereum DAO.

Bitcoin has several implementations besides Bitcoin Core, this includes libbitcoin, ABCore and btcd to name a few.

And none is used for mining as those who have skin in the game dont play risky games.

leshik commented 4 years ago

I don't see how Bisq compares to Ethereum or the Ethereum DAO.

Bisq itself doesn't compare of course. Bailing out investors does.

Bitcoin has several implementations besides Bitcoin Core, this includes libbitcoin, ABCore and btcd to name a few.

And none is used for mining as those who have skin in the game dont play risky games.

You misunderstand what SITG is. And how mining is relevant here? They just have a different scope (validating nodes). Any venture incurs risks, especially in the space of Bitcoin. You can't make trading absolutely safe (that's what governments always do by "protecting" investors... and always fail – but Bisq is supposed to free us of government supervision, isn't it?) People will always lose money due to different reasons, you can't prevent this with technology. You can only mitigate some of the risks. But different people have different needs and different risk tolerance, why do you feel that you can decide for them?

leshik commented 4 years ago

I've built risq and started to play with the API. I was surprised by the amount of work @bodymindarts has already done. I think that at least the first goal is achieved – risq has proven that it can interoperate with the current implementation. And the pace at which @bodymindarts has gotten to this point makes me think the dev workflow, at least, deserves the attention.

chimp1984 commented 4 years ago

Bailing out investors does.

How is the security measurments which required an update related to "Bailing out investors"?

But different people have different needs and different risk tolerance, why do you feel that you can decide for them?

If stolen bank account scammers wouldnot have been stopped, Bisq would have been dead. Or how many people do you think would be willing to trade on Bisq when the chance that they get a trade with the scammer is very high?

leshik commented 4 years ago

Bailing out investors does.

How is the security measurments which required an update related to "Bailing out investors"?

It effectively kicked out of trading or imposed ridiculously low limits on people who did nothing wrong, just had a relatively new account.

But different people have different needs and different risk tolerance, why do you feel that you can decide for them?

If stolen bank account scammers wouldnot have been stopped, Bisq would have been dead. Or how many people do you think would be willing to trade on Bisq when the chance that they get a trade with the scammer is very high?

All those people who can make decisions for themselves and aren't needed in this helicopter parenting would trade successfully. If somebody is ok to take a risk and lose money trading with a new account and/or using a payment method that is subjected to chargebacks, that's their choice. Imposing restrictions by force you just promote infantilism, thus no skin in the game. Best thing software can do is to warn, but not to restrict the trading.

wiz commented 4 years ago

I spoke to @bodymindarts about his implementation and it looks really cool. I want to look into using his early implementation for the Alerts service-check and Markets API server use cases, we can probably use his minimal P2P implementation for those right away.

My only concern going forward is the very high number (hundreds?) of trusted third party dependencies which we need to cut down: https://github.com/bodymindarts/risq/blob/master/Cargo.lock

niyid commented 4 years ago

I am guessing this will also be initiated as an Incubation project? I hope to see it kick off as soon as possible.

bodymindarts commented 4 years ago

There are currently 20 direct dependencies some of which could conceivably be removed if desired.

I have added a small make command to output the total number of deps (including build and dev deps). Its not exact but gives a good estimation: To run it you need to install the cargo-tree plugin.

$ cargo install cargo-tree
$ make no-of-deps
  cargo tree | grep -v '(*)' | grep -v '\[' | wc -l
    223

I'm interested to know how this compares with bisq but cannot find a simple way of listing the deps.

leshik commented 4 years ago

@bodymindarts Maybe ./gradlew desktop:dependencies is the equivalent for bisq?

bodymindarts commented 4 years ago

@leshik yes but the output is no easy to convert into a simple number. The structure of the output has lots of duplication and I'm not sure which sections are the ones that we're interested in.

leshik commented 4 years ago

@bodymindarts yes, but if you remove all the garbage from the output, and include all the dependencies (even compileOnly and test), you get 87 deps in current master. This include OpenJFX, Kotlin runtime, and Tor for all platforms.

leshik commented 4 years ago

BTW there is an ongoing effort integrating rust in Bitcoin Core: https://github.com/bitcoin/bitcoin/issues/17090 They decided to go without cargo and use rustc directly.

bodymindarts commented 4 years ago

Just a note: I completely agree with the overall approach of cutting down on deps! Currently this is still an experiment and read-only so I have erred on the side of productivity. At the latest by the time something is released that interacts with private keys every dependency should be reflected on wether or not we can do without.

chimp1984 commented 4 years ago

When counting number of dependencies we should look at the risk level of each dependency. Dependencies from large organisations/companies which are long term and largely used represent much lower risk than those from rather unknown developers or small projects.

wiz commented 4 years ago

@chimp1984 I was thinking of something like this too, I want to make a table of entire dependency tree with the maintaining organization / group / people and assign a risk score for each one. This gives us a pragmatic way to remove the "high risk" ones.

chimp1984 commented 4 years ago

@ExPrgrmmr Please stop trolling. Github is for serious work discussions.

mpolavieja commented 4 years ago

Agree, this is the right way to go. A trade protocol with BTC payments between Bisq wallets on the Lightening Network.

But in order to propose anything on top of LN, feasibility has to be determined to some extent. At this moment and as far as I know there is no solution nor expected solution for LN multisig. If you are aware of a feasible multisig solution on top of LN, please ellaborate.

chimp1984 commented 4 years ago

There are theoretical ideas how to do it but as far I am aware nobody is working on that and it does not seem likely to become part of LN standard in the forseeable future.

mpolavieja commented 4 years ago

The only way I can think of using LN is having a very good reputation system without security deposits:

  1. The seller would only accept fiat from fully signed accounts (a minimum of reputation).
  2. The seller sends the BTC only after receiving the fiat. This could be done in small chunks to reduce risk for the buyer (this process of chunking the trade would be fast with revolut or instant SEPA).

In any case, this could always be improved with a small BSQ bond on the side of the seller to at least cover each chunk.

(this was a very quick and dirty comment, so I am sure it has plenty of errors)

chimp1984 commented 4 years ago

I have several serious concerns with how that proposal have escaped it's initial goals.

Building on top of Bisq v2 is premature

The original goal was a feasibility study for a potential new trade protocol (Bisq v2). But we don’t know yet if Bisq v2 is feasible from the conceptual point of view and the security model as well as we don’t know if the changes for the economic model of the DAO would carry too high risks. I think the right process would be to solve the conceptual challenges for Bisq v2 first and only if those are solved to start thinking about the implementation details (e.g. which language to use). Further the initial view that Bisq v2 requires a hard fork must not be taken as “fact” but as first "educated guess". It might be that we find ways to integrate it in Bisq v1 and/or deploy it without hard fork as we found a way to deploy the new trade protocol (1.2) without a hard fork.

Moving goal posts

As far I understand the original goal has changed in the meantime (not documented in that proposal) and the current goal is to do a full re-implementation of Bisq. As far I am aware @bodymindarts states that reimplementing the DAO is not a goal but he also told me initially that reimplementing Bisq and the trade protocol is not a goal and the goal posts have moved in just a few weeks.

Multiple implementations are not a good idea for consensus systems

We have seen from the Ethereum space that having multiple implementations can cause severe problems and also Satoshi was strongly against alternative implementations of Bitcoin and still up to today no miner uses alternative implementations to Bitcoin Core for mining as the risk is too high.

Bisq is a complex system of different levels of consensus and compatibility requirements and we have seen already in the recent issue with the requests for 9 MB of Tradestatistic data from risq nodes that this risk is real.

The spectrum of that consensus and compatibility requirements are difficult to see and understand. It can reach from relatively soft cases like the requirement that nodes are persisting data and using resource files for optimisation to not over-consume resources of other nodes (seed nodes in that case) to hard cases like the DAO consensus system where in the worst case a fork can destroy the value of all BSQ stakeholders.

How to integrate the DAO?

I know that @bodymindarts states that reimplementing the DAO is not a goal. But thinking the consequences of that statement further I see serious difficulties. If the DAO is not supported at all we exclude the traders from the DAO ecosystem and we amplify the role of the trusted and centralized BTC donation address holder who need to do the work for converting BTC to BSQ and burning them instead of the users who use BSQ for trade fees. So basically we are damaging our goal and achievment that users pay their trade fee in BSQ to have a decentralised form for distributing the revenue to contributors.

Shipping the DAO java code inside of risq would add a really heavy resource burden as you need also the JVM and you run the p2p network, BitcoinJ wallet and much more, basically the full Bisq app. So all the promised benefits to have a faster and more light weight risq app would get inverted. So I don't see a feasible solution for that problem.

Reimplementing the DAO is an absolute no-go from my point of view as the risk to break it is way too high and the whole project value is on stake here. So any BSQ stakeholder could lose all their BSQ holdings. I certainly will not take that risk.

Splitting the small developer community and dev resources

We don't have enough dev resources to improve Bisq as we would like to and to add new features. With losing some of those dev resources for an alternative implementation we would reduce that work force even more. We should not be tricked by seeing a quick prototype. I assume everyone is familiar with the 80-20 rule. Bisq had when it started 6 years ago a working trade protocol prototype with p2p network and bitcoin wallet in 3 months out, but it took another 3 years to get it production ready for launch.

Missing strategy

I don't see any clear strategy plan. Do you suggest 2 parallel implementations? Or is the goal to replace the java implementation? What are the expected benefits and what are the costs? Do you think a reimplementation with one core developer who is learning a new language and doing most of the work alone will lead to significant better code quality? I would estimate a high quality reimplementation to take 5 high skilled devs (who have already experience with rust) about 6-12 months. And even then I assume there are high risks to be compatible and do not cause lots of consensus issues. And excluding the reimplementation of the DAO. I am sure with an estimated 450 000 - 900 000 USD budget we can do more then simple provide an alternative Bisq version in another language.

Risks with dependencies

I estimate the risk of dependencies provided by large corporations or projects in Java is lower than the risk to use dependencies from rather new languages like Rust with a smaller ecosystem where some critical infrastructure libraries are provided by small teams or even single developers.

Being conservative and building on top of old and "boring" technologies is the better choice for security critical projects like Bisq.

Some comments to your "Major Triggers":

The current state of the code base has a lot of technical debt

To reimplement a large and complex project like Bisq without the risk to end up with a similar level of technical depts require a rather large team of highly experienced developers doing TTD or pair programming. So far I have not seen any of those requirements fulfilled in the risq project (learning a language by building a financial app does not seem to me the right approach). So far I see @bodymindarts is the main developer with the huge majority of commits. The stats at https://github.com/bodymindarts/risq/graphs/contributors shows a much worse dev distribution as we have in Bisq (https://github.com/bisq-network/bisq/graphs/contributors). Sure it's very early days and I don't want to overinterpret that but I also assume it will be very hard to find experienced Rust developers (not those many who are in the process of learning it).

The most important dependency of Bisq (BitcoinJ) is unmaintained and untrustworthy. There is no alternative in the JVM ecosystem.

As soon we have the resources we will consider to implement alternative to BitcoinJ, though there are not really a good one out unfortunately. Neutrino was at least a year ago not feasible (See our study at: https://github.com/bisq-network/bisq/issues/1062#issuecomment-446755367) and using Electrum is the most likely choice. I don’t see how using the JVM limits us here. I am not aware of a Rust SPV model which would fulfil Bisq's requirements (client side bloom filter still does not support unconfirmed txs as far I am aware).

Integrating an API into the current Bisq code base is a significant challenge.

This is a misleading statement. I just provided a GRPC prototype with placing an offer via API in roughly 1 day. The reason why we did not get much progress on the API side is a complex mix of lack of reviewers/maintainers and lack of capability of the project devs to do the required refactoring which led to the unpleasant deadlock situation we have and it is a pity that this causes so much mis-interpretations and bad reputation.

bodymindarts commented 4 years ago

I have several serious concerns with how that proposal have escaped it's initial goals.

Building on top of Bisq v2 is premature The original goal was a feasibility study for a potential new trade protocol (Bisq v2).

This is not accurate. The goal is to find out wether or not an implementation in another language (other than java) would be possible at all. Now - when the details of V2 are figured out - we know we have that option. Before it was conjecture.

Further the initial view that Bisq v2 requires a hard fork must not be taken as “fact” but as first "educated guess". It might be that we find ways to integrate it in Bisq v1 and/or deploy it without hard fork as we found a way to deploy the new trade protocol (1.2) without a hard fork.

I am aware that this is based on conjecture. I even write:

It is very likely that off-chain trading will not be backwards compatible.

This is not a proposal to do V2 in Rust. This is a proposal to find out wether or not V2 in any language other than java could even be considered (once conceptual details have been decided).

Moving goal posts As far I understand the original goal has changed in the meantime (not documented in that proposal) and the current goal is to do a full re-implementation of Bisq.

This is not accurate. This proposal has not changed.

Me and @wiz have however opportunistically used the resulting code base for other things than was originally intended that has already provided significant value, even though this was just a POC:

So there are no moving goal posts for this proposal. All concerns regarding re-implement V1 should be addressed in the android proposal thread as they are not relevant for this proposal.

How to integrate the DAO? I know that @bodymindarts states that reimplementing the DAO is not a goal. But thinking the consequences of that statement further I see serious difficulties. If the DAO is not supported at all we exclude the traders from the DAO ecosystem and we amplify the role of the trusted and centralized BTC donation address holder who need to do the work for converting BTC to BSQ and burning them instead of the users who use BSQ for trade fees. So basically we are damaging our goal and achievment that users pay their trade fee in BSQ to have a decentralised form for distributing the revenue to contributors.

This is an excellent point so I would like to clarify. While I consider having 2 implementations of the DAO consensus rules quite risky I don't see an issue with having multiple BSQ wallet implementations. Just like Bisq itself contains a wallet implementation for BTC but doesn't implement all consensus rules (ie it is not a BTC full node) it should be no problem to integrate a BSQ wallet (for trading fees) without re-implementing the DAO.

If you think about the use cases the way ppl use a trading app is completely different than how and when they need to use the DAO governance features. I see no reason that this functionality needs to be co-located into 1 app.

Splitting the small developer community and dev resources

This point is not relevant here. 1 developer is enough to do a feasibility study. Please move the comments relating to a potential re-implementation to the android thread

Missing strategy I don't see any clear strategy plan. Do you suggest 2 parallel implementations? Or is the goal to replace the java implementation? What are the expected benefits and what are the costs? Do you think a reimplementation with one core developer who is learning a new language and doing most of the work alone will lead to significant better code quality?

This proposal is intended for information gathering to be able to make a more informed decision when the time to implement V2 comes. Forming a concrete strategy for V2 at this point is hard as there are so many unknowns conceptually.

one core developer who is learning a new language

I don't think this point is relevant as the POC is about feasibility but I would like to mention that I am not currently learning a new language. Over 2 years ago I was a core maintainer of one of the largest open source rust code bases.

Risks with dependencies I estimate the risk of dependencies provided by large corporations or projects in Java is lower than the risk to use dependencies from rather new languages like Rust with a smaller ecosystem where some critical infrastructure libraries are provided by small teams or even single developers.

Being conservative and building on top of old and "boring" technologies is the better choice for security critical projects like Bisq.

This may be true in general. However I think for the most critical security related dependency that interfaces with the private keys this is not the case. https://github.com/rust-bitcoin is actively developed and maintained by a number of bitcoin-core devs with very high reputation in the Bitcoin space. Rust is even being considered to be included into bitcoin-core https://github.com/bitcoin/bitcoin/issues/17090. The same cannot be said about the JVM or BitcoinJ.

Indeed while Java may be a mature language, from the POV of the larger crypto-community it is quite an unusual choice. I think re-considering this choice, or investigating wether we even have the option to reconsider for V2 is reasonable

Some comments to your "Major Triggers": The current state of the code base has a lot of technical debt

I think most of these points are better addressed in the android thread. 1 dev is enough for a investigation / POC.

The most important dependency of Bisq (BitcoinJ) is unmaintained and untrustworthy. There is no alternative in the JVM ecosystem.

As soon we have the resources we will consider to implement alternative to BitcoinJ, though there are not really a good one out unfortunately. Neutrino was at least a year ago not feasible (See our study at: bisq-network/bisq#1062 (comment)) and using Electrum is the most likely choice. I don’t see how using the JVM limits us here. I am not aware of a Rust SPV model which would fulfil Bisq's requirements (client side bloom filter still does not support unconfirmed txs as far I am aware).

There are implementations of bip 157 / bip 158 in the rust-bitcoin org.

Its great that this research has been done. This proposal adds to that research and allows us to make a more informed decision about how to deal with this highly significant security concern.

chimp1984 commented 4 years ago

@mpolavieja Can you please close that issue and mark is as rejected (in DAO voting)?

mpolavieja commented 4 years ago

Rejected in DAO voting cycle 8