💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1123812481)
I’d probably prefer using `.value()` for new code I’d write, but the asterisk-variant seems prevalent throughout this code.

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1123811383)
Yes, agreed using an optional would be more explicit, however handling the optional return value would touch a bunch of lines here, and given that the result should never be empty unless something went wrong or the function got called with an empty `txids` input, I feel it’s a bit of a cosmetic improvement here.

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1123823270)
Thanks, will consider, change but feel it’s okay at this time.

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1123754749)
I’ve removed the `reserve(…)` call here.

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1123819758)
As above with `GetIterVec()`, an empty vector is not a valid outcome for a call, so I tend to leave as is.

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1124908789)
Updated

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1125071450)
Thanks, I’m not going to interfere here, because outpoints are tiny anyway, and it seems more readable without the move or constructor.

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1124956187)
Thanks that’s much nicer

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1125071911)
I’ll have to revisit this one.

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1124966759)
It seems to me that if we don’t use the result of `find`, it’s clearer that we just care about whether a key is present instead of where in the sequence it appears

💬 Xekyo commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#issuecomment-1454249283)
Rebased on latest version of #27021

👍 yahiheb approved a pull request: "Fix typos in comments to make linter happy"
(https://github.com/bitcoin/bitcoin/pull/27197)

👍 kristapsk approved a pull request: "Fix typos in comments to make linter happy"
(https://github.com/bitcoin/bitcoin/pull/27197)
utACK 987f1bb41c0a8c54422066e10d1c63e19c4df66d

🚀 fanquake merged a pull request: "Fix typos in comments to make linter happy"
(https://github.com/bitcoin/bitcoin/pull/27197)

👍 fanquake approved a pull request: "util: add missing include and fix function signature"
(https://github.com/bitcoin/bitcoin/pull/27192)
ACK 8847ce44e0713350a6d3524f62eaeb10ba548bae

🚀 fanquake merged a pull request: "util: add missing include and fix function signature"
(https://github.com/bitcoin/bitcoin/pull/27192)

💬 fanquake commented on issue "Adoption of BIP39/44/49/84, and classification of (extended) pub/priv keys, addresses, mnemonics, etc":
(https://github.com/bitcoin/bitcoin/issues/17748#issuecomment-1454608242)
cc @achow101

💬 pablomartin4btc commented on pull request "Mask values on Transactions View":
(https://github.com/bitcoin-core/gui/pull/708#issuecomment-1454642004)
Updates:
- Cleanup from previous approach.
- Closing all dialogs with transaction details that were opened from the transactions view when mask values is selected.

💬 mauri146K commented on issue "signrawtransactionwithkey command shouldn't output the "Witness program was passed an empty witness" error for a TapRoot transaction":
(https://github.com/bitcoin/bitcoin/issues/27017#issuecomment-1454644421)
> There is the irrelevant error message output by the signrawtransactionwithkey command.
>
> **Expected behavior**
>
> Hex string of the raw transaction with signature OR meaningful message about an alternative way to achieve one.
>
> **Actual behavior**
>
> ```
> {
> "hex": "0200000001a2c0d82460883696219dbca6f545f72963b2b3ee085d832eb5ef9a69a374af160000000000fdffffff01e011000000000000225120052e44f45a6e381be8e06d3f3362b58034a68ba98081e24de7bfc5795420a90b00000000",
> "complete": false,
>
...

💬 fanquake commented on pull request "doc: Expand scantxoutset help text to cover tr() and miniscript":
(https://github.com/bitcoin/bitcoin/pull/27155#issuecomment-1454659148)
> Should we version the doc/descriptors.md file? By that I mean literally add details in the doc saying when specific formats were added.

cc @achow101