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.

Sign up for GitHub

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

Jump to bottom

Add Wallet::list_output method #1190

Merged
merged 4 commits into from
Nov 21, 2023
Merged

Add Wallet::list_output method #1190

merged 4 commits into from
Nov 21, 2023

Conversation

evanlinjin
Copy link
Member

@evanlinjin evanlinjin commented Nov 1, 2023 •
edited
Loading

Fixes #1184

Description

Introduce Wallet::list_output method that lists all outputs (both spent and unspent) in a consistent history.

Changelog notice

  • Rename LocalUtxo to LocalOutput.
  • Add Wallet::list_output method.

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

@evanlinjin evanlinjin marked this pull request as ready for review November 1, 2023 20:26
@evanlinjin evanlinjin self-assigned this Nov 1, 2023
notmandatory
notmandatory approved these changes Nov 1, 2023
View reviewed changes
Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

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

ACK 63dea46

One small NIT is I'd label 723658f as a "refactor!" even though it's only a rename:

https://softwareengineering.stackexchange.com/questions/381736/renaming-%e2%8a%88-refactoring

@nondiremanuel nondiremanuel added this to the 1.0.0-alpha.3 milestone Nov 5, 2023
@nondiremanuel nondiremanuel mentioned this pull request Nov 5, 2023
Closed
38 tasks
@evanlinjin evanlinjin force-pushed the issue/1184 branch 2 times, most recently from 2108c1d to a84446f Compare November 6, 2023 03:26
@vladimirfomene
Copy link
Contributor

ACK a84446f

danielabrozzoni
danielabrozzoni reviewed Nov 9, 2023
View reviewed changes
crates/bdk/src/wallet/mod.rs Outdated Show resolved Hide resolved
crates/bdk/tests/wallet.rs Show resolved Hide resolved
@evanlinjin evanlinjin force-pushed the issue/1184 branch 2 times, most recently from 356381b to 01037cd Compare November 14, 2023 22:34
notmandatory
notmandatory approved these changes Nov 16, 2023
View reviewed changes
Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

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

Re-ACK 01037cd

@notmandatory notmandatory added new feature New feature or request module-wallet labels Nov 16, 2023
@nondiremanuel nondiremanuel mentioned this pull request Nov 19, 2023
Closed
54 tasks
@evanlinjin
docs(bdk): clarify insert_txout docs
278210b 
Inserted txouts will not be shown in `list_unspent` or `list_output`.
notmandatory
notmandatory approved these changes Nov 20, 2023
View reviewed changes
Copy link
Member

@notmandatory notmandatory left a comment •
edited
Loading

Choose a reason for hiding this comment

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

re-ACK 278210b

New docs do a good job of clarifying for users which outputs they'll get from list_output, and which they won't from insert_txout.

danielabrozzoni
danielabrozzoni approved these changes Nov 21, 2023
View reviewed changes
Copy link
Member

@danielabrozzoni danielabrozzoni left a comment

Choose a reason for hiding this comment

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

ACK 278210b

Thanks for the docs update! Looking fantastic.

@danielabrozzoni danielabrozzoni merged commit 9e681b3 into bitcoindevkit:master Nov 21, 2023
12 checks passed
@nondiremanuel nondiremanuel mentioned this pull request Dec 2, 2023
Closed
55 tasks
@notmandatory notmandatory mentioned this pull request Jan 3, 2024
Closed
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielabrozzoni danielabrozzoni danielabrozzoni approved these changes

@notmandatory notmandatory notmandatory approved these changes

@LLFourn LLFourn Awaiting requested review from LLFourn

Assignees

@evanlinjin evanlinjin

Labels
module-wallet new feature New feature or request
Projects
BDK
Archived in project
Milestone
1.0.0-alpha.3
Development

Successfully merging this pull request may close these issues.

Add Wallet::list_outputs method.
5 participants
@evanlinjin @vladimirfomene @notmandatory @danielabrozzoni @nondiremanuel