LLFourn / bdk_core_staging

Staging area for bdk_core initial development

15 stars 12 forks source link

RPC wallet syncing example with Functional tests #170

Open rajarshimaitra opened 1 year ago

rajarshimaitra commented 1 year ago

This PR adds the complete RPC syncing example with functional tests added in bdk_rpc_wallet_example/tests.

The RPC sync is by the "import into a core wallet" approach.

The functional tests covers 3 basic tests

Basic sync
Basic reorg situation
Sending transactions.

This can be used as a reference to use the bitcoind backend to run similar electrum and esplora functional tests.

More tests would be added in this PR or later.

codecov-commenter commented 1 year ago

Codecov Report

Merging #170 (cb9957b) into master (2a11787) will not change coverage. The diff coverage is n/a.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##           master     #170   +/-   ##
=======================================
  Coverage   56.74%   56.74%           
=======================================
  Files          10       10           
  Lines         252      252           
=======================================
  Hits          143      143           
  Misses        109      109

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

rajarshimaitra commented 1 year ago

The latest update does the following:

Fix the failing tests.
Adds docs and comments.
Merge all the RPC and tests commits together.
Overrides #79 .

rajarshimaitra commented 1 year ago

Thanks @LLFourn for the review.. Updated as suggested.

Helper functions moved to main.rs.
wallet_sync moved to lib.rs.
Couldn't remove the struct RpcClient as bdk_cli::handle_commands expect a generic trait bound Broadcast and we can't impl foreign traits on foreign types.
- Blockchain tests only testing for sparsechain structure.
- We can't do create transaction test if we aren't fully syncing the tracker, which is the case currently as we are only getting the update_sparsechain from wallet_sync.

LLFourn commented 1 year ago

Thanks @rajarshimaitra.

* Helper functions moved to `main.rs`.

* `wallet_sync` moved to `lib.rs`.

* Couldn't remove the struct `RpcClient` as `bdk_cli::handle_commands` expect a generic trait bound `Broadcast` and we can't impl foreign traits on foreign types.

We could fix this by getting rid of the Broadcast trait and just passing in a closure to the cli lib's main handler that takes a tx and broadcasts it. See #198

* Blockchain tests only testing for sparsechain structure.

* We can't do create transaction test if we aren't fully syncing the tracker, which is the case currently as we are only getting the `update_sparsechain` from `wallet_sync`.

This is good because this library doesn't provide create tx functionality.

rajarshimaitra commented 1 year ago

We could fix this by getting rid of the Broadcast trait and just passing in a closure to the cli lib's main handler that takes a tx and broadcasts it. See https://github.com/LLFourn/bdk_core_staging/pull/198

That totally makes sense.. Will update accordingly.. Should I just build this over #198 or port the change?