Move wallet interoperabity tests out of testutils::blockchain_tests bdk_blockchain_tests macro

[notmandatory]

This came up during testing for #593, see afilini#4 (comment).

1. Move TestClient and related impls to testutils/mod.rs
2. Add a new testuitls/wallet_tests module with new bdk_wallet_tests macro.
3. Move tests that aren't related to blockchain syncing from bdk_blockchain_tests to new bdk_wallet_tests macro, in particular:
   • test_add_data
   • test_send_to_bech32m_addr
   • test_taproot_key_spend
   • test_taproot_script_spend
   • test_sign_taproot_core_keyspend_psbt
   • test_sign_taproot_core_scriptspend2_psbt
   • test_sign_taproot_core_scriptspend3_psbt

Open for suggestions/discussion before making any changes.

[rajarshimaitra]

My SoB mentees are working on refactoring the test modules.. Makes sense to add this into their scope of project?? Should not be too much work, or we need it before than that??

[notmandatory]

I think that'd be great if your SoB mentees have time for this too. But before they start we should get @afilini to confirm this is what he has in mind.

[afilini]

I've edited the pr body to remove some tests which I think were actually blockchain related (testing sync edge cases).

On top of the tests we already have which should just be moved, I would focus on testing interoperability with Core using psbts, for example creating a psbt in core and signing it with bdk, and vice versa.

Core has a few different rpcs for dealing with psbts, so we should also consider testing against them to see if we are compatible (like utxoupdatepsbt).

[afilini]

It would also be helpful to add some tests that spend utxos using non standard sighashes (for both taproot and segwit/legacy), because we are already testing that BDK applies them correctly, but we never check the signatures produced.

Having a Core instance available is helpful because we can try broadcasting the tx and Core will validate every aspect of it and let us know if there are any problems.

[notmandatory]

Is sending a TX to Core to broadcast better than using bitcoinconsensus code with the verify_tx function ?

[rajarshimaitra]

Is sending a TX to Core to broadcast better than using bitcoinconsensus code with the verify_tx function ?

I would guess so.. I think bitcoinconsensus only verifies that the script execution passes as per consensus rules.. But there are many other policies (p2p, mempool) etc that are enforced by core on top of that..

[rajarshimaitra]

In light of recent update with #624 .. Is this issue still relevant??

[notmandatory]

It looks like the above tests are still grouped together with the blockchain tests after being converted from macros to functions. I think we should still leave this open, but it can be done after #624 is merged.

[notmandatory]

I'm closing this since all the tests are going to be changing with the new bdk_core 1.0.0 work.