feat!: add wallet birthday

[luisschwab]

Closes #110.

TBD

[luisschwab]

I wonder if this should be public. It gives the user the freedom to set whatever value he wants, even if they're incorrect. @ValuedMammal or @evanlinjin can you shed some light on what would happen if I create a CheckPoint with a height+hash pair that doesn't exist on the chain, and then synched from genesis?

[ValuedMammal]

Good question and probably requires testing before going ahead with this feature. The hope is that the bad block will be "reorged out" of the wallet, but that'll depend on which block IDs the chain source decides to return.

[luisschwab]

I think the best approach would be to not allow manually setting the birthday, and instead rely on the result of a full-scan to define it.

[ValuedMammal]

I was more in favor of the approach in #348 which provided an option on CreateParams to simply add a checkpoint to the local chain during Wallet creation.

[luisschwab]

I was more in favor of the approach in #348 which provided an option on CreateParams to simply add a checkpoint to the local chain during Wallet creation.

Indeed, this would be a better flow, but may conflict with a possibly inexistent BlockId set during wallet creation. I need to do some testing to see what would happen.

[evanlinjin]

I was more in favor of the approach in #348 which provided an option on CreateParams to simply add a checkpoint to the local chain during Wallet creation.

Indeed, this would be a better flow, but may conflict with a possibly inexistent BlockId set during wallet creation. I need to do some testing to see what would happen.

Some thoughts:

• Birthdays are only useful for block-based chain sources.
• bitcoind_rpc emitters trace back from a no-longer-existing-block to a block that is currently in chain (which is the correct behavior) - so no conflict at all!

The only fear is if the chain source has never even seen the no-longer-existing-block - in which case, the chain source starts syncing from genesis (as that would be the closest checkpoint that connects).

So if the user creates a wallet just before a reorg, that would be non-ideal.

My suggestion:

bitcoind_rpc::Emitter::new has a start_height parameter. This is used in the case the closest block that connects is below the start_height. So if the tip is reorged out, and we only have tip + genesis, we will start from start_height instead. To convey the wallet birthday with the checkpoint tip, while still taking into account reorgs, we can use tip_height - FINAL_DEPTH as the start_height parameter. FINAL_DEPTH can be as conservative as you want.

bitcoind_rpc::FilterIter::new does not have such a parameter. We can add it?

Then we provide docs as how to convey the wallet's birthday to the chain source using the checkpoint tip.

[codecov]

Codecov Report

❌ Patch coverage is 86.32479% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.98%. Comparing base (c1f052e) to head (41e5b65).

Files with missing lines	Patch %	Lines
src/wallet/error.rs	0.00%	8 Missing ⚠️
src/wallet/persisted.rs	0.00%	4 Missing ⚠️
src/wallet/changeset.rs	92.10%	3 Missing ⚠️
src/wallet/mod.rs	96.55%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #368      +/-   ##
==========================================
- Coverage   86.99%   86.98%   -0.02%     
==========================================
  Files          25       25              
  Lines        8860     8973     +113     
==========================================
+ Hits         7708     7805      +97     
- Misses       1152     1168      +16     

Flag	Coverage Δ
rust	86.98% <86.32%> (-0.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.