-
Notifications
You must be signed in to change notification settings - Fork 112
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
bitcoin core v23 creates descriptor wallets by default #1192
base: master
Are you sure you want to change the base?
Conversation
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. |
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. |
if possible we could already make the new version compatible with Sqlite |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure its possible to do so given the current project structure, but this seems like a PR for the ION repo, not Sidetree.
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:
@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. |
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! |
@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:
But your insights in decentralized-identity/ion#275 tells me it might be way more complicated that that. Will report my progress here. |
@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. |
… parameter for descriptors
8322d03
to
2258a2f
Compare
this should be moved to the new impl repo https://github.com/decentralized-identity/sidetree-reference-impl |
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.