search

LLFourn / bdk_core_staging

Staging area for bdk_core initial development
15 stars 12 forks source link

Add type parameter to TxGraph<T> #171

Closed LLFourn closed 1 year ago

LLFourn commented 1 year ago

This allows create a TxGraph<&Transaction> or a TxGraph<Cow> etc. It also allows application specific transaction types.

The main motivation for this is allowing a ChainGraph<TxHeight, Cow> so we can inflate a sparse chain (inflate_update) to a chain graph from another chain graph without copying transaction data between them. This is instead of inflate_changeset which was unsound as identified in #162.

See how this improves in the electrum example.

fixes #162

codecov-commenter commented 1 year ago

Codecov Report

Merging #171 (f20ddbf) into master (f7b26aa) will decrease coverage by 10.53%. The diff coverage is 91.66%.

: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     #171       +/-   ##
===========================================
- Coverage   65.36%   54.83%   -10.53%     
===========================================
  Files           9       10        +1     
  Lines         384      248      -136     
===========================================
- Hits          251      136      -115     
+ Misses        133      112       -21
Impacted Files Coverage Δ
bdk_chain/src/file_store.rs 18.18% <ø> (ø)
bdk_chain/src/lib.rs 100.00% <ø> (ø)
bdk_chain/src/sparse_chain.rs 77.77% <ø> (ø)
bdk_chain/src/tx_graph.rs 75.00% <83.33%> (-8.77%) :arrow_down:
bdk_chain/src/chain_graph.rs 81.81% <100.00%> (ø)
bdk_chain/src/keychain.rs 56.89% <100.00%> (ø)
bdk_chain/src/tx_data_traits.rs 100.00% <100.00%> (ø)

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

LLFourn commented 1 year ago

This is ready for review. I think the codecov is very broken on this one.

evanlinjin commented 1 year ago

This is very cool and I agree with the changes. I read through everything, ran tests and ran the examples.

I encountered an error while running the electrum example (output here). Not sure if it is related to changes in this PR, or if this problem exists elsewhere.