JoinMarket-Org / joinmarket-clientserver

Bitcoin CoinJoin implementation with incentive structure to convince people to take part
GNU General Public License v3.0
728 stars 178 forks source link

Implement Bitcoin Core descriptor wallet support #1571

Open kristapsk opened 1 year ago

kristapsk commented 1 year ago

No BDB creation, unless -deprecatedrpc=create_bdb was just merged into Bitcoin Core master and is planned to be released with v26. According to schedule, planned release date is 2023-11-22.

Currently only legacy (BDB) Core wallets are supported by JoinMarket. This will cause problems for users if don't get out JoinMarket release with descriptor wallet support before Bitcoin Core 26.0 is released.

There was old fast attempt by me to already add descriptor wallet support in #1064, but it was decided back then that using addr() descriptors is not the best way, wpkh() would be proper way. But that will need more changes at JoinMarket codebase. Also there is no 100% certainty about custom gaplimit handling, see discussion in the same #1064.

AdamISZ commented 1 year ago

I agree that it's getting quite urgent. Indeed even today, good luck compiling Core with the legacy wallet support :)

kristapsk commented 11 months ago

Bitcoin Core v26.0 has been tagged - https://github.com/bitcoin/bitcoin/releases/tag/v26.0.

kristapsk commented 8 months ago

Legacy wallet removal will be one of priority projects for Bitcoin Core 28.0. https://github.com/bitcoin/bitcoin/issues/29465#issuecomment-1986888307

laanwj commented 7 months ago

Correct, version 28.0 (development on current master now that 27.0 is branched off) is extremely likely not going to have legacy wallet support anymore. We'll have to find some solution for this.

danielqba commented 7 months ago

Are there any docs on how to create a new wallet in the meantime?

kristapsk commented 7 months ago

@danielqba https://github.com/JoinMarket-Org/joinmarket-clientserver/blob/master/docs/USAGE.md

s373nZ commented 6 months ago

I'm very curious about this issue and hesitant to try using JoinMarket before it is resolved. Probably don't understand all the nuances fully, but is the reason JoinMarket uses old BDB wallet format in order to increase the anonymity set?

I'd love to see some movement on this issue, and even help out, but I don't know enough yet to understand the context. Where could one learn more about the background on this?

kristapsk commented 6 months ago

is the reason JoinMarket uses old BDB wallet format in order to increase the anonymity set?

No, reason is pure Bitcoin RPC API incompatibilities between BDB and descriptor wallets. JoinMarket uses importmulti call, which is not available for descriptor wallets. We could hack around by just importing addresses descriptors using importdescriptors instead (see #1064), but that does not seem to be the best solution, could import xpub's instead, but then need to figure out is it possible to do custom gap limits.

No difference in anonymity set, both wallets would use native segwit P2WPKH addresses currently.

achow101 commented 4 days ago

but then need to figure out is it possible to do custom gap limits.

How custom of gap limits do you need?

If you are running bitcoind, the -keypool= option will be present and it controls the default gap limit. When importing a descriptor, the range can be specified and that range will override any -keypool setting for the initial import. If the range is bigger than the -keypool, nothing new will be generated until enough addresses are retrieved to make the unused range smaller than -keypool. However, you can also use the keypoolrefill RPC to periodically force additional lookahead addresses to be generated regardless of -keypool.

kristapsk commented 4 days ago

but then need to figure out is it possible to do custom gap limits.

How custom of gap limits do you need?

We have custom gap limit parameter that can be specified for any call to wallet-tool.py that is affected by it:

  -g GAPLIMIT, --gap-limit=GAPLIMIT
                        gap limit for wallet, default=6

By default JM has gap limit of 6 (default can be changed in joinmarket.cfg) and imports new watch only addresses in legacy wallets as needed. But you might want to specify custom one when doing wallet recovery or rescan.

However, you can also use the keypoolrefill RPC to periodically force additional lookahead addresses to be generated regardless of -keypool.

Probably this is the solution, will look into it.

Another nuance - when JM yield generator initiates participating in coinjoin process, it gets change address, which is never reused again, even if coinjoin does not succeed. JoinMarket knows that this address is kinda "used", but Bitcoin Core will not, as there are no actual transactions to that address.

whitslack commented 4 days ago

Another nuance - when JM yield generator initiates participating in coinjoin process, it gets change address, which is never reused again, even if coinjoin does not succeed. JoinMarket knows that this address is kinda "used", but Bitcoin Core will not, as there are no actual transactions to that address.

Why wouldn't you call getnewaddress on the Bitcoin Core wallet? Bitcoin Core will remember the "current" index of the descriptor and will not return the same address again.

In general it seems like JoinMarket has been reinventing the wheel a lot when it comes to the wallet. There's really no need to duplicate all the wallet functionality in JoinMarket. You really could just use one descriptor wallet in Bitcoin Core per mixdepth and do away with almost all of the wallet code in JoinMarket. You can simply call listunspent to get the UTxOs in a mixdepth, and you can sign PSBTs using walletprocesspsbt.

Maybe JoinMarket was originally designed in an era when the Bitcoin Core wallet lacked needed functionality, but I think Bitcoin Core has caught up, and JoinMarket's redundant effort is looking kind of silly at this point.

PulpCattel commented 1 day ago

Thank you @achow101!

+1 what @whitslack said. I'd love to see JoinMarket move in that direction (at least to the extent possible) and I'd be happy to review and test PRs that help us get there.