Skip to content

Fix BitcoinClient LoadWallet Errors#48

Open
sangaman wants to merge 2 commits intodecentralized-identity:mainfrom
sangaman:loadwallet
Open

Fix BitcoinClient LoadWallet Errors#48
sangaman wants to merge 2 commits intodecentralized-identity:mainfrom
sangaman:loadwallet

Conversation

@sangaman
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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