✅ sdaftuar closed an issue: "Syncing headers with feeler-peers"
(https://github.com/bitcoin/bitcoin/issues/16859)

🤔 darosior reviewed a pull request: "script: Add a separate ScriptError for empty pubkeys encountered in Tapscript"
(https://github.com/bitcoin/bitcoin/pull/33961#pullrequestreview-3541861752)
Concept/Approach ACK.

💬 maflcko commented on pull request "scripted-diff: Use LogInfo over LogPrintf":
(https://github.com/bitcoin/bitcoin/pull/29641#discussion_r2590403179)
It is an internal bug, so it is dead code and doesn't really matter much. I think internal bugs should be using `Assume(false)`. Logs could go with `LogError("%s", STR_INTERNAL_BUG("..."))`, but I wanted to keep the changes here minimal. Happy to go with `LogError("%s", STR_INTERNAL_BUG(` if you feel strongly.

🤔 darosior reviewed a pull request: "test: Add DERSIG unit tests to script_tests.json"
(https://github.com/bitcoin/bitcoin/pull/33973#pullrequestreview-3541895187)
Makes sense to test `DERSIG` separately from `STRICTENC` since it's the only one enabled by consensus. Concept/Approach ACK.

Maintainers could we get the CI ran here?

💬 ryanofsky commented on pull request "test: interface_ipc.py minor fixes and cleanup":
(https://github.com/bitcoin/bitcoin/pull/34003#issuecomment-3614166836)
<!-- begin push-2 -->
Updated d179f713792432c82befd39a288ab16fec13bab1 -> 6ef092c0e4343fc421c323ff09d3c791fb4bc33a ([`pr/pyipc.1`](https://github.com/ryanofsky/bitcoin/commits/pr/pyipc.1) -> [`pr/pyipc.2`](https://github.com/ryanofsky/bitcoin/commits/pr/pyipc.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/pyipc.1..pr/pyipc.2))<!-- end --> splitting into two commits

re: https://github.com/bitcoin/bitcoin/pull/34003#issuecomment-3611494379

> > Destroy calls were being made at
...

🤔 stickies-v reviewed a pull request: "Add util::Expected (std::expected)"
(https://github.com/bitcoin/bitcoin/pull/34006#pullrequestreview-3541953136)
Concept ACK. Functions with return type `std::optional<E>` returning a truthy value on failure is unintuitive, this is much better.

👍 darosior approved a pull request: "net: fix use-after-free with v2->v1 reconnection logic"
(https://github.com/bitcoin/bitcoin/pull/33956#pullrequestreview-3541954260)
utACK 167df7a98c8514da6979d45e58fcdcbd0733b8fe

🤔 mzumsande reviewed a pull request: "net: fix use-after-free with v2->v1 reconnection logic"
(https://github.com/bitcoin/bitcoin/pull/33956#pullrequestreview-3541981127)
ACK 167df7a98c8514da6979d45e58fcdcbd0733b8fe

didn't test it, but but the explanation of the bug make sense and since all the network threads have been stopped at this point anyway I can't see a downside to clearing `m_reconnections` here.

👍 maflcko approved a pull request: "test: Remove `system_tests/run_command` runtime dependencies"
(https://github.com/bitcoin/bitcoin/pull/33929#pullrequestreview-3542024077)
lgtm

💬 maflcko commented on pull request "test: Remove `system_tests/run_command` runtime dependencies":
(https://github.com/bitcoin/bitcoin/pull/33929#discussion_r2590523065)
nit: This will fail with spaces in the dir:


```
$ ./a\ space/test_bitcoin -t system_tests
Running 1 test case...
unknown location(0): fatal error: in "system_tests/run_command": subprocess::CalledProcessError: execve failed: No such file or directory (2)
test/system_tests.cpp(29): last checkpoint

*** 1 failure is detected in the test module "Bitcoin Core Test Suite"

👍 sedited approved a pull request: "script: Add a separate ScriptError for empty pubkeys encountered in Tapscript"
(https://github.com/bitcoin/bitcoin/pull/33961#pullrequestreview-3542392743)
ACK 9d5021a05bd33c73276909eec961777867ddb412

👍 andrewtoth approved a pull request: "Broadcast own transactions only via short-lived Tor or I2P connections"
(https://github.com/bitcoin/bitcoin/pull/29415#pullrequestreview-3542841874)
ACK 2d398050ee31db05e9222149b5e60572ac31d803

💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2591174137)
nit
```suggestion
const auto pushed_tx_opt{m_tx_for_private_broadcast.GetTxForNode(pfrom.GetId())};
```

💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2591173094)
nit
```suggestion
const auto opt_tx{m_tx_for_private_broadcast.PickTxForSend(node.GetId())};
```

💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2591176187)
nit
```suggestion
if (const auto num_broadcasted{m_tx_for_private_broadcast.Remove(ptx)}) {
```

⚠️ fugatecorey22 opened an issue: "Admin Dashboard"
(https://github.com/bitcoin/bitcoin/issues/34009)
npm create vite@latest myapp --template react
cd myapp
npm install
npm run dev

📝 rkrux opened a pull request: "wallet: check for `agg_pub` validity in MuSig2 signing"
(https://github.com/bitcoin/bitcoin/pull/34010)
Otherwise conversion of an invalid `agg_pub` to `XOnlyPubKey` in a later step few lines below crashes the flow.

Should fix #33999.

<!--
*** 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 improv
...

👍 sedited approved a pull request: "test: interface_ipc.py minor fixes and cleanup"
(https://github.com/bitcoin/bitcoin/pull/34003#pullrequestreview-3543353009)
ACK 6ef092c0e4343fc421c323ff09d3c791fb4bc33a

Thanks for the cleanup! Have another suggestion for saving a few lines and naming clarity:

<details>
<summary>Diff</summary>

```diff
diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py
index 804f7f8f64..f8f3a76ec7 100755
--- a/test/functional/interface_ipc.py
+++ b/test/functional/interface_ipc.py
@@ -176,2 +176 @@ class IPCInterfaceTest(BitcoinTestFramework):
- template3 = await template2.wa
...

💬 sedited commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#issuecomment-3615621559)
> What is wrong with the most obvious approach to error handling?

I am guessing you are asking why not just use exceptions? If so, I think they could be an alternative for these situations too. I'd be curious how a similar function contract could be enforced with an exception.

👍 sedited approved a pull request: "depends: Propagate native C compiler to `sqlite` package"
(https://github.com/bitcoin/bitcoin/pull/33995#pullrequestreview-3543468882)
ACK 710031ebef838d2f0a1effa19170bef7b130bbeb