decentralized-identity / sidetree

Sidetree Specification and Reference Implementation
https://identity.foundation/sidetree/spec
Apache License 2.0
438 stars 112 forks source link

bitcoin core v23 creates descriptor wallets by default #1192

Open LiranCohen opened 2 years ago

LiranCohen commented 2 years ago

Bitcoin Core v23 has descriptor wallets as the default for the createwallet RPC.

Since SideTree isn't currently supporting descriptor wallets, the easiest path forward is to set descriptors parameter to false.

Legacy wallets require Berkeley DB in order to work. I usually build from source and include BDB so I am leaving this in draft mode until I can test against the built release binaries.

I will do some more testing and update the ION Installation documentation to reflect the requirement of Berkeley DB, as well as potentially update the provided script to pull in v23 of bitcoin core binary.

coffnix commented 2 years ago

Take a look:

https://bitcoin.stackexchange.com/questions/99620/why-is-the-bitcoin-core-wallet-database-moving-from-berkeley-db-to-sqlite

https://github.com/bitcoin/bitcoin/issues/20160

LiranCohen commented 2 years ago

Thanks @coffnix!

So yeah it seems like we would need to migrate away from legacy wallets before v26 (late 2023).

Setting createwallet descriptor parameter to false should be fine for the time being, but I'll see if I can start working on a separate PR for supporting descriptor wallets within SideTree/ION.

LiranCohen commented 2 years ago

I've done some more testing and the binary comes compiled with BDB which would allow for descriptors to be set to false and work.

However this change is not backward compatible with anything before v0.21.0 when the descriptors option was added.

Do we want to force a newer release of ION to be only compatible with v0.21.0+ or do we want to check for a version and based on that include the descriptors parameter in the walletcreate rpc?

I will also make an update to the bitcoin core installation script that is provided within the ION Installation guide to check that it installs bitcoin core v23 correctly.

Opening this PR for review and discussion.

coffnix commented 2 years ago

if possible we could already make the new version compatible with Sqlite

LiranCohen commented 2 years ago

if possible we could already make the new version compatible with Sqlite

That would take modifying to support descriptor wallets. I've put together a placeholder Issue for researching what it would take and tracking any progress: https://github.com/decentralized-identity/ion/issues/304

Not sure its possible to do so given the current project structure, but this seems like a PR for the ION repo, not Sidetree.

@OR13 The code for the bitcoin core RPCs unfortunately live within the Sidetree repo, I was surprised to find that myself when debugging this. That could be also a separate Issue/PR to try and accomplish.

thehenrytsai commented 2 years ago

Just providing an update that I have not forgotten about this PR. I'll be looking into this soon, as soon as I get the improvement to docker support work done!

thehenrytsai commented 2 years ago

@LiranCohen, my ION docker PR finally got merged in. I will be therefore finally be looking into testing this PR out in the next few days!

I am in the camp that it is entirely fine to break compatibility for any pre v23.0 versions as it can act as a forcing function to not use a to-be-obsolete version anyway. But based on what you said, your change is compatible with v21+, so it could be as simple as:

  1. updating the docker image to use v23; and
  2. initialize the DB using legacy Berkley DB

But your insights in https://github.com/decentralized-identity/ion/issues/275 tells me it might be way more complicated that that. Will report my progress here.

LiranCohen commented 2 years ago

@thehenrytsai Thanks!

I think it should be pretty straight forward like you mention, let me know if you need any help testing or are having any issues.

decentralgabe commented 1 year ago

this should be moved to the new impl repo https://github.com/decentralized-identity/sidetree-reference-impl