💬 Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#issuecomment-3200698339)
> @Eunovo Nice work on addressing the issues identified in previous tests. All of those are working as expected now.
>
> I am including a patch that adds 2 more failing tests when adding silent-payments address in the transaction output: self.test_sendrawtransaction() self.test_mixed_output_types()
>
> [test_sp_receive.patch](https://github.com/user-attachments/files/21834964/test_sp_receive.patch)

I think these might be more appropriate for the sending PR https://github.com/bitcoin/bit
...

💬 hebasto commented on pull request "Release: Prepare "Translation string freeze" step":
(https://github.com/bitcoin/bitcoin/pull/33193#issuecomment-3200791226)
Rebased.

💬 fanquake commented on pull request "threading: remove ancient CRITICAL_SECTION macros":
(https://github.com/bitcoin/bitcoin/pull/32592#issuecomment-3200797532)
> This is tagged for 30.0, but feature freeze is in less than two weeks, and it still needs rebase, so it'll likely miss the milestone.

Feature freeze doesn't seem important here, given this isn't a feature; it's part of a cleanup of which the prior PRs have already landed, so landing this any time before branch off seems fine (assuming it gets rebased, have pinged @theuni).

💬 hebasto commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2285302380)
Translations followup in https://github.com/bitcoin/bitcoin/pull/33193.

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