💬 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

💬 instagibbs commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2325208508)
IIUC block requests are cleaned up during disconnect, so this would allow noban/manual connections to re-start trying to give us blocks vs freeze them out?

💬 hebasto commented on pull request "stabilize translations by reverting old ids by text content":
(https://github.com/bitcoin/bitcoin/pull/33270#issuecomment-3258498758)
> ... can you please point me to the discussion that you were referring to with "reintroducing Python scripts"...

The Python dependency for the `translate` build target was recently removed in https://github.com/bitcoin/bitcoin/pull/33209 by @purpleKarrot.

👍 ryanofsky approved a pull request: "Add functional test for IPC interface"
(https://github.com/bitcoin/bitcoin/pull/33201#pullrequestreview-3189572477)
Code review ACK a341e11ac92b30aac856049c696a9ac39854569d. Changes since last review: rebasing, switching to miniwallet and expanding wallet test, improving pycapnp install steps in instructions and CI.

Looking at CI changes here, it's interesting that low-level way we configure CI jobs allows dependencies to be installed that might not be used, and things to be built that are not tested. I think a more ideal config / script boundary might make the config only responsible for specifying what s
...

🤔 stickies-v reviewed a pull request: "wallet: reduce unconditional logging during load"
(https://github.com/bitcoin/bitcoin/pull/33299#pullrequestreview-3189575123)
Concept ACK, but I think unconditional logging in the ctor is not the right approach, suggested alternative:

<details>
<summary>git diff on 689a321976</summary>

```diff
diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp
index 79e2c9688d..11d7c9029d 100644
--- a/src/wallet/init.cpp
+++ b/src/wallet/init.cpp
@@ -19,6 +19,7 @@
#include <util/moneystr.h>
#include <util/translation.h>
#include <wallet/coincontrol.h>
+#include <wallet/sqlite.h>
#include <wallet/wallet.h>
#in
...

💬 instagibbs commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#discussion_r2325239854)
How about something like:

// We keep the failed partialBlock to disallow processing another compact block announcement from the same
// peer for the same block. We let the full block download below continue under the same m_downloading_since timer.

💬 hebasto commented on pull request "doc: fix `LIBRARY_PATH` comment":
(https://github.com/bitcoin/bitcoin/pull/33308#discussion_r2325242873)
> I think it's clear it applies to both, given it's the same code?

Yes, that’s certainly one way to look at it. I’m willing to admit it might just be my own bias to think that an inline comment applies only to the specific line it’s attached to.

That could explain why I (mistakenly) removed the comment along with the code in https://github.com/bitcoin/bitcoin/pull/32764.