Skip to content

Add TxBuilder step 1: CoinControl#1004

Closed
danielabrozzoni wants to merge 1 commit intobitcoindevkit:masterfrom
danielabrozzoni:tx_builder_design
Closed

Add TxBuilder step 1: CoinControl#1004
danielabrozzoni wants to merge 1 commit intobitcoindevkit:masterfrom
danielabrozzoni:tx_builder_design

Conversation

@danielabrozzoni
Copy link
Copy Markdown
Member

@danielabrozzoni danielabrozzoni commented Jun 5, 2023

Description

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

@danielabrozzoni
Copy link
Copy Markdown
Member Author

danielabrozzoni commented Jun 5, 2023

Replying to @evanlinjin's comment https://gist.github.com/evanlinjin/41e6a23cc08e888884bded327e290866?permalink_comment_id=4587231#gistcomment-4587231 here:

  • Let's make a PR soon! It's easier to discuss that way (like on the PR).
  • Let's disallow duplicate UTXOs. Need some sort of filter HashSet or BTreeSet.
  • It will be better to use utxos: impl IntoIterator so the caller won't need to .collect first if their UTXOs is not a Vec.

ACK

  • add_planned_utxos() essentially forces the caller to group UTXOs using the same descriptor + assets together first. Would this be a problem?

I don't think it is, especially since I have a new list_unspent_by_keychain method in the wallet. But if you see any better method to do this, lmk!

Thoughts on spec-writing in general

Agreed, I'll try to apply those heuristics to the txbuilder docs, thanks!

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.

1 participant