💬 maflcko commented on pull request "test: modify logging_filesize_rate_limit params":
(https://github.com/bitcoin/bitcoin/pull/33211#issuecomment-3200809745)
> Small worry is by increasing the test rest window to 1h is that this will impact CI if a test will hang for some reason, it is a big increase.

The timeout is purely virtual and never waited on or synced on in real wall-clock time, so I don't think this comment applies?

💬 janb84 commented on pull request "test: modify logging_filesize_rate_limit params":
(https://github.com/bitcoin/bitcoin/pull/33211#issuecomment-3200830291)
> > Small worry is by increasing the test rest window to 1h is that this will impact CI if a test will hang for some reason, it is a big increase.
>
> The timeout is purely virtual and never waited on or synced on in real wall-clock time, so I don't think this comment applies?

> > Small worry is by increasing the test rest window to 1h is that this will impact CI if a test will hang for some reason, it is a big increase.
>
> Any test can slow down or hang for multiple reasons, that's wh
...

💬 maflcko commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2285330734)
this is a no-op, no? The `BaseException` above calls `self.log.exception`, which fully logs the exception.

Also, I don't think the logs contain "unexpected exception". In https://github.com/bitcoin/bitcoin/actions/runs/17053347607/job/48345965497#step:6:5037 the error is the stderr:

```


stderr:
[node 0] Cleaning up ipc directory '/tmp/test-ipc-xj2xyof4'

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

```
09:15:59 < sipa> anyone have any clue what's going on here? https://github.com/bitcoin/bitcoin/actions/runs/17053347607/job/48345965497?pr=33201
09:16:13 < sipa> 240/272 - interface_ipc.py failed, Duration: 6 s
09:16:18 < sipa> 2025-08-18T22:28:30.854262Z TestFramework (INFO): Tests successful
09:28:06 < instagibbs> TIL SystemExit and GeneratorExit don't inherit Exception, so maybe SystemExit is triggering, causing the failure to not be set
09:46:23 < sipa> instagibbs: let's
...

💬 ryanofsky commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3200849464)
I've summarized current state of this PR below, and think this maybe ready for merge.

### Review

- ryanofsky: current ack, low level reviews
- ismaelsadeeq: current ack, testing, and comments on documentation and related prs
- josibase: current ack, high level review, no code comments
- fanquake: low level reviews, most thorough build & CI testing, reporting and fixing openbsd & lint/tidy issues previously, and current [gcc 11 / capnproto 0.8](https://github.com/bitcoin/bitcoi
...

🤔 janb84 reviewed a pull request: "Release: Prepare "Translation string freeze" step"
(https://github.com/bitcoin/bitcoin/pull/33193#pullrequestreview-3132438847)
re ACK 0df2c3c42e8a3ee3da4ca6c4b9b9c28060bb1fe3

changes since last ACK:
- rebase (churn is inline with changes from #33209 )

💬 BrandonOdiwuor commented on pull request "Wallet: "listreceivedby*" fix":
(https://github.com/bitcoin/bitcoin/pull/30972#discussion_r2285345277)
I have updated the code to only do the second `IsMine` check if the address is not in `mapTally`

💬 BrandonOdiwuor commented on pull request "Wallet: "listreceivedby*" fix":
(https://github.com/bitcoin/bitcoin/pull/30972#issuecomment-3200864286)
Rebased and updated to address https://github.com/bitcoin/bitcoin/pull/30972#discussion_r1989789588

✅ ryanofsky closed a pull request: "build: Enable ENABLE_IPC option by default"
(https://github.com/bitcoin/bitcoin/pull/33190)

💬 ryanofsky commented on pull request "build: Enable ENABLE_IPC option by default":
(https://github.com/bitcoin/bitcoin/pull/33190#issuecomment-3200889700)
Thanks everybody who commented! Will close this in favor of #31802. If anybody thinks this should be reopened, I'd be curious and can reopen it. But balance of opinion seems to be in favor of not doing this.

💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2285441508)
IIUC it's failing because stderr is nonempty:

https://github.com/bitcoin/bitcoin/blob/22e689587a7ae02f82ad2464017731f3b23b5363/test/functional/test_runner.py#L760

because the [log](https://github.com/bitcoin/bitcoin/actions/runs/1753347607/job/48345965497) ([raw](https://productionresultssa13.blob.core.windows.net/actions-results/bfb1b205-2f78-46f1-95d9-f32ca7112d38/workflow-job-run-45b4c18f-0ff3-5fd3-8716-ea6a91129b24/logs/job/job-logs.txt?rsct=text%2Fplain&se=2025-08-19T14%3A22%3A57Z&sig
...

💬 ryanofsky commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2285457363)
If we wanted to annotate it, could write as `extra_init: list[dict[str, Any]] = ...` (with `from typing import Any`) but would also seem reasonable to suppress this with `# type: ignore[var-annotated]`

💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#issuecomment-3201001743)
Some Major Updates:
- The `SilentPaymentDescriptorScriptPubKeyMan` now obtains through a `GetScanKey` method, only available on Silent Payment descriptors
- The spend key is no longer derived during descriptor parsing; it now has to be "expanded" the same way other descriptors are expanded
- `LoadKeysAndChangeLabel` has been replaced with an overridden `TopUpWithDB` method. The wallet calls `TopUp` when it wants the SPKMans to derive and load new keys and scripts.

💬 sipa commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-3201020041)
With #33190 closed, I'll comment further here.

@achow101 @ryanofsky In my [comment](https://github.com/bitcoin/bitcoin/pull/33190#issuecomment-3196869655) in the other PR regarding seeing this PR as a successor to that one, I failed the consider the possibility of enabling it in release builds without making it default in from-source builds, like Qt, ZMQ, USDT, so I saw it as two separate sequential decisions to be made, rather than seeing them as orthogonal.

That said, I think there is a
...

💬 janb84 commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2285491902)

>
> Maybe this line should be changed to use stdout instead of stderr?
>
> https://github.com/bitcoin/bitcoin/blob/530ca0bc57477b77f60af142695d9bf200ef6e3c/test/functional/test_framework/test_node.py#L232
>
> (Sorry if this is the bug)

I Could reproduce the bug locally and this change solves it for me.

<details>

Without change :

```shell
Remaining jobs: [interface_ipc.py]
1/1 - interface_ipc.py failed, Duration: 4 s

stdout:
2025-08-19T14:35:47.614991Z TestFramework (I
...

💬 hodlinator commented on pull request "build: Enable -DCMAKE_EXPORT_COMPILE_COMMANDS=1":
(https://github.com/bitcoin/bitcoin/pull/33187#issuecomment-3201147329)
### Concept

Pretty sure this not being enabled has wasted my time (even though I mostly worked on PRs which didn't include the #33101 change). I assumed it was something broken with my LSP plugin, and was meaning to look into it eventually, getting by through grepping files.

Having this on by default saves many developers from spending time on figuring this aspect out. As shown by the author, changing the default is commonplace among other projects: https://github.com/bitcoin/bitcoin/pull/
...

💬 marcofleon commented on pull request "headerssync: Preempt unrealistic unit test behavior":
(https://github.com/bitcoin/bitcoin/pull/32579#issuecomment-3201214771)
Re ACK 53341ea10dc2f7df371b416060863bbc094b8773

Breaking up the three scenarios into their own test cases makes sense. I checked to make sure the test logic is unchanged.

💬 hebasto commented on pull request "build: Enable -DCMAKE_EXPORT_COMPILE_COMMANDS=1":
(https://github.com/bitcoin/bitcoin/pull/33187#issuecomment-3201231535)
> Having this on by default...

Why doesn’t `export CMAKE_EXPORT_COMPILE_COMMANDS=ON` in ~/.profile (or something similar) work for you?

💬 theuni commented on pull request "guix: build for Linux HOSTS with `-static-libgcc`":
(https://github.com/bitcoin/bitcoin/pull/33181#issuecomment-3201253439)
Concept ACK.

But note that this may require some care when we start shipping the kernel, as it likely needs to be built against a shared libgcc (and maybe libstdc++, depending on what our api/abi looks like at that point).

Maybe add a note so we don't forget?

💬 sipa commented on pull request "build: Enable -DCMAKE_EXPORT_COMPILE_COMMANDS=1":
(https://github.com/bitcoin/bitcoin/pull/33187#issuecomment-3201255500)
We generally don't include editor-specific configuration, as it gets unclear what the bar for relevance is, and easily gets unmaintainable. For example, `.gitignore` also doesn't include such things either (see #32417).

That doesn't mean that we can't have some documentation about integration in specific systems.

💬 purpleKarrot commented on pull request "build: Enable -DCMAKE_EXPORT_COMPILE_COMMANDS=1":
(https://github.com/bitcoin/bitcoin/pull/33187#issuecomment-3201268524)
> A different approach that might be more palatable would be to only enable `CMAKE_EXPORT_COMPILE_COMMANDS` in the dev-mode preset in _CMakePresets.json_.

I would object much less than setting this in `CMakeLists.txt`.