💬 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
...

💬 inkhaton commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-3258382540)
IT doesnt have to be some imaginary illegal number. Just the binary of a jpg thats child porn. With that every person running a node in most countries can be arrested.

🤔 janb84 reviewed a pull request: "ci: Checkout latest merged pulls"
(https://github.com/bitcoin/bitcoin/pull/33303#pullrequestreview-3189412062)
re ACK faeb3320952906b6147b06170059e71d7d59f4bd

Looks good !

changes sinds last ACK:
- using YAML_KEYS for templating with descriptive naming (moved from env)

💬 yuvicc commented on issue "InitError() doesn't always halt node startup when blockchain state exists":
(https://github.com/bitcoin/bitcoin/issues/33276#issuecomment-3258423195)
I dug into this issue and found that this behavior lies with the `-stopatheight` argument. This argument only stops validation of blocks until that height but doesn't stop further downloading of blocks. That might be the reason we see that it validates the remaining blocks from the index and updates the current height next time when it starts.


Running the same command `build/bin/bitcoind -datadir=demo -stopatheight=1000`, I see it has blocks till 1520 in the db.
```
2025-09-05T13:32:14Z Loadin
...

💬 Crypt-iQ commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2325190068)
Comment added in 7c1b388c7ab2059b562c89b1e7c7dd5b641a9cb8

💬 Crypt-iQ commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2325190937)
Added this in 7c1b388c7ab2059b562c89b1e7c7dd5b641a9cb8