🤔 marcofleon reviewed a pull request: "fuzz: bound some miniscript operations to avoid fuzz timeouts"
(https://github.com/bitcoin/bitcoin/pull/30197#pullrequestreview-2177671042)
Code review ACK bc34bc288824978ef4b98e8802b47cb863c8a8c2. The added comments are useful, thanks for those. Tested on the three inputs in https://github.com/bitcoin/bitcoin/issues/28812 that caused the timeouts.

🚀 fanquake merged a pull request: "fuzz: bound some miniscript operations to avoid fuzz timeouts"
(https://github.com/bitcoin/bitcoin/pull/30197)

👍 hebasto approved a pull request: "fix: rendering an amp characters in the wallet name for QMenu"
(https://github.com/bitcoin-core/gui/pull/828#pullrequestreview-2177678028)
re-ACK 7581817826e000c4f0a505e7f2785a611cac8f55

💬 hebasto commented on pull request "fix: rendering an amp characters in the wallet name for QMenu":
(https://github.com/bitcoin-core/gui/pull/828#discussion_r1677806107)
Maybe
```suggestion
// The single ampersand in the menu item's text sets a shortcut for this item.
```
?

💬 stratospher commented on pull request "Add fuzz test for FSChaCha20Poly1305, AEADChacha20Poly1305":
(https://github.com/bitcoin/bitcoin/pull/28263#discussion_r1677813086)
done.

🤔 glozow reviewed a pull request: "net: fix race condition in self-connect detection"
(https://github.com/bitcoin/bitcoin/pull/30394#pullrequestreview-2177691583)
ACK 16bd283b3ad05daa41259a062aee0fc05b463fa6

💬 pablomartin4btc commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-2228495226)
> For instance, the tests that pass a wallet CLI option to a non-wallet CLI command (-generate) seem to be mixing issues under test...
>
> ```diff
> - # Single or Multi wallet modes don't matter here as -generate command validation is performed before we send to RPC
> - # Note also that -rpcwallet arg intentionally adds some confusion but could be any arg starting with "-"
> - self.log.info('Test -generate with nblocks and -rpcwallet afterwards')
> -
...

💬 stratospher commented on pull request "test: Fix intermittent failure in p2p_v2_misbehaving.py":
(https://github.com/bitcoin/bitcoin/pull/30420#discussion_r1677835214)
done.

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2228520507)
I incorporated the interface changes from #30440. In particular I made sure we only call `getBlock()` in the context of a `RequestTransactionData` message. The avoids sending the full serialized block over the interface as much as possible.

💬 stratospher commented on pull request "test: Fix intermittent failure in p2p_v2_misbehaving.py":
(https://github.com/bitcoin/bitcoin/pull/30420#discussion_r1677845216)
if the test is run in a loop, sometimes `"socket no message in first %I seconds"` is displayed and sometimes `"V2 handshake timeout"` is displayed.

[if `sendSet` is not set, "socket no message in first %i seconds"](https://github.com/bitcoin/bitcoin/blob/262260ce1e919613ba60194a5861b0b0a51dfe01/src/net.cpp#L1970) is displayed since that check is above [the check for the handshake timeout](https://github.com/bitcoin/bitcoin/blob/262260ce1e919613ba60194a5861b0b0a51dfe01/src/net.cpp#L1985) in `I
...

💬 instagibbs commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2228528366)
was chatting offline and was reminded of why I landed on a witness program over doing script trickery like:

`OP_DEPTH OP_NOT`

the issue here is that a miner could insert a `OP_NOP` onto the scriptSig, resulting in a different valid txid, breaking presigned transaction chains. Seems to suggest you really do need segwit here.

💬 ryanofsky commented on pull request "refactor: add coinbase constraints to BlockAssembler::Options":
(https://github.com/bitcoin/bitcoin/pull/30356#issuecomment-2228550103)
Code review ACK e67e4669a9e263d90f0e276c8e65e93c39170375, but I think the current implementation is too complicated and has too many layers of indirection.

Would suggest applying the following patch and squashing the PR down to a single commit which would be just +41/-16 with the suggested changes:

<details><summary>diff</summary>
<p>

```diff
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -313,7 +313,6 @@ BITCOIN_CORE_H = \
util/hasher.h \
util/insert.h \
util/macros.h \
...

💬 maflcko commented on pull request "test: Fix intermittent failure in p2p_v2_misbehaving.py":
(https://github.com/bitcoin/bitcoin/pull/30420#discussion_r1677866036)
> if the test is run in a loop, sometimes `"socket no message in first %I seconds"` is displayed and sometimes `"V2 handshake timeout"` is displayed.

I couldn't reproduce this locally. Also, it seems odd that one member field of a peer affects the field of another peer.

💬 stratospher commented on pull request "test: Fix intermittent failure in p2p_v2_misbehaving.py":
(https://github.com/bitcoin/bitcoin/pull/30420#discussion_r1677876281)
here's a failure I get when removing `peer2`:

```
$ test/functional/p2p_v2_misbehaving.py
2024-07-15T13:53:35.169000Z TestFramework (INFO): PRNG seed is: 5439305671400460804
2024-07-15T13:53:35.169000Z TestFramework (INFO): Initializing test directory /var/folders/23/1bj9v8kx4pjdz4nm49dhx2440000gn/T/bitcoin_func_test_72i8znef
2024-07-15T13:53:35.444000Z TestFramework (INFO): Sending ellswift bytes in parts to ensure that response from responder is received only when
2024-07-15T13:53:35.
...

💬 brunoerg commented on pull request "net: call `Select` with reachable networks in `ThreadOpenConnections`":
(https://github.com/bitcoin/bitcoin/pull/29436#issuecomment-2228574679)
Rebased

💬 hebasto commented on pull request "Enable customisation of QR Code font":
(https://github.com/bitcoin-core/gui/pull/820#issuecomment-2228582318)
Concept ACK on adding a new customization option for the QR code font.

Lightly tested b14c9d0572eeae0fa286e0d2b96474a6f587b18e. It seems to work as expected.

However, I think that QR image widget must consider the font size for adjusting its own size. Otherwise, the image can be unreadable and confusing for the user:

The second commit modifies the code introduced in the first one. Therefore, pleas
...

💬 stratospher commented on pull request "test: Fix intermittent failure in p2p_v2_misbehaving.py":
(https://github.com/bitcoin/bitcoin/pull/30420#discussion_r1677885509)
>Also, it seems odd that one member field of a peer affects the field of another peer.

no, it doesn't! I just want some way to wait some time so that `sendSet` for the same peer can get set. it will eventually get set but not immediately.

we could replace:
```
peer2 = node0.add_p2p_connection(P2PInterface())
assert peer2.is_connected
```

with

```
import time; time.sleep(3)
```

and it would do!

💬 hebasto commented on pull request "Bugfix - don't allow multiple dialogs for same tx in TransactionView":
(https://github.com/bitcoin-core/gui/pull/817#issuecomment-2228600085)
> Currently a user can open unlimited tx details dialogs for the same tx id.

Concept ACK. Such a behavior can confuses the user.

💬 maflcko commented on pull request "test: Fix intermittent failure in p2p_v2_misbehaving.py":
(https://github.com/bitcoin/bitcoin/pull/30420#discussion_r1677898327)
Ok, I see. Would an alternative be to wait until the RPC's peer info `bytessent` or `bytesrecv` increased after the `send_raw_message`?

Alternatively, I presume the reason that `sendSet` is set is because `send_raw_message` is executed. So the comment could simply say the this is to ensure the message sent is actually received.

Anything is fine tough. I see now why it is needed.

💬 maflcko commented on pull request "test: Fix intermittent failure in p2p_v2_misbehaving.py":
(https://github.com/bitcoin/bitcoin/pull/30420#issuecomment-2228609082)
ACK 5a6bb243adb705ac876ef1efd58a4768fa4ed033