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

Fix BitcoinClient LoadWallet Errors #48

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sangaman
Copy link

Don't pass second param to loadWallet

This second boolean parameter causes an error on error on Bitcoin Core v0.20.1 and v25.0. It is not a documented parameter in v25 and newer versions of Bitcoin Core.

This parameter, in some versions of bitcoin, was supposed to cause a wallet to be automatically loaded, but this is unnecessary as sidetree makes a loadwallet call anyway.

Catch duplicate loadWallet error

Copy link
Contributor

@thehenrytsai thehenrytsai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @sangaman, thanks for submitting the PR. The PR looks good but just for sanity: Were you able to run the ION node on other versions with no issues? My node is running on v0.21.0 which is newer than v0.20.1 but it doesn't seem to have this issue.

sangaman added 2 commits July 22, 2023 02:30
This second boolean parameter causes an error on Bitcoin Core v0.20 and
older versions.

This parameter, in newer versions of bitcoin, is supposed to cause a
wallet to be automatically loaded, but this is unnecessary as sidetree
makes a loadwallet call anyway.
@sangaman
Copy link
Author

Hey @sangaman, thanks for submitting the PR. The PR looks good but just for sanity: Were you able to run the ION node on other versions with no issues? My node is running on v0.21.0 which is newer than v0.20.1 but it doesn't seem to have this issue.

Hi @thehenrytsai, thanks for your attention. I think I was actually mistaken earlier that v25.0 doesn't support this second load_on_startup parameter. In fact it looks like v0.21 and above DO support this parameter, but v0.20 and older do not. So really this PR just enables compatibility with bitcoin core version 0.20 and below. It does remove this load_on_startup flag for all versions, however it seems to me that isn't a problem (and may even be preferred) since the loadwallet call is made every time ION starts up regardless.

The reason I'm using v0.20.1 is because of this issue decentralized-identity/ion#275 - and it seems the PR resolving it (decentralized-identity/sidetree#1192) is still not merged.

I fixed a couple of tests and reworded the commit message to reflect that this fix only address v0.20 versions of bitcoin core and below.

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

Successfully merging this pull request may close these issues.

2 participants