search

decentralized-identity / sidetree-reference-impl

Sidetree Reference Implementation
Apache License 2.0
5 stars 7 forks source link

Fix BitcoinClient LoadWallet Errors #48

Open sangaman opened 1 year ago

sangaman commented 1 year ago

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

sangaman commented 1 year ago

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 https://github.com/decentralized-identity/ion/issues/275 - and it seems the PR resolving it (https://github.com/decentralized-identity/sidetree/pull/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.