💬 MirekLabada commented on pull request "222":
(https://github.com/bitcoin/bitcoin/pull/33496#issuecomment-3347144943)
? fix this please

📝 MirekLabada opened a pull request: "?"
(https://github.com/bitcoin/bitcoin/pull/33497)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 ryanofsky commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3347241863)
> Err, right. Updated the description with your suggested trace filtering.

Thanks created https://github.com/bitcoin-core/libmultiprocess/issues/219 to track this (feel free to create new issues directly in the future). Thanks for updating the rust client, too!

💬 hebasto commented on pull request "build: Move CMAKE_SKIP_INSTALL_RPATH from CMake to Guix script":
(https://github.com/bitcoin/bitcoin/pull/33470#discussion_r2388170464)
> Can this just be `CMAKE_SKIP_RPATH`?

With this suggestion having been adopted, the following lines of code can be dropped as well: https://github.com/bitcoin/bitcoin/blob/dd5c517757f97b68f7eb07628222c958b47f742b/CMakeLists.txt#L632 and https://github.com/bitcoin/bitcoin/blob/d8fe258cd6105704bf4427eda048dd7085ca516d/src/CMakeLists.txt#L420

I understand that this overlaps with https://github.com/bitcoin/bitcoin/pull/33247, so it's OK to leave this branch as is.

👍 hebasto approved a pull request: "build: Move CMAKE_SKIP_INSTALL_RPATH from CMake to Guix script"
(https://github.com/bitcoin/bitcoin/pull/33470#pullrequestreview-3280122695)
ACK dd5c517757f97b68f7eb07628222c958b47f742b, I have reviewed the code and it looks OK.

💬 hebasto commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3347322127)
> For me, the checksum is the same as in the PR:
>
> ```shell
> $ wget -q -O - https://github.com/fukuchi/libqrencode/archive/refs/tags/v4.1.1.tar.gz |sha256
> 5385bc1b8c2f20f3b91d258bf8ccc8cf62023935df2d2676b5b67049f31a049c
> $
> ```
>
> but CI:
>
> ```
> sha256sum: WARNING: 1 computed checksum did NOT match
> /home/admin/actions-runner/_work/_temp/depends/sources/qrencode-4.1.1.tar.gz: FAILED
> ```

I believe it's a cache issue.

@willcl-ark @m3dwards @maflcko

Can the d
...

💬 maflcko commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3347363844)
> Can the dependency CI cache be invalidated: (1) for this PR; (2) globally?

I wouldn't know how, other than by changing the cache key value (both for restore and save): https://github.com/bitcoin/bitcoin/blob/d8fe258cd6105704bf4427eda048dd7085ca516d/.github/actions/restore-caches/action.yml#L21

💬 hebasto commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3347417118)
> I believe it's a cache issue.

Resolved by using the new `$(package)_file_name`.

💬 m3dwards commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3347455469)
> Can the `depends` CI cache be invalidated: (1) for this PR; (2) globally?

I would have thought the cache should have been invalidated automatically, looking into it.

💬 kevkevinpal commented on pull request "ci: check if file or directory already exists for ${HOME}/.bitcoin instead of failing":
(https://github.com/bitcoin/bitcoin/pull/33486#issuecomment-3347489891)
> I don't think it is supported or even encouraged to run this internal CI script on an outside machine.

Yea I understand this, I was just playing around with the scripts to understand better how the CI works

> Also, I fail to see how this could even work, as you did not include any steps how to reproduce.

Yes, I updated the description. Basically, just create a directory in `$HOME/.bitcoin` and then try to run `./ci/test/03_test_script.sh` and you should see the error.

For me it is
...

💬 kevkevinpal commented on pull request "ci: check if file or directory already exists for ${HOME}/.bitcoin instead of failing":
(https://github.com/bitcoin/bitcoin/pull/33486#issuecomment-3347493339)
closing as it doesn't fix any currently existing issue

✅ kevkevinpal closed a pull request: "ci: check if file or directory already exists for ${HOME}/.bitcoin instead of failing"
(https://github.com/bitcoin/bitcoin/pull/33486)

💬 plebhash commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3347520481)
@ryanofsky I don't really used `gdb` nor `lldb` on a regular basis, so I apologize if I did anything wrong. I hope the logs below are giving you the right info.

Here's the sequence of steps I took:
- do `lldb -- ./build/bin/bitcoin-node -testnet4 -ipcbind=unix`
- inside `lldb` terminal, do `process handle -p true -s false -n false SIGINT` so that the `SIGINT` goes to the process and not to `lldb`
- inside `lldb` terminal, do `run`
- launch my (now outdated) Rust code that does `waitTipChanged`

...

👍 stickies-v approved a pull request: "ci: Drop support for EOL macOS 13"
(https://github.com/bitcoin/bitcoin/pull/33489#pullrequestreview-3280376156)
ACK fab9ddaf554837624fa8f388e046a30d5bf7626f

macOS 13 is EOL, and the minimum Xcode version that ships the documented minimum clang-16 is Xcode 16.

💬 stickies-v commented on pull request "ci: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2388333312)
For my understanding: because Xcode 15 supports macOS 14 this change is unrelated to dropping macOS 13 support, and instead could/should have been part of #30263 since Xcode 15 doesn't satisfy our minimum clang version anymore, right? If that's correct, it looks like there's lingering usage of Xcode 15 that can be bumped up, but orthogonal to this PR.

👍 stickies-v approved a pull request: "ci: use latest versions of lint deps"
(https://github.com/bitcoin/bitcoin/pull/33487#pullrequestreview-3280415193)
utACK aeab2c7f1d151beda3eff94fb4effc85a4d4a344

📝 naiyoma opened a pull request: "p2p: Mitigate GETADDR fingerprinting by setting address timestamps to a fixed value"
(https://github.com/bitcoin/bitcoin/pull/33498)
TLDR: When a node is connected to multiple networks (e.g., clearnet and Tor), it keeps separate ADDR response caches for each network. These are refreshed about once per day (randomized between 21 and 27 hours). See →[ https://github.com/bitcoin/bitcoin/blob/master/src/net.cpp#L3519](https://github.com/bitcoin/bitcoin/blob/master/src/net.cpp#L3519)
This is an example of a dual-homed node’s addrman response, with separate caches:

_IPv4 response:_
```
{102.130.242.11:8333 : 1741100851}
{119
...

👍 ryanofsky approved a pull request: "rpc: Handle -named argument parsing where '=' character is used"
(https://github.com/bitcoin/bitcoin/pull/32821#pullrequestreview-3280437895)
Code review ACK 24391ed7804a724a62034b21150c89e45ac9b625. Just rebased after #33230

💬 ryanofsky commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2388373555)
In commit "rpc: Handle -named argument parsing where '=' character is used" (04e78595410c557c970eb3994a40586231d5b38e)

This new paragraph is nice, but I think it would make sense to move it earlier in the description so JSON parameters (which are more important to list) are covered first, and STRING parameters (which are less important to list) are covered second). This should also be clearer than talking about JSON parameters, then string parameters, then JSON parameters again. Would suggest
...

💬 ryanofsky commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2388389106)
In commit "test: Add functional tests for named argument parsing" (24391ed7804a724a62034b21150c89e45ac9b625)

It seems like this is getting pretty repetitive. Maybe just match once with a `(STRING|JSON_OR_STRING|JSON)` pattern?