josie
josie
Concept ACK This is coming along nicely! I do have a few suggestions: I think the correct place to do the filtering is in `AvailableCoins`. This would make it more...
updated `IsConfSupported` to be pass by reference, per @luke-jr 's feedback; `git range-diff master 019e02c deba6fe`
big Concept ACK Still in the process of reviewing this, but it looks good so far. Building off of this PR in #28560
ACK https://github.com/bitcoin/bitcoin/commit/0295b44c257fe23f1ad344aff11372d67864f0db Thanks for taking the suggestions! Looks like the CI failure is a transient failure, I ran the same test locally and it passed. Probably just needs to be...
> Still not a huge fan of https://github.com/bitcoin/bitcoin/commit/596642c5a9f52dda2599b0bde424366bb22b3c6e and I have same concerns as josibake My concerns were addressed by https://github.com/bitcoin/bitcoin/pull/25273#discussion_r1420969629. I think the approach in https://github.com/bitcoin/bitcoin/commit/596642c5a9f52dda2599b0bde424366bb22b3c6e is much simpler...
For the reviewers of this PR, https://github.com/bitcoin/bitcoin/pull/28560 is a follow-up that further refactors `FundTransaction` by cleaning up the SFFO parsing and removing the SFFO logic from `FundTransaction`. It does this...
I'll echo @real-or-random 's point and add: > and then we recommend users that use huge list of pub keys to sort them before passing them in with the stdlib...
> Dropped the signing changes as they were causing issues with being able to sign transactions that we have the keys for but not the descriptors. https://github.com/bitcoin/bitcoin/pull/23417 would allow us...
ACK https://github.com/bitcoin/bitcoin/commit/46b3c569cfd01a45e5d9cf9428a07b06652e3df9 Looks good, thanks for taking the suggestions! Having the callbacks seems like a really nice feature: based on one of your review comments, I took a stab at...