Skip to content

Conversation

@notmandatory
Copy link
Member

@notmandatory notmandatory commented Jun 27, 2023

Description

This is based on #1002. Putting it out there to see if anyone finds it useful to have in an example README.

Notes to the reviewers

Changelog notice

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

evanlinjin and others added 9 commits June 26, 2023 08:52
* `bdk_chain/std` should also enable `miniscript/std`
* `hashbrown/serde` should only be enabled when `bdk_chain/serde` is
  enabled
* use the version of `hashbrown` that `bitcoin` and `miniscript` is
  using
This allows the data-source thread to hold a reference to checkpoints
without a lock on `LocalChain` itself.

Introduce `LocalChain::update` that replaces `determine_changeset` and
`apply_update`. This returns a closure that actually updates `self` when
called. This method allows for efficient and elegant updating while
being able to "preview" the update before applying.

The `LocalChain` update/determine_changeset tests have been updated to
also check for the final state after applying the update (not just
looking at the changeset).

Update `keychain::LocalUpdate` struct to use `CheckPoint`

Instead of containing a complete `LocalChain`, the update uses
`CheckPoint`. This simplifies the API since updating a `LocalChain` only
requires a `CheckPoint` now.

The examples and chain source `..Ext` implementations have all been
updated to use the new API.

Additionally, `..Ext` implementations didn't 100% guarantee consistency
of the updates, the logic has been changed to enforce better guarantees.
`prune_and_apply_update` first scans all txs contained in `update`
through the index, then filters out txs using `I::is_tx_relevant` before
applying the update. This is useful for block-by-block syncing.

`Wallet::apply_update` now has a second input; `prune: bool`. If `prune`
is set, irrelevant transactions of `update` will not be included.
Revert `get_or_insert` back as `insert_block`.

Method `update` now mutates `LocalChain` directly, instead of mutating
via a second call.

`CheckPoint::new_with_prev` is replaced with `CheckPoint::extend`.
Within `update()`, it is not always necessary to call `fix_links()`. The
logic to detect this was wrong previously.

Add test that would fail with the previous logic.
@evanlinjin
Copy link
Member

This look good, thank you for this @notmandatory. We just need to include steps for specifying the descriptor, either via DESCRIPTOR env, or via cli arg.

@notmandatory
Copy link
Member Author

Closing this one since it's a dup of #1106 .

@notmandatory notmandatory mentioned this pull request Oct 13, 2023
8 tasks
@notmandatory notmandatory deleted the pull/1002 branch May 26, 2025 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants