✅ fanquake closed a pull request: "XFj65z15pTke5yJtjDB3Su4BLfKPqTBt81"
(https://github.com/bitcoin/bitcoin/pull/27184)

📝 fanquake locked a pull request: "XFj65z15pTke5yJtjDB3Su4BLfKPqTBt81"
(https://github.com/bitcoin/bitcoin/pull/27184)
<!--XFj65z15pTke5yJtjDB3Su4BLfKPqTBt81[XFj65z15pTke5yJtjDB3Su4BLfKPqTBt81](XFj65z15pTke5yJtjDB3Su4BLfKPqTBt81)
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experien
...

📝 han0147 opened a pull request: "Create master"
(https://github.com/bitcoin/bitcoin/pull/27185)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

📝 han0147 opened a pull request: "Patch 1"
(https://github.com/bitcoin/bitcoin/pull/27186)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 han0147 commented on pull request "Create master":
(https://github.com/bitcoin/bitcoin/pull/27185#issuecomment-1451312029)
j

✅ han0147 closed a pull request: "Create master"
(https://github.com/bitcoin/bitcoin/pull/27185)

📝 han0147 opened a pull request: "Create pk"
(https://github.com/bitcoin/bitcoin/pull/27187)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

👋 han0147's pull request is ready for review: "Create pk"
(https://github.com/bitcoin/bitcoin/pull/27187)

✅ fanquake closed a pull request: "Patch 1"
(https://github.com/bitcoin/bitcoin/pull/27186)

📝 fanquake locked a pull request: "Patch 1"
(https://github.com/bitcoin/bitcoin/pull/27186)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

✅ fanquake closed a pull request: "Create pk"
(https://github.com/bitcoin/bitcoin/pull/27187)

📝 fanquake locked a pull request: "Create pk"
(https://github.com/bitcoin/bitcoin/pull/27187)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

📝 fanquake locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/27185)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#issuecomment-1451488906)
Code review ACK 0bb90cabc8e2c6bbbc3c52ec1170e725435b625d modulo tests.

I believe this PR is a good improvement over status quo.

The biggest worry I have now it somewhat ambiguous terminology:
- `mixed` groups could mean mixed type or mixed effective value
- `all` groups not always refers to actually all groups, but filtered by `CoinEligibiltyFilter`
- two structs `Groups` vs `OutputGroups`

I don't have better suggestion as of now, so it's not blocking. I'll think more and come back
...

💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1121297405)
nit: While we are at it, what do you think about passing `wallet.chain()` instead of whole wallet object?

💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1121298272)
Nit: this is redundant, we shouldn't have non-spendable coins here at all. They has to be filtered in `AvailableCoins`

💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1121303672)
nit: I'd prefer just having two methods for positive only and for all groups. For me this would be cleaner code instead of controlling the flow with bool params.

💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1121328934)
The comment is wrong

💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1122739758)
nit: it'd be great if we can disambiguate the term `mixed`. Now it applies to both a) mixed type groups b) mixed effects value group

💬 S3RK commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1121336522)
Not sure why this is needed