Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Wallet Development Direction? Improve Bitcoin.org Inclusion Scoring? #43

Closed
nopara73 opened this issue Jan 31, 2019 · 17 comments
Closed

Comments

@nopara73
Copy link
Contributor

While we seem to be eligible for Bitcoin.org inclusion, I think it makes sense to bring closer improvements we were planning to do anyway, but we were not considering them as priority. It may be the right timing, because we have just completed all our "priority" issues we had and we are looking for a new direction to take our development. And you know me guys, I love to work on the basics, so working on it can be highly beneficial in the long term.
In this issue I gather all the possible improvements that can potentially improve our scoring in the website. I also include improvements and tasks those, while do not help to improve scoring, but the review of @crwatkins and @harding identified as possible improvements to our wallet.

Before that, first question should be for us to decide if this goal is worth pursuing. Please see the "Alternatives" section at the end and share your opinions @lontivero @danwalmsley @molnard @hgergoil @bharmat

Bitcoin Core Integration

a) Level.

Current: 3.

Level: Each wallet must have a level property assigned. A value must be in a range between 1 and 4. Level represents a category of a wallet:

  • Level 1 - Full nodes
  • Level 2 - SPV, Random servers
  • Level 3 - Hybrid, Multisig wallets

We could easily make Wasabi SPV, we could just synchronize the header chain with NBitcoin, that's how HiddenWallet worked. However I believe that would be less secure, than our current way of doing things, since then Wasabi users could be Sybil attacked or miner attacked. Now, if a node sends us a block that does not match the hash we get from the backend, then we don't accept the block. This applies to our new partial Bitcoin Core integration, thus the local node is untrusted. I've seen @NicolasDorier and @petertodd share a similar concerns with SPV nodes. Thus I think working on level 2 would not make sense.

Thus if we want to improve the level we need a full node integration. One way to go about it could be to use Stratis's C# Bitcoin full node code: https://github.com/stratisproject/StratisBitcoinFullNode or go with bitcoind. While the first option is appealing, many users use and trust more bitcoind already, and it's probably more stable so it may be smarter to go down on that path. The ceveat is the difficulty of handling Core's constant changes. Shit can happen if the user upgrades its own node, but not its built-in Wasabi Core node, or the other way around. Also note that we need built-in Wasabi node. Maybe we could do the same way as we do with Tor, if the user has Tor we use that, if he doesn't we use Wasabi's. Anyway, it makes this complicated, but it's not unsolvable. It's just a detail.

I imagine this integration as an opt-out integration. Until Bitcoin Core is not fully synced, Wasabi would be used in light wallet mode, like how @jonasschnelli built his full-spv Bitcoin Core PR. However the a dialog should be shown to the user at first start that "hey, Wasabi is going to synchronize Bitcoin Core, and will work in light wallet mode until it's fully synced" and then buttons: "ok" and "no i want light wallet mode only."

b) Validation

Currently: checkfailvalidationcentralized. I believe the above improvement would make it checkneutralvalidationvariable. Or is it checkgoodvalidationfullnode? Not sure. @crwatkins can you help me out with this?

Deterministic Build

Current: checkpasstransparencyopensource. We could improve it with adding deterministic builds: checkgoodtransparencydeterministic. I still have my doubts as to how useful it is, but at the very least we can research into this topic and even if our opinion does end up it considering it unnecessary we can still add it if it does not come with considerable administration overhead or caveats: WalletWasabi/WalletWasabi#1078

Two-Factor-Auth

Current: checkfailenvironmentdesktop. This is another thing I'm not sure about if we can improve upon it. There are two levels we can target: checkgoodenvironmenthardware and checkpassenvironmenttwofactor.
@crwatkins can you help me out with this? Based on Electrum's scoring I see that opt-in two factor makes it eligible to checkpassenvironmenttwofactor, but opt-in hardware wallet integration does not make it eligible to checkgoodenvironmenthardware. Is that correct? If so, the best course of action is to add 2FA to Wasabi.
One thing to note though. We must be very careful with 2FA, because as Electrum's developer @ecdsa noted in Building on Bitcoin "users are more often losing their passwords than getting hacked." This corresponds with our experience, we didn't even yet have any user of ours who got hacked but we already had a handful of them who lost access to the wallet. (Some succeeded to gain access though.) I don't want us to end up doing more harm than good. Maybe we should consider building in our password typo fixer/bruteforcer to improve the situation before we ruin it with 2FA: WalletWasabi/WalletWasabi#1035

Network Level Privacy

I don't want to go into the details here, rather I just refer to the conversation: bitcoin-dot-org/Bitcoin.org#2788 But what we can improve here are the following:

a) Tor Integration to NBitcoin

To communicate with peers over Tor. One way to do this would be to implement Tor communication to NBitcoin, but maybe we should take NBitcoin's code, implement Tor, wait half a year or so until it stabilizes, and then contribute it back to NBitcoin. @lontivero @NicolasDorier opinions?

b) Bitcoin Core Integration with Tor By Default

If we add Bitcoin Core, then we should not at any circumstances broadcast transactions on the clearnet through it, luckily Tor comes with Wasabi, so connecting the dots here should not be difficult.

RBF and CPFP

Currently: checkpassfeecontroldynamic, next level to target: checkgoodfeecontrolfull.
This requires RBF and CPFP. They are problematic from a privacy/wallet fingerprinting point of view, (mainly the former) so if we decide to implement this we must be very careful and a comprehensive elaborate strategy is needed for them.

Audit

Wasabi did not yet receive an audit. @kristovatlas has expressed interest in doing one some time in the future. When we'll have the resources in the future we can hire him to do so.

Alternatives

While this can be a direction we could take Wasabi's development, there are others we can consider. Feel free to suggest new ones.

  • Hardware Wallet Integration
  • UI And UX Improvement Sprint
  • Mixing Improvements: Reverse CoinJoins and P2EP with Reverse CoinJoins
  • Accessibility Improvement Sprint: OSX signing, EV HTTPS, rpm, pacman, etc packages and package managers, sweep private key, sweep receive to wrapped segwit, implement bech32 to other wallets/libraries like BitcoinJ, etc...
  • Just keep fixing issues in an unorganized way.
  • Keep it safe and fix issues tagged with "solution is non-controversial"
  • Tacking all the debug issues.
  • Concentrate on to bring more users on board, which brings us more resources, so we can pay more developers so we can more effectively address issues. For this note that about 7+-2 developers should be the maximum number of devs working on Wasabi, it looks like too much devs make things get out of hand, since over 7 people the administration and collaboration makes the work less and less effective. Refer to: The Mythical Man-Month and The Magical Number Seven, Plus or Minus Two
@NicolasDorier
Copy link

NicolasDorier commented Jan 31, 2019

Wasabi wallet would be the perfect wallet if it could just connect to a chosen bitcoind to update its state.
No need of shipping bitcoind with Wasabi wallet, bitcoind is not really good for desktop usage, while wasabi is. Just allowing connection to a known bitcoin node would be enough.

It means that it would stop fetching the filters from the coordinator to update the wallet, and that if the coordinator disagree with the configured node about the chain, you should just disable coinjoin feature without preventing users to use the wallet features.

I don't understand what you intend to do by using TOR to connect to your bitcoin node. I think that the way you are doing now by fetching the filters through TOR from the coordinator is the best way to do if the user does not have his own node. Connecting to random node is not an improvement. If the user have his own node, then you just fetch the block from it without TOR.

@nopara73
Copy link
Contributor Author

nopara73 commented Jan 31, 2019

Wasabi wallet would be the perfect wallet if it could just connect to a chosen bitcoind to update its state. No need of shipping bitcoind

Tor connection of bitcoind must be enforced by default, because of transaction broadcasting over clearnet, which we can only do if we ship. It can be opt-out by using your own bitcoind of course. Or should we just testacceptmempool and then broadcast transaction with NBitcoin over Tor? Anyway, in Wasabi we are very careful to make everything work out of the box and our users are very grateful for this, so shipping with bitcoind is important from this point of view, too.

Just allowing connection to a known bitcoin node would be enough.

There was an issue @luke-jr brought up recently with trusted nodes and Samourai, I don't remember exactly what was this or if it applies to local connections, too (I think yes, because any malware can hijack the bitcoind ports if bitcoind is not running.) Anyway, we must pay attention to this also.

It means that it would stop fetching the filters from the coordinator to update the wallet, and that if the coordinator disagree with the configured node about the chain, you should just disable coinjoin feature without preventing users to use the wallet features.

Filters have nothing to do with coinjoins, so luckily we don't have to bother with this.

I don't understand what you intend to do by using TOR to connect to your bitcoin node.

End-to-end encryption of block downloading, so ISP cannot spy. I also argue this is minor, but in the Bitcoin.org PR this was the main concern of the reviewers, which would bring down Wasabi's privacy rating to be exactly equivalent to Bread wallet's.

PS. @NicolasDorier Thanks for the book recommendation of The Mythical Man-Month (seen you tweet about it.) This has confirmed many of my pre-existing biases haha.

@lontivero
Copy link

lontivero commented Jan 31, 2019

We have two extremely scarce resources here: bitcoins (budget) and developers time. It is necessary then to focused on the most important things and we must decide and communicate clearly to the team and community what is the vision and what are the principles that drive the development direction.

In my very personal opinion Wasabi development should be focused on a clear statement as the one we said from the beginning: bringing privacy back to the users - what kind of users should be defined too, btw. For this reason I wouldn't allow other people criteria to have significant influence our decisions. I am not sure that satisfying the bitcoin.org criteria to achieve a higher score is what our users need.

I am not saying we shouldn't listen to them, we must always be thankful to smart people sharing honest feedback we us (thanks guys), What I am saying is that IMO we can use the same time and money to provide much better features.

I understand that there are some low hanging fruit like deterministic builds that we could implement but I would say that that is not a priority, we can do it when we can make a PR to bitcoin.org when it is done.
2FA is another thing that I think will be developed only for scoring higher in bitcoin.org. What our users are asking for again and again is integration with hardware wallets, better privacy features (I have my own list too) but no one has asked for 2FA.

The Bitcoin Core integration is the one that I agree most. I think we should allow Wasabi to connect with a local/LAN node. In that way we are improving privacy because we don't connect to random nodes, performance because Wasabi can fetch blocks from the local node using the p2p protocol, bandwidth , etc. However I do not agree with distributing the bitcoin core with Wasabi, I think what @NicolasDorier says is the way, I am 100% with that approach.

The Tor integration with NBitcoin is something that I would like to have and we just need to find the best way to do it. Implementing the Tor protocol is possible (but the rendezvous protocol necessary for connecting with hidden services is more complex) but I think a first approach could be simply allowing NBitcoin to connect through a socks5 proxy.

It would be great to have an audit and I have received many in my life I my experience they are very useful.

I agree with the most of the alternatives, those are to me the ones that add higher value for our users by far.

@lontivero
Copy link

About the problem we the samurai wallet I remember the were connecting with a node through the RPC interface so, there have exposed the RPC interface. That's not what we have are planning to do, the idea is simply connect with a node in the LAN using the p2p.

@harding
Copy link

harding commented Jan 31, 2019

I also would not recommend putting too much energy into getting the best possible score on Bitcoin.org. Because of the way the data are presented on that site (only after you choose what wallet you want to look at), it's unlikely that better scores increase the number of downloads much. Wasabi already scores better than almost all wallets on that site, so anyone using that site to compare it to other wallets shouldn't be disappointed. Finally, Wasabi focuses on a particular use case (privacy) whereas Bitcoin.org is by necessity focused on more general wallet concerns, so you'll always be compared there based primarily on your most basic features (those most wallets have in common) rather than your more advanced features, where I think you excel.

That said, I would very much like it if Wasabi could use my own full node for verification of transactions received by my wallet. Using it just on the same localhost as Bitcoin Core is an easy start, and over LAN would be nice.

Tor connection of bitcoind must be enforced by default, because of transaction broadcasting over clearnet, which we can only do if we ship. [...] Or should we just testacceptmempool and then broadcast transaction with NBitcoin over Tor?

I would suggest that you just continue doing sending over Tor the way you are now. As long as the transaction is being sent to a hidden service, it shouldn't be any easier to trace it back to a particular Wasabi wallet or a Bitcoin Core node.

To use testmempoolaccept, you'd need to interact with Bitcoin Core over RPC, and I'd suggest avoiding that if at all possible. It's also not necessary for this particular case. You can just send the transaction to the remote node over Tor, wait a minute or two for the tx to propagate[1], and then send a P2P protocol getdata(txid) to your personal node. If your node has the transaction, you node thought it was valid. If not, something went wrong.

[1] A few recent changes by Bitcoin Core have slightly slowed down tx propagation (but not block propagation) in order to enhance privacy.

@crwatkins
Copy link

I'm a bit swamped with some other issues, but I wanted to take a quick moment to echo @harding's recommendation to not let the bitcoin.org scoring sway your development priorities. There are a lot of good ideas in the scoring list, but if an item is a good idea for Wasabi, then do it because it is a good idea and not because it will get you a better (or different) score.

I'll try to get back to you shortly with some clarifications on possible misconceptions about the scoring, along with my own personal opinion on which might be best for Wasabi.

@NicolasDorier
Copy link

One idea, integrating https://github.com/bitcoin/bips/blob/master/bip-0151.mediawiki support in NBitcoin.
Then I can make a proxy between Bitcoin and Wasabi which do the decryption and implement BIP157.

Then on the remote bitcoin core, you would just have to activate TOR.

@nopara73
Copy link
Contributor Author

nopara73 commented Feb 1, 2019

@NicolasDorier User's Bitcoin Core cannot implement our client side filtering stuff. I mean it can, but it's not feasible. It takes about 2-3 weeks on a badass server to build the filter table with txindex=1. (Despite we only build bech32 filter table.)

@harding
Copy link

harding commented Feb 1, 2019

@nopara73 you shouldn't need txindex to build the filter table, so perhaps you're building it in an inefficient way (e.g. using getrawtransaction to get each transaction individually rather than getting all the transactions from a block simultaneously using getblock <hash> 2). For comparison, the PR to integrate BIP157 filters directly into Bitcoin Core (which will obviously be faster than using RPCs), the time to generate filters for all transactions in the entire block chain was less than an hour, see Bitcoin Core PR 14121

@lontivero
Copy link

@harding we are not using getrawtransaction but getblock RPC. Anyway, you're right, it could be much faster to fetch block using the p2p protocol. In fact we generated the filter for the blockchain (since the activation of segwit) with a very inefficient code that we improved for several orders of magnitude after. Obviously we cannot have the C++ performance here.

Your other point about testmempoolaccept RPC is a bit more difficult to change, in fact there are few alternatives and they all are controversial. We use it because the coinjoin building process accepts unconfirmed coins (only if they come from a previous Wasabi coinjoin) and the bitcoin node has very good reasons to keep under some limits the size of the unconfirmed transactions' tree waiting in the mempool so, during the coinjoin building process we add the unconfirmed coin to the coinjoin transaction and invoke testmempoolaccept to check if the inclusion of that coin makes the mempool acceptance policy fail. We pushed a PR to introduced a change in bitcoin but it was not the best solution it seems. So, it is a bit ugly but it allow us to prevent some DoS attacks.

I propose to move these technical discussions to new issues and let this issue to agree on the development direction. If you agree I can create the issues, just let me know.

@harding
Copy link

harding commented Feb 1, 2019

@lontivero oh, sorry for derailing the thread. No need to create separate discussions; I'll just unsubscribe from notifications on this thread so that I'm not tempted to comment any more.

@nopara73
Copy link
Contributor Author

nopara73 commented Feb 1, 2019

@harding I looked at the code and we only do getblockcount and getblock RPC queries, so txindex is indeed not needed.
@lontivero Indeed it's been optimized by orders of magnitude by then. I remember I paid attention to do it as cleanly and simply as possible without caring about performance at all since it was just a "to be ever done once and be done with it."

We maintain a bech32 utxoset from segwit activation. We update it against all the tansactions ever since. (Meanwhile building the block filters.)

I also looked at Core's code, but I don't understand it. I also tried to find the main idea on how to build those filters so fast, but I don't find it written down anywhere. Do you happened to know? Or is it just micro-optimizations? Like manipulating bits and bytes, using pointers, efficient data structures and stuff? Honestly don't see how one could avoid iterating through every single transaction ever happened with an utxo-set at the time of the transaction.

Stepping back from the details, filter building shouldn't be delegated to clients. I mean Bitcoin Core has all the data anyway the filters would tell Wasabi without even waiting hours and CPU time to build the filters.

I propose to move these technical discussions to new issues and let this issue to agree on the development direction. If you agree I can create the issues, just let me know.

You are right, it started out as a "what to do next with Wasabi?" and ended up being a "hash tree is more efficient than an array" kind of thread and I realize this whole comment of mine doesn't help either.

@nopara73
Copy link
Contributor Author

nopara73 commented Feb 1, 2019

@lontivero So, you think we should start working on hardware wallet integration next? It's true this topic may comes up most frequently and people cashing out to their hardware wallets are a privacy risk for the rest of us.

@lontivero
Copy link

I think we should continue working on improving the privacy of our users and there are two main features IMO that goes in that direction:

Allow sending money hidden in the coinjoin transactions

Strategically speaking we should consider to implement this soon because it is to me far superior to interactive P2EP alternatives because it offers far higher privacy, it is really permissionless (you don't need the receiver to share utxos with you) and it doesn't require the receiver to have money in order to allows you to have privacy, it can allow users send money almost for free, it can help to increase even more the anonymity set of the coinjoins, etc.

OTOH there is a lot of effort by other teams on bustapay, payjoin and stowaway (and probably others) and we could risk being let behind. I think this feature can make us clearly different, and clearly the best from a privacy point of view.

Hardware wallet integration

This is the most requested feature and we have no alternative other than implement it. From a privacy point of view I think it is an important feature because HW users have no coins control and they are loosing privacy often (I have no proof but that's what I believe).


Others

The most frequently asked question is "Why does my exchange say 'Invalid bitcoin address'?". This is not in the list of things we have already discussed but the bech32 adoptions is surprisingly low and I don't see that situation changing soon. We should have a discussion about it.

@nopara73 nopara73 transferred this issue from WalletWasabi/WalletWasabi Feb 1, 2019
@nopara73
Copy link
Contributor Author

nopara73 commented Feb 1, 2019

@lontivero

Hardware Wallets

I tried to come up with a hardware wallet integration strategy, but I failed. I don't think we can do that. #42

Reverse CoinJoins

I don't think it should be our next move, I want the code to mature a bit more before we start this. We already deviated way too much from the ZeroLink specification. Let's be more wishful about it.

Bech32 Compatibility

I'd be fine working on that. We can create a temporary wrapped segwit address to facilitate users to send-sweep money to the wallet and also we could work on implementing bech32 sendability to BitcoinJ, which could probably make the largest impact on the ecosystem: WalletWasabi/WalletWasabi#951

@crwatkins
Copy link

Although this conversation seems to have moved on, I'll comment on the scoring that I promised yesterday:

Currently: checkfailvalidationcentralized. I believe the above improvement would make it checkneutralvalidationvariable. Or is it checkgoodvalidationfullnode? Not sure. @crwatkins can you help me out with this?

checkneutralvalidationvariable is for hardware wallets that don't do their own validation, so it would most likely be checkgoodvalidationfullnode if you provide adequate warnings to users while not in full node validation mode.

Current: checkfailenvironmentdesktop. This is another thing I'm not sure about if we can improve upon it. There are two levels we can target: checkgoodenvironmenthardware and checkpassenvironmenttwofactor.

checkgoodenvironmenthardware is for hardware wallets. checkpassenvironmenttwofactor is for multisig wallets which use a third party to cosign the transaction based on 2FA.

And one you didn't ask about:

Currently: checkpassfeecontroldynamic, next level to target: checkgoodfeecontrolfull.
This requires RBF and CPFP.

checkgoodfeecontrolfull actually only requires RBF or CPFP support (not both).

Those are the bitcoin.org clarifications as I understand them.

@nopara73
Copy link
Contributor Author

nopara73 commented Aug 1, 2019

Superseded by #68

@nopara73 nopara73 closed this as completed Aug 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants