⚠️ Sjors opened an issue: "rpc: analyzepsbt should explicitly check for an invalid taproot_key_path_sig"
(https://github.com/bitcoin/bitcoin/issues/33320)
While testing #29675 my final combined PSBT ended up having an invalid `taproot_key_path_sig`, which I found out by logging the `sigdata.complete` value at the end of `ProduceSignature`.

We should of course never produce such an invalid value and I'll continue digging for the root cause. But external applications could do this too.

Currently `analyzepsbt` will give a useless `"next": "updater"` status when this happens.

We should instead display an error.

Not sure what the best approach woul
...

💬 TheCharlatan commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2324982685)
Sorry for the churn, but I think I made a mistake here:
```diff
diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py
index a87ada4ffa..8410fa24be 100755
--- a/test/functional/interface_ipc.py
+++ b/test/functional/interface_ipc.py
@@ -149,0 +150 @@ class IPCInterfaceTest(BitcoinTestFramework):
+ self.log.debug("Wait for another, but time out")
@@ -155,2 +156 @@ class IPCInterfaceTest(BitcoinTestFramework):
- self.log.debug("Wait for anot
...

💬 Crypt-iQ commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2324984965)
> where all of our other peers don't support compact block

This, or a malicious peer somehow controls all three slots (harder since one slot is outbound), or we are in `-blocksonly` mode.

> In any case, this was non-obvious to me. Maybe add a comment in the spot where we fall back to GETDATA explaining that we don't wipe the partialBlock there on purpose?

Good idea, I will add a comment. I am not sure if the existing logic is on purpose, but the way it is written in master makes it impo
...

💬 Crypt-iQ commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2324986541)
Good point, I will preserve the logic by disconnecting the peer.

💬 ryanofsky commented on pull request "multiprocess: Don't require bitcoin -m argument when IPC options are used":
(https://github.com/bitcoin/bitcoin/pull/33229#discussion_r2324985565)
> No access to `self.logger` here?

Yeah sorry, this is just a workaround for debugging. This test is just not working on windows and I was trying to debug it using CI to avoid needing to setup a windows build and vm and see if the issue happens there.

So far I've fixed one issue where unicode path CI uses was not properly handled by `bitcoin` wrapper on windows, but there is another issue where stdout/stderr process of the `bitcoind` do not seem to be captured by python when it is invoked
...

💬 ryanofsky commented on pull request "multiprocess: Don't require bitcoin -m argument when IPC options are used":
(https://github.com/bitcoin/bitcoin/pull/33229#discussion_r2319858536)
re: https://github.com/bitcoin/bitcoin/pull/33229#discussion_r2313967295

Good catch, changed to bitcoin-qt. Also good to know text doesn't appear in gui --version output but I think that is ok as long as the test is only calling bitcoin node. I do think it's probably good for the Init::exeName method to return the right name for consistency even it's not called right now.

💬 ryanofsky commented on pull request "multiprocess: Don't require bitcoin -m argument when IPC options are used":
(https://github.com/bitcoin/bitcoin/pull/33229#discussion_r2319889144)
re: https://github.com/bitcoin/bitcoin/pull/33229#discussion_r2313941771

> [5a28d0b](https://github.com/bitcoin/bitcoin/commit/5a28d0b027fdca07c55c853a550324c0aa1c450f): did you mean to add `-ipcconnect` and `-ipcfd` here?

Yeah they are not necessary but listed to be more future-proof. It makes sense for any IPC options to choose the IPC binary.

💬 ryanofsky commented on pull request "multiprocess: Don't require bitcoin -m argument when IPC options are used":
(https://github.com/bitcoin/bitcoin/pull/33229#discussion_r2319883129)
re: https://github.com/bitcoin/bitcoin/pull/33229#discussion_r2313969321

> `bitcoin node` command?

Thanks, updated

💬 Sjors commented on issue "rpc: analyzepsbt should explicitly check for an invalid taproot_key_path_sig":
(https://github.com/bitcoin/bitcoin/issues/33320#issuecomment-3258218791)
FWIW this particular situation happened because a ledger signed one of the script paths and my WIP branch for HWI ended up putting that signature in the key path.

💬 Crypt-iQ commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2325023271)
It may be possible to call `reset()` on `partialBlock` to wipe it. I was assuming that wiping would mean calling `RemoveBlockRequest` (resets `m_downloading_since` which allows block download stalling) followed by `BlockRequested`. I will have to think more on this because I am not sure if it introduces a footgun.

👍 willcl-ark approved a pull request: "ci: Checkout latest merged pulls"
(https://github.com/bitcoin/bitcoin/pull/33303#pullrequestreview-3189268483)
ACK faeb3320952906b6147b06170059e71d7d59f4bd

This looks correct to me.

For pushes we get the default behaviour, and for pull requests it evaluates to `github.ref` (e.g., `refs/pull/<pr-num>/merge`, ensuring the checkout uses the **dynamic** merge ref for the PR.

This avoids pinning to a specific commit SHA (on re-runs) and fetches the latest merge state.

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2325049046)
Right. Fixed?

💬 TheCharlatan commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2325063247)
I think that should fix the problem, but the log line is still in the wrong place.

💬 fanquake commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2325069692)
This needs `python3-pip` in `PACKAGES`.

🤔 janb84 reviewed a pull request: "ci: reduce runner sizes on various jobs"
(https://github.com/bitcoin/bitcoin/pull/33319#pullrequestreview-3189341103)
ACK 5eeb2facbbbbf68a2c30ef9e6747e39c85d7b116

PR reduces select container sizes / cirrus runners. Not using unnecessary "large" runners is a good idea.

- code review ✅
- All CI builds run ✅

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2325096198)
Right. More fixed?

💬 fanquake commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2325112783)
and `--break-system-packages`. Probably time to roll this job back into the CI, now that we've swtiched over.

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2325115865)
Done. I was going to say, this doesn't seem to be invoked anywhere?

💬 fanquake commented on pull request "depends: disable builtin rules and suffixes.":
(https://github.com/bitcoin/bitcoin/pull/33045#issuecomment-3258373225)
> or could we use -R as well?

Pushed up a commit.

💬 stwenhao commented on issue "signet: disk-space-DoS due to low mining difficulty":
(https://github.com/bitcoin/bitcoin/issues/33266#issuecomment-3258376429)
> and would avoid this issue without needing a (signet only) (consensus) code change.

Yes, but signet-only consensus code change can be beneficial. If your difficulty requires grinding 2^20 hashes, then I think nonce values should be masked, to make sure, that upper bits are set to zero. Then, if the difficulty will increase, the code can automatically raise that barrier, so that the nonce restrictions will be active only for difficulties requiring less than 2^32 hashes.

Because if you need to
...