Fix SQLite panic when syncing many UTXOs

[notmandatory]

Description

fixes #1827

I wrapped the SqliteDatabase sqlite connection in an Rc so the same connection can be reused when a batch is started. This prevents a large sync from trying to start and commit a batch (db transaction) while the initial non-batch connection is still busy writing it's data.

Notes to the reviewers

I had to comment out the test_batch_script_pubkey() test for sqlite since the fix in this PR prevents a new sqlite DB connections from being created when a batch is started. In practice our sync'ing code does all its initial work without starting a batch (db transaction), then before finishing it starts a batch to do some cleanup work and commits it.

Changelog notice

• Fix SQLite panic when syncing many UTXOs Fix SQLite panic when syncing many UTXOs #1828

Checklists

All Submissions:

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

Bugfixes:

• This pull request breaks the existing API
• I've added tests to reproduce the issue which are now passing
• I'm linking the issue being fixed by this PR

[notmandatory]

Test completes without panic in ~58 mins.

test blockchain::electrum::test::test_electrum_large_num_utxos ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 327 filtered out; finished in 3501.34s

[notmandatory]

I removed this since it's no longer needed.

[notmandatory]

Rebased on release/0.30 branch.

[notmandatory]

The sync is still very slow, but that's due to how electrum syncing works. It requests complete histories for all addresses for every sync. To support these sort of very large UTXO wallets on a mobile device I think a protocol like CBF (kyoto) is the only way to do it. CBF syncs block by block, so once blocks with new TX are processed they don't get processed again. Down side of CBF is it doesn't check the mempool, but a way to combine CBF for committed TX and electrum or esplora to get unconfirmed mempool TX should work.

[nymius]

I'm trying to run test_electrum_large_num_utxos, reducing NUM_TX to 10, as you mentioned in this comment, but I'm getting dependency errors. Could you provide more insight about how to test it? I think I missing an important part of the setup.

[reez]

Just adding some info when pointing to this branch from bdk-ffi and building.

I'm using a branch based on the bdk-ffi 0.31.2 tag.

That branch was pointing to bdk version 0.29.0, so I pointed to this PR branch instead "https://github.com/notmandatory/bdk", branch = "fix/many_utxos"

I won't list every error because from what I can tell the errors all seem to stem from the same root cause of Rc not being Send + Sync, but definitely let me know if you'd like to me to follow up with them.

bdk-ffi % cargo build

error[E0277]: `Rc<Connection>` cannot be sent between threads safely
     |
1461 | uniffi::deps::static_assertions::assert_impl_all!(r#Wallet: Sync, Send);
     |                                                   ^^^^^^^^ `Rc<Connection>` cannot be sent between threads safely
     |
     = help: within `AnyDatabase`, the trait `std::marker::Send` is not implemented for `Rc<Connection>`, which is required by `wallet::Wallet: Sync`

[notmandatory]

I'm replacing the Rc with a r2d2::Pool which is Send + Sync and builds with bdk-ffi. But I'm also having trouble re-running the test so trying to figure that out now.

[notmandatory]

I retested with the r2d2 connection pool and test passes with no errors:

test blockchain::electrum::test::test_electrum_large_num_utxos ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 327 filtered out; finished in 3427.42s

I also verified bdk-ffi builds with no errors. We just need to add one new bdk-ffi::BdkError enum variant in the UDL:

[Error]
enum BdkError {
  ...
  "R2d2",
};

[notmandatory]

I'm trying to run test_electrum_large_num_utxos, reducing NUM_TX to 10, as you mentioned in this comment, but I'm getting dependency errors. Could you provide more insight about how to test it? I think I missing an important part of the setup.

You should be able to test it now. I think I forgot to check in some dependency fixes that were preventing the electrum tests from building.