LLFourn / bdk_core_staging

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

Allow custom chain index parameter #61

Closed LLFourn closed 1 year ago

LLFourn commented 1 year ago

Usually transactions are indexed by their height. But you might want to extend this (e.g. include block position) or some other metadata.

I've made the index quite restrictive (e.g. requires Copy) since we'll need to persist the data and for our simple file based persistence we'll need it to be fixed size for now.

This supersedes #60.

TODO: This fixes a bug where an inclusive txid range search would not find the txids because we set the range to Txid::all_zeros (unless the txid was all zeroes). We need a test for that. I also added an API to search via the index itself. Handy if you want to do pagination of transactions by your own index (a good one would be block position) as mentioned in https://github.com/bitcoindevkit/bdk/issues/794

codecov-commenter commented 1 year ago

Codecov Report

Merging #61 (3090530) into master (17e0698) will increase coverage by 14.37%. The diff coverage is 41.66%.

@@             Coverage Diff             @@
##           master      #61       +/-   ##
===========================================
+ Coverage   55.04%   69.42%   +14.37%     
===========================================
  Files           8        7        -1     
  Lines        1228      906      -322     
===========================================
- Hits          676      629       -47     
+ Misses        552      277      -275     
Impacted Files Coverage Δ
bdk_core/src/sparse_chain.rs 19.23% <41.66%> (+2.21%) :arrow_up:
bdk_core/src/chain_graph.rs

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

evanlinjin commented 1 year ago

@LLFourn I mostly agree with the approach here. However, my main concern is with ChainIndex being a trait. I think we should have TxHeight explicitly included at the start of ChainIndex. I've continued the work of this PR in #60 whilst porting over the fixes and improvements introduced here.

Tests for ranging is also added in #60

evanlinjin commented 1 year ago

Replaced by #60

After looking through both code-bases, I think I will go with #60 instead of #61 as I feel it is more idiomatic, and in terms of functionality, they don't differ too substantially from my understanding.