bdk icon indicating copy to clipboard operation
bdk copied to clipboard

Add functions to test persistence

Open 110CodingP opened this issue 4 months ago • 7 comments

Description

Added basic tests to the testenv for testing custom persistence backends. Partially solves bdk_wallet#14 and might help with bdk_wallet#234.

Notes to the reviewers

An alternative approach could be to create a struct whose methods are these tests but the current approach seems better since these tests are meant to be used as unit tests. For the same reason redundant functions to check persistence of individual fields of the ChangeSets are provided.

Update

  • Modified the default feature of testenv to specify bdk_chain/default as the only optional dependency.
  • Feature gate test utils functions introduced by default feature.

Changelog notice

Changed
    - default feature in testenv to specify only `bdk_chain/default` as the optional dependency.
Added
    - functions to testenv to check `ChangeSet`s are persisted and merged `ChangeSet`s are loaded properly ,gated by the `default` feature flag.
    - tests to `file_store` and `chain` based on utilities added to `testenv`.

Todo

  • [x] Add docs
  • [x] Add tests to bdk_chain based on testenv utilities

Checklists

All Submissions:

New Features:

  • [x] I've added tests for the new feature
  • [x] I've added docs for the new feature

110CodingP avatar Aug 12 '25 19:08 110CodingP

I tried re-running CI to no avail, maybe try force pushing.

ValuedMammal avatar Aug 20 '25 14:08 ValuedMammal

Great idea. ConceptACK

evanlinjin avatar Aug 29 '25 05:08 evanlinjin

In 10c42dc I added a miniscript feature to testenv but I am not sure if this is the correct approach. Earlier I was including the miniscript feature of bdk_chain by default but that doesn't work since miniscript requires either std or no-std but cargo doesn't allow specifying the feature of a dependency of a dependency. Adding miniscript as a separate dependency also does not work since we require bdk_chain/miniscript. But the current solution is also not satisfactory since miniscript feature requires the std feature!

110CodingP avatar Sep 14 '25 18:09 110CodingP

In 10c42dc I added a miniscript feature to testenv but I am not sure if this is the correct approach. Earlier I was including the miniscript feature of bdk_chain by default but that doesn't work since miniscript requires either std or no-std but cargo doesn't allow specifying the feature of a dependency of a dependency. Adding miniscript as a separate dependency also does not work since we require bdk_chain/miniscript. But the current solution is also not satisfactory since miniscript feature requires the std feature!

We could have the testenv default feature enable bdk_chain/default, which in turn enables miniscript/std. I'm not aware of anyone needing a miniscript/no-std feature for the testenv crate.

ValuedMammal avatar Sep 20 '25 13:09 ValuedMammal

Should the miniscript feature introduced be removed? I was thinking if some users might not want bdk_chain/miniscript feature to be enabled?

110CodingP avatar Sep 22 '25 17:09 110CodingP

miniscript feature still depends on std feature of testenv since ig it would be confusing otherwise like when miniscript is enabled but not std even though miniscript would activate bdk_chain/std. Made miniscript a default feature too. Sorry if I am overthinking :sweat_smile: .

110CodingP avatar Sep 24 '25 08:09 110CodingP

~~Since this is currently based on the master branch should we re-assign this to the 3.0 milestone and then do a new PR to back port it to 2.2?~~ Nevermind, this is for the bdk crate so my question doesn't apply.

notmandatory avatar Sep 24 '25 20:09 notmandatory