[DRAFT] added sqlx, removed rusqlite

[matthiasdebernardini]

Description

async postgres support for persistance

Notes to the reviewers

Here I am implementing async postgres via the SQLx crate. Currently there is only synchronous support via the FileStore and rustqlite implementations.

A big issue with this PR is that Rusqlite uses libsqlite3, and so does SQLx but with different versions. Cargo does not allow this, even though it should work, there is currently a bug that the SQLx maintianers showed me that explains this behavior. Nevertheless, including both sqlx and rusqlite in the same workspace will not work - bdk having a strict msrv requirement also does not help (once the fix lands in cargo, it will take time for the msrv of cargo to catch up).

My issues are with the PersistAsyncWith trait, I was unable to implement it. Some guidance would be appreciated.

[tnull]

Given that SQLx is very explicit about not supporting any kind of MSRV, I don't think switching to it would be a real option?

I think the operations for ChangeSet persistence are really some really trivial INSERTs/SELECTs, I wonder if we could just write a thin wrapper around libsqlite3-sys ourselves rather than pulling in a completely separate project without MSRV and a larger dependency tree? FWIW, we've been looking towards doing the same for LDK/LDK Node (which currently also depends on rusqlite) for some time.

[evanlinjin]

I think you forgot to include these files in the PR.

[matthiasdebernardini]

This should no longer be a problem

[evanlinjin]

@matthiasdebernardini thank you for this PR. I think I figured out the problem. The trait definition was not strict enough with lifetime bounds. Please see here: https://github.com/matthiasdebernardini/bdk/pull/2

I'm wondering if #1547 will be in the beta. If so, maybe it's best I do the fix there. Edit: The fix is included in 8498a5f.

[evanlinjin]

Looking at @tnull's comment, @matthiasdebernardini can you please elaborate on what you specifically need. Is it postgres support, async support, or do you specifically need postgres + async?

It'll be easier to assist if we know what the goals are.

[matthiasdebernardini]

@evanlinjin we want to store our users wallets and load them quickly so the wallet feels snappy, that is our high level goal. We picked postgres because we like it and trust it. We are very paranoid about losing our customers wallet descriptors. Initially, I was storing the flat files in their own column, reading/writing it from the disk and uploading/downloading it from postgres (I had it working in S3 at one point as well).

The reason I want to store it remotely is that we want to keep our bdk api webserver stateless, so the sqlite/flat file method wont be a good fit for us.

As far async support is concerned, I am already running all our bdk code within an axum webserver, so using it is advantageous for scenarios where we need to fetch a lot of wallets all at once. We could go with a sequential (non-async/normal) method of persisting but async would be our preference.

In short, we don't need either, but async postgres is our #1 preference by a long shot. I agree with @tnull our needs are very basic. I picked SQLx because I prefer SQL over an ORM and I was having a lot of success using it to finish our app. I do think if you want to offer async remote persistence for your users out-of-the-box now, then a different crate may be better.

[evanlinjin]

@matthiasdebernardini if we do the sqlx persistence in a separate crate, would we still run into the bug you mentioned in the PR description?

[matthiasdebernardini]

It should work as long as you don't enable the rusqlite feature in that project, yes.

[matthiasdebernardini]

I think reworking the lifetimes might help, I am still unable to make it compile.

error: lifetime may not live long enough
  --> crates/wallet/src/wallet/persisted.rs:36:13
   |
11 | impl<'c> chain::PersistAsyncWith<sqlx::Transaction<'c, sqlx::Postgres>> for Wallet {
   |      -- lifetime `'c` defined here
...
20 |         db: &mut sqlx::Transaction<'c, sqlx::Postgres>,
   |             - let's call the lifetime of this reference `'1`
...
36 |             Box::pin(create(db, params))
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ associated function was supposed to return data with lifetime `'c` but it is returning data with lifetime `'1`

error: implementation of `chain::sqlx::Acquire` is not general enough
  --> crates/wallet/src/wallet/persisted.rs:36:13
   |
36 |             Box::pin(create(db, params))
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `chain::sqlx::Acquire` is not general enough
   |
   = note: `chain::sqlx::Acquire<'_>` would have to be implemented for the type `&mut PgConnection`
   = note: ...but `chain::sqlx::Acquire<'0>` is actually implemented for the type `&'0 mut PgConnection`, for some specific lifetime `'0`

error: lifetime may not live long enough
  --> crates/wallet/src/wallet/persisted.rs:56:9
   |
11 | impl<'c> chain::PersistAsyncWith<sqlx::Transaction<'c, sqlx::Postgres>> for Wallet {
   |      -- lifetime `'c` defined here
...
41 |         conn: &mut sqlx::Transaction<'c, sqlx::Postgres>,
   |               - let's call the lifetime of this reference `'1`
...
56 |         Box::pin(load(conn,params))
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^ associated function was supposed to return data with lifetime `'c` but it is returning data with lifetime `'1`

error: implementation of `chain::sqlx::Acquire` is not general enough
  --> crates/wallet/src/wallet/persisted.rs:56:9
   |
56 |         Box::pin(load(conn,params))
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `chain::sqlx::Acquire` is not general enough
   |
   = note: `chain::sqlx::Acquire<'_>` would have to be implemented for the type `&mut PgConnection`
   = note: ...but `chain::sqlx::Acquire<'0>` is actually implemented for the type `&'0 mut PgConnection`, for some specific lifetime `'0`

error: lifetime may not live long enough
  --> crates/wallet/src/wallet/persisted.rs:69:9
   |
11 | impl<'c> chain::PersistAsyncWith<sqlx::Transaction<'c, sqlx::Postgres>> for Wallet {
   |      -- lifetime `'c` defined here
...
60 |         db: &mut sqlx::Transaction<'c, sqlx::Postgres>,
   |             - let's call the lifetime of this reference `'1`
...
69 |         Box::pin(persist(db, changeset))
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ associated function was supposed to return data with lifetime `'c` but it is returning data with lifetime `'1`

error: lifetime may not live long enough
  --> crates/wallet/src/wallet/persisted.rs:69:9
   |
11 | impl<'c> chain::PersistAsyncWith<sqlx::Transaction<'c, sqlx::Postgres>> for Wallet {
   |      -- lifetime `'c` defined here
...
61 |         changeset: &<Self as chain::Staged>::ChangeSet,
   |                    - let's call the lifetime of this reference `'2`
...
69 |         Box::pin(persist(db, changeset))
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ associated function was supposed to return data with lifetime `'c` but it is returning data with lifetime `'2`

error: implementation of `chain::sqlx::Acquire` is not general enough
  --> crates/wallet/src/wallet/persisted.rs:69:9
   |
69 |         Box::pin(persist(db, changeset))
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `chain::sqlx::Acquire` is not general enough
   |
   = note: `chain::sqlx::Acquire<'_>` would have to be implemented for the type `&mut PgConnection`
   = note: ...but `chain::sqlx::Acquire<'0>` is actually implemented for the type `&'0 mut PgConnection`, for some specific lifetime `'0`

error: implementation of `chain::sqlx::Acquire` is not general enough
   --> crates/wallet/src/wallet/persisted.rs:128:9
    |
128 |         Box::pin(persist(db, changeset))
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `chain::sqlx::Acquire` is not general enough
    |
    = note: `chain::sqlx::Acquire<'_>` would have to be implemented for the type `&mut PgConnection`
    = note: ...but `chain::sqlx::Acquire<'0>` is actually implemented for the type `&'0 mut PgConnection`, for some specific lifetime `'0`

error: implementation of `Send` is not general enough
   --> crates/wallet/src/wallet/persisted.rs:128:9
    |
128 |         Box::pin(persist(db, changeset))
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Send` is not general enough
    |
    = note: `Send` would have to be implemented for the type `&Pool<Postgres>`
    = note: ...but `Send` is actually implemented for the type `&'0 Pool<Postgres>`, for some specific lifetime `'0`