💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2324877876)
Switched to it in docs and macOS CI.

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2324878217)
@TheCharlatan Thanks, included the MiniWallet approach!

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2324878775)
Resolving, thanks to MiniWallet-based test.

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3258111873)
Strange error, https://github.com/bitcoin/bitcoin/actions/runs/17491932432/job/49683489249?pr=33201:

```
node0 stderr bitcoin-node: ipc/libmultiprocess/include/mp/proxy-io.h:279: void mp::Waiter::post(Fn &&) [Fn = (lambda at
./ipc/libmultiprocess/include/mp/type-context.h:67:19)]: Assertion `!m_fn' failed.
```

in line

```python
template4 = await template2.result.waitNext(ctx, waitoptions)
```

Any idea, @ryanofsky?

💬 Sjors commented on issue "`combinepsbt` RPC does not work with P2TR inputs":
(https://github.com/bitcoin/bitcoin/issues/27219#issuecomment-3258138039)
@achow101 can be closed?

💬 husaria-labs commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-3258161475)
All it will take is someone to put a binary of child porn jpeg and anyone running nodes will be doing it illegaly in most countries.

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3258166117)
Debugged on IRC:

```
07:50:56 < sipa> ryanofsky: any idea about this error in https://github.com/bitcoin/bitcoin/actions/runs/17491932432/job/49683489249?pr=33201 :
07:51:08 < sipa> node0 stderr bitcoin-node: ipc/libmultiprocess/include/mp/proxy-io.h:279: void mp::Waiter::post(Fn &&) [Fn = (lambda at
./ipc/libmultiprocess/include/mp/type-context.h:67:19)]: Assertion `!m_fn' failed.
07:55:12 < kevkevin> looks like the test got hung up on this line in the ipc test: templ
...

📝 willcl-ark opened a pull request: "ci: reduce runner sizes on various jobs"
(https://github.com/bitcoin/bitcoin/pull/33319)
These jobs can likely use reduced runner sizes to avoid wasting our CPU quota, as much of the long-running part of the job is single-threaded.

This will also give us more (job) parallelisem from the same number of CPU that we are using.

Suggested in: https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2321775620

💬 0xB10C commented on issue "signet: disk-space-DoS due to low mining difficulty":
(https://github.com/bitcoin/bitcoin/issues/33266#issuecomment-3258202695)
My impression is that just increasing the difficulty on a public Signet should be enough and would avoid this issue without needing a (signet only) (consensus) code change. It doesn't even need to be difficulty 1 (i.e. 2^32 work) - similar to the default signet only requiring 5% of work at the risk of 19 alternative blocks per tip. The attack is easily detectable and if exploited over a long period of time, the signet admin can further increase the difficulty at any time. Alternatively, publishi
...

⚠️ 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.