💬 luke-jr commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2390361071)
Seems like this ought to check if the peer accepted it, and if not, return the rejection reason.

💬 maflcko commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#discussion_r2390376092)
> Upgrading Xcode 15.4 on macOS 14.4 requires updating macOS itself, at least to 14.5.

Correct. This should be no problem, because running on insecure minor releases is no different from running on EOL versions. See also the auto-upgrade: https://support.apple.com/guide/mac-help/software-update-settings-on-mac-mchla7037245/15.0/mac/15.0

> Therefore, the effective minimum supported macOS version becomes 14.5, not 14.0.

Correct. However, I don't think it is beneficial to micro-manage the
...

💬 pejfat08-rgb commented on issue "Inconsistent CJDNS address handling in Local addresses and AddLocal logs":
(https://github.com/bitcoin/bitcoin/issues/33471#issuecomment-3350965244)
- [ ]

📝 willcl-ark opened a pull request: "ci: fix buildx gha cache authentication on forks"
(https://github.com/bitcoin/bitcoin/pull/33508)
When using `docker buildx build` in conjunction with the `gha` backend cache type (as we do in our CI) it's important to specify the URL and TOKEN needed to authenticate.

On Cirrus runners this is working with only `ACTIONS_CACHE_URL` and `ACTIONS_RUNTIME_TOKEN`, but this is not enough for the GitHub backend.

Fix this by exporting all `ACTIONS_*` variables.

This fixes docker build layer cache restore/save on forks or where GH-hosted runners are being used, and addresses https://github.c
...

💬 willcl-ark commented on pull request "ci: fix buildx gha cache authentication on forks":
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3351249496)
Fork run ongoing at: https://github.com/willcl-ark/bitcoin/actions/runs/18126864863

💬 willcl-ark commented on issue "Revisiting us self-hosting parts of our CI":
(https://github.com/bitcoin/bitcoin/issues/31965#issuecomment-3351258011)
> > Yes, I've seen GHA caching work, will try and find logs.
>
> For reference, this still fails for me with `#4 ERROR: failed to parse error response 400: <h2>Our services aren't available right now</h2><p>We're working to restore all services as soon as possible. Please check back soon.</p>0eZvSaAAAAAClNmDP6CfYS7fcNJbDhpBqRE0yRURHRTA1MTYARWRnZQ==: invalid character '<' looking for beginning of value`

This did indeed require a fix, opened #33508 to address.

💬 willcl-ark commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3351288235)
> > 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.

So what has happened here is that the depends caches have missed on an exact cache hit, but restored the latest backup hit via:

https://github.com/bitcoin/bitcoin/blob/25212dfdb4cd7291392b6a94130f658c5bfa0a48/.github/actions/restore-caches/action.yml#L29-L31

It was my understanding that `make -C depends` was able
...

💬 hebasto commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#issuecomment-3351338626)
The feedback from @luke-jr and @willcl-ark have been addressed.

The PR description has been updated.

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

💬 hebasto commented on pull request "depends: Update URL for `qrencode` package source tarball":
(https://github.com/bitcoin/bitcoin/pull/33494#discussion_r2390916307)
Reworked.

👍 stickies-v approved a pull request: "wallet: reduce unconditional logging during load"
(https://github.com/bitcoin/bitcoin/pull/33299#pullrequestreview-3284021822)
ACK fc861332b351c9390400054ff74193ce26eb0713

nit: Would still prefer doing the logging from `WalletInit::Construct`, `VerifyWallets` feels out of place to me, but no biggie.

<details>
<summary>git diff on fc861332b3</summary>

```diff
diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp
index 79e2c9688d..4013e12967 100644
--- a/src/wallet/init.cpp
+++ b/src/wallet/init.cpp
@@ -20,6 +20,7 @@
#include <util/translation.h>
#include <wallet/coincontrol.h>
#include <wallet/walle
...

💬 stickies-v commented on pull request "wallet: reduce unconditional logging during load":
(https://github.com/bitcoin/bitcoin/pull/33299#discussion_r2390924243)
nit: docstring doesn't correspond to what's actually being printed. To avoid it going stale quickly, how about a simple:

> // Logs information about the database(s) used by the wallet.

💬 stickies-v commented on pull request "wallet: reduce unconditional logging during load":
(https://github.com/bitcoin/bitcoin/pull/33299#discussion_r2390909544)
nit: the function name is informative enough imo, no need for docstring here

📝 maflcko opened a pull request: "ci: Check macos-cross"
(https://github.com/bitcoin/bitcoin/pull/33509)
bla

💬 maflcko commented on pull request "ci: Checkout latest merged pulls":
(https://github.com/bitcoin/bitcoin/pull/33303#discussion_r2391260220)
Hmm, reminds me that this hunk (and the one above) for `windows-cross` and `windows-native-test` was erroneous to replace, because the two tasks depend on each other and must checkout the exact same commit. Otherwise, there could be a CI failure when a pull is merged in between the run of the first task and the second.

Possible solutions could be:

* Revert the two hunks and restore the default behavior here.
* Export the commit ID from the first task and plug it into the checkout of the
...

👋 willcl-ark's pull request is ready for review: "ci: fix buildx gha cache authentication on forks"
(https://github.com/bitcoin/bitcoin/pull/33508)

🤔 stratospher reviewed a pull request: "p2p: Use network-dependent timers for inbound inv scheduling"
(https://github.com/bitcoin/bitcoin/pull/33464#pullrequestreview-3284507245)
ACK beb75e4. liked the way the cache_id logic from getaddr caching was reused here.
observed something similar with @danielabrozzoni's patch!

```
2025-09-30T12:29:05Z Scheduling next inv send time for inbound peer id = 0 on network 8495781695020407500 at time 1759235352548196µs
2025-09-30T12:29:05Z Scheduling next inv send time for inbound peer id = 4 on network 8495781695020407500 at time 1759235352548196µs
2025-09-30T12:29:06Z Scheduling next inv send time for inbound peer id = 2 on net
...

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r2391266074)
Added `// Avoid non-loopback network traffic during tests.`

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r2391272544)
I added brief comments to the newly added functions. `ci/README.md` seems to me too high level for this. No strong opinion though.

💬 vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-3351911963)
`f4fdf81d3a...39f90a4a78`: add some comments, suggested by @fjahr above. Also, restore the `exit 1` which I accidentally removed in a previous push.

> I was hoping I could easily test ...

The traffic is detected and reported in your CI jobs (search for `Error: outbound`), but there was no `exit 1` 🤦. Sorry for that :face_in_clouds: :face_with_head_bandage:

💬 vasild commented on pull request "net: support overriding the proxy selection in ConnectNode()":
(https://github.com/bitcoin/bitcoin/pull/33454#issuecomment-3351986932)
`2ae966b222...0c718da693`: https://github.com/bitcoin/bitcoin/pull/33454#discussion_r2390323798 plus use the correct comment: `/*proxy_override=*/`.