[RPC] Ignore importdescriptor error

[rajarshimaitra]

Description

Fixes #859

The importdescriptor call will throw a Resource temporarily unavialble error only in the first initial sync, where the wallet has to rescan from genesis.

The sync process carries on as usual after the error, so this patch makes it ignore only that particular error and carry on the with sync. All other errors are captured as usual.

Notes to the reviewers

To test this out, sync any descriptor, with the rpc blockchain, in signet/testnet, with a fresh database. This should fail in the current master with the following error.
Rpc(JsonRpc(Transport(SocketError(Os { code: 11, kind: WouldBlock, message: "Resource temporarily unavailable" }))))

This patch will ignore this particular error and carry on with the sync.

Checklists

All Submissions:

• I've signed all my commits
• I followed the contribution guidelines
• I ran cargo fmt and cargo clippy before committing

Bugfixes:

• I'm linking the issue being fixed by this PR

[w0xlt]

Concept ACK.
I will test this PR with a full scan.
But I consider this more of a welcome workaround than a solution.

I think it is also important to know the root cause of this error and how to prevent it from occurring.

[rajarshimaitra]

I think it is also important to know the root cause of this error and how to prevent it from occurring.

Totally. I will keep digging into this, but the root cause is likely this one: #650 (comment)

[notmandatory]

Concept ACK, needs a rebase.

[wild-kard]

I have tested this PR as I have been encountering the
bitcoin Rpc(JsonRpc(Transport(SocketError(Os{code: 11, kind: WouldBlock, message: "resource temporarily unavailable"}))))
error when invoking the .sync() method on my bdk wallet.

Ubuntu 22.04.1 LTS
Running a local bitcoin daemon with Bitcoin 23.0-x86_64-linux-gnu

After completing a complete rescan using this build I did not receive any RPC errors and was successfully able to sync.

[danielabrozzoni]

I don't think this is the right way to handle the problem, as I suppose that this won't import the descriptors in the Bitcoin Core wallet! Also see: https://discord.com/channels/753336465005608961/978744259693916230/1075135066268774462

To the people that tested this PR: did you test with a non-empty wallet, and were you able to see your funds after testing?

[wild-kard]

I don't think this is the right way to handle the problem, as I suppose that this won't import the descriptors in the Bitcoin Core wallet! Also see: https://discord.com/channels/753336465005608961/978744259693916230/1075135066268774462

To the people that tested this PR: did you test with a non-empty wallet, and were you able to see your funds after testing?

Daniela you are correct. I tested this with a non empty wallet and the funds are not showing properly.

[w0xlt]

I think the problem in the PR code is:
. The normal flow in the BDK is to wait for the import/synchronization to finish before continuing the execution.
. With this PR, execution continues when it receives the error, but Bitcoin Core is still importing/synchronizing the wallet.

That makes sense?

[rajarshimaitra]

With this PR, execution continues when it receives the error, but Bitcoin Core is still importing/synchronizing the wallet.

I think what's happening is Bitcoin core throws the error during the import call, and doesn't import all the addresses. So the initial assumption of the PR is wrong.

It seems like this has been addressed in the jsonrpc crate here apoelstra/rust-jsonrpc#72

This should be available in the latest rust-jsonrpc, which is a dep of dep for us. So we need to wait for it to land in the latest bitcoin-core-rpc.

#859 (comment)

In the mean, please feel free to suggest any other possible workaround..

[w0xlt]

I tested with jsonrpc = 0.14 locally but the error persisted.

[notmandatory]

I'm removing this from the next release milestone since it needs more work. Also any fix should be targeted to one of the 1.0.0 alpha releases after #793 is merged, and then if possible back ported to a 0.27.x bug fix release.

[danielabrozzoni]

Hey, we are in the process of releasing BDK 1.0, which will under the hood work quite differently from the current BDK. For this reason, I'm closing all the PRs that don't really apply anymore. If you think this is a mistake, feel free to rebase on master and re-open!