⚠️ hebasto opened an issue: "An overflow in `TapSatisfier::FromPKHBytes`?"
(https://github.com/bitcoin/bitcoin/issues/28871)
In the master branch @ fb85bb277670aad28fef51b7313d4a96cdaa760f, the following code in the `DecodeScript` function: https://github.com/bitcoin/bitcoin/blob/fb85bb277670aad28fef51b7313d4a96cdaa760f/src/script/miniscript.h#L2312-L2313 explicitly passes 33 bytes to the `TapSatisfier::FromPKHBytes` function, which expects 32 bytes only: https://github.com/bitcoin/bitcoin/blob/fb85bb277670aad28fef51b7313d4a96cdaa760f/src/script/sign.cpp#L295-L302

💬 sipa commented on issue "An overflow in `TapSatisfier::FromPKHBytes`?":
(https://github.com/bitcoin/bitcoin/issues/28871#issuecomment-1810116173)
Not an issue; if this runs in tapscript context, the code will bail out before reaching this line.

💬 hebasto commented on issue "An overflow in `TapSatisfier::FromPKHBytes`?":
(https://github.com/bitcoin/bitcoin/issues/28871#issuecomment-1810123165)
Right. But a compiler might be not aware of that:
```
/usr/include/c++/11/bits/stl_algobase.h:431: warning: '__builtin_memcpy' writing 33 bytes into a region of size 32 overflows the destination [-Wstringop-overflow=]
431 | __builtin_memmove(__result, __first, sizeof(_Tp) * _Num);
|
/usr/include/c++/11/bits/stl_algobase.h: In function 'DecodeScript':
script/sign.cpp:299:21: note: destination object 'pubkey' of size 32
299 | XOnlyPubKey pubkey;
|
...

🤔 vasild reviewed a pull request: "net: improves addnode / m_added_nodes logic"
(https://github.com/bitcoin/bitcoin/pull/28155#pullrequestreview-1729644861)
ACK 0420f99f429ce2382057e101859067f40de47be0

💬 vasild commented on pull request "net: improves addnode / m_added_nodes logic":
(https://github.com/bitcoin/bitcoin/pull/28155#discussion_r1392528755)
nit, naming: maybe a different name would have been better than `ConnectNodePublic()`, given that it contains extra logic on top of the private `ConnectNode()`.

💬 m3dwards commented on issue "26.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/28866#issuecomment-1810142470)
@willcl-ark thank you for your review. Addressed all items.

💬 sipa commented on pull request "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes":
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-1810204432)
Do we actually want to resolve all forms of ambiguity? E.g. what about the distinction between an OP_1 vs. a direct push of 0x01, vs. OP_PUSHDATA1 of 0x01? I think it'd be nice if there was a one-to-one correspondence between the disassembly and the actual script bytes.

My contribution to this bikeshedding would be:
* `OP_n` -> `n` (so `-1`, `0`, `1`, ..., `10`, ..., `16`).
* Drop "OP_" prefix for other opcodes too; they're redundant.
* Other *minimal* pushes (so direct pushes, as well as
...

💬 seedhammer commented on issue "MuSig2 support":
(https://github.com/bitcoin/bitcoin/issues/23326#issuecomment-1810236476)
> Beyond that, there are indeed a number of things that need to be decided:
>
> * How to integrate into descriptors. One possibility is to only support an `agg` fragment as leaf key (e.g. `agg(xpub1/.../*,xpub2/.../*)`), but it's also possible I think to support treating `agg` of xpubs as a new "xpub" itself (e.g. `agg(xpub1/...,xpub2/...)/*`) for example by defining the aggregated chaincode as a hash of the child chaincodes. This complicates signing further, however, though I don't think
...

💬 fanquake commented on issue "An overflow in `TapSatisfier::FromPKHBytes`?":
(https://github.com/bitcoin/bitcoin/issues/28871#issuecomment-1810258979)
I'm assuming this is with GCC 11 under LTO? I'd suggest using a newer, "smarter" compiler. The same warnings don't appear under GCC 13.

💬 maflcko commented on pull request "Use serialization parameters for CTransaction":
(https://github.com/bitcoin/bitcoin/pull/28438#issuecomment-1810260619)
re-ACK a0c254c13a3ef21e257cca3493446c632b636b15 🐜

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK a0c254c13a3ef21e257c
...

🤔 Sjors reviewed a pull request: "[WIP] Cluster mempool implementation"
(https://github.com/bitcoin/bitcoin/pull/28676#pullrequestreview-1729707068)
Couple more comments / questions. To be continued...

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1392567718)
54f39ca8f101483f5f82707689ca49431d4091e5: what if the deleted transaction makes it so there are now two clusters? This is also safe to ignore?

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1392646700)
54f39ca8f101483f5f82707689ca49431d4091e5: shouldn't this be `fee / size`? Or do you mean to use an inverse fee rate for performance (`inv_fee_rate`)?

💬 Sjors commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1392640692)
54f39ca8f101483f5f82707689ca49431d4091e5: So you're creating a chunk for each new transaction and then erasing it if the fee rate goes down. Why not the other way around?

✅ hebasto closed an issue: "An overflow in `TapSatisfier::FromPKHBytes`?"
(https://github.com/bitcoin/bitcoin/issues/28871)

💬 BrandonOdiwuor commented on pull request "GUI getrawtransaction implementation":
(https://github.com/bitcoin-core/gui/pull/777#issuecomment-1810358549)
> Cool, that's pretty nice!
>
> Just thinking out loud, but it does make me wonder if perhaps a new `Verify` or `Utils` button, in line with "Send | Recieve | Transactions" could be even nicer? If we added it there, we might consider adding a few more utils to the GUI like "Decode raw transaction", "Decode Script", "Get Descriptor Info", "Verify Message" etc. which folks who use the gui generally use other software for today...

@willcl-ark would love to hear more opinions on this, I also t
...

💬 willcl-ark commented on pull request "gui: getrawtransaction implementation":
(https://github.com/bitcoin-core/gui/pull/777#issuecomment-1810365085)
Yeah I don't want to derail this pr, which I think looks good on its own!

If there was desire to add more utils we could always move this later; I think this looks nice.

💬 jb55 commented on issue "macOS qt QTimer::stop crash on v26.0rc2":
(https://github.com/bitcoin/bitcoin/issues/28867#issuecomment-1810415284)
I haven't been able to reproduce it since it happened

⚠️ MarnixCroes opened an issue: "Send: ability to (re)view automatically selected coins "
(https://github.com/bitcoin-core/gui/issues/778)
### Please describe the feature you'd like to see added.

Ability to see the automatic coin selection in the GUI.

### Is your feature related to a problem, if so please describe it.

Even people who "know" how to use coin control may use the automatic coin selection with the assumption that it will select the "best" coin(s) for the tx. Especially when having many utxo's.

So user wants to use the automatic coin selection, but at the same time review the automatically selected coin(s). For exa
...

🚀 fanquake merged a pull request: "test, refactor: Magic bytes array followup"
(https://github.com/bitcoin/bitcoin/pull/28857)