Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve txout listing and balance APIs for redesigned structures #975

Merged
merged 4 commits into from
May 11, 2023

Conversation

evanlinjin
Copy link
Member

@evanlinjin evanlinjin commented May 10, 2023

Description

As noted in #971 (comment).

Instead of relying on a OwnedIndexer trait to filter for relevant txouts, we move the listing and balance methods from IndexedTxGraph to TxGraph and add an additional input (list of relevant outpoints) to these methods.

This provides a simpler implementation and a more flexible API.

Other Fixes

The Persist::commit method is fixed in 96b1075.

Previously, regardless of whether writing to persistence backend is successful or not, the logic always cleared self.staged. This is changed to only clear self.staged after successful write.

Additionally, the written changeset (if any) is returned, and PersistBackend::write_changes will not be called if self.staged is empty.

Notes to the reviewers

Yes, slightly more boilerplate to do the same things, but less code to maintain and a much more flexible API. Very worth it IMO.

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

Instead of relying on a `OwnedIndexer` trait to filter for relevant
txouts, we move the listing and balance methods from `IndexedTxGraph` to
`TxGraph` and add an additional input (list of relevant outpoints) to
these methods.

This provides a simpler implementation and a more flexible API.
crates/chain/src/indexed_tx_graph.rs Show resolved Hide resolved
crates/chain/src/tx_graph.rs Outdated Show resolved Hide resolved
crates/chain/src/tx_graph.rs Outdated Show resolved Hide resolved
Previously, regardless of whether writing to persistence backend is
successful or not, the logic always cleared `self.staged`. This is
changed to only clear `self.staged` after successful write.

Additionally, the written changeset (if any) is returned, and
`PersistBackend::write_changes` will not be called if `self.staged` is
empty.
Rename the `S` trait bound to `OI` (outpoint index) to emphasize it's
not only for spk indexing.
@evanlinjin evanlinjin requested a review from LLFourn May 11, 2023 04:46
Copy link
Contributor

@LLFourn LLFourn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is better. Looking forward to seeing how this works in the examples.

@LLFourn
Copy link
Contributor

LLFourn commented May 11, 2023

ACK ed89de7

@evanlinjin evanlinjin merged commit 34d6087 into bitcoindevkit:master May 11, 2023
@notmandatory notmandatory added this to the 1.0.0-alpha.1 milestone May 15, 2023
@notmandatory notmandatory mentioned this pull request Jul 4, 2023
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants