👍 l0rinc approved a pull request: "doc: fix `LIBRARY_PATH` comment"
(https://github.com/bitcoin/bitcoin/pull/33308#pullrequestreview-3186756095)
code review ACK a2a35b58cb957b7fba113eb4cf933d5e7b9bf8ef

💬 sipa commented on pull request "doc: archive v29.1 release notes":
(https://github.com/bitcoin/bitcoin/pull/33309#issuecomment-3255200938)
@l0rinc This is just an unmodified copy from the release notes from the 29.x branch: https://github.com/bitcoin/bitcoin/blob/v29.1/doc/release-notes.md, so they survive in the master branch.

💬 laanwj commented on issue "natpmp: quiet down unconditional logging":
(https://github.com/bitcoin/bitcoin/issues/33301#issuecomment-3255228389)
> Is it worth trying every 5 minutes forever instead of giving up after a number of tries? Maybe just log unconditionally once but still try ever 5 minutes?

Yes, imo it is. And i don't think exponential backoff would be good here, either. If the user switches to a network with a PCP-supporting router, we want to use that as soon as possible.

But moving the noisy failure messages to `net` category makes sense, imo.

💬 l0rinc commented on pull request "index: Force database compaction in coinstatsindex":
(https://github.com/bitcoin/bitcoin/pull/33306#discussion_r2323203417)
could you please separate the low risk changes from the riskier ones? This extraction seems like a simple refactor, would be great to see this in a very simple first commit, so that we can focus on the new call in `src/index/coinstatsindex.cpp`

💬 l0rinc commented on pull request "index: Force database compaction in coinstatsindex":
(https://github.com/bitcoin/bitcoin/pull/33306#discussion_r2323209517)
as other have also mentioned, compaction is already a background process, it is triggered regularly, it should already take care of these as far as I can tell.
Is it urgent to do the expensive compaction regularly? Doing more compactions should take more time overall, can you please share your benchmarks in this regard?

💬 l0rinc commented on pull request "index: Force database compaction in coinstatsindex":
(https://github.com/bitcoin/bitcoin/pull/33306#discussion_r2323218302)
is `const` the right thing to advertise here? And is it a `noexcept`?

💬 l0rinc commented on pull request "index: Force database compaction in coinstatsindex":
(https://github.com/bitcoin/bitcoin/pull/33306#discussion_r2323214015)
shouldn't this be a debug? Do we need 91 instances of this?
(haven't checked, but will this be triggered for genesis?)

💬 l0rinc commented on pull request "index: Force database compaction in coinstatsindex":
(https://github.com/bitcoin/bitcoin/pull/33306#discussion_r2323228522)
nit: to avoid counting the `0`s manually:
```C++
if (block.height % 10'000 == 0) {
```
or
```C++
if (!(block.height % 10'000)) {
```

💬 willcl-ark commented on pull request "ci: Checkout latest merged pulls":
(https://github.com/bitcoin/bitcoin/pull/33303#issuecomment-3255266380)
> > Kinda in doubt about the use of YAML_KEY_01, in code it makes it clear that it is a YAML key but in the action execution it is visible as a `env` variable :
>
> I know, but I don't know any alternative. See also the GitHub schema validator on a previous run: [bitcoin/bitcoin/actions/runs/17471191651/workflow](https://github.com/bitcoin/bitcoin/actions/runs/17471191651/workflow):
>
> ```
> .github/workflows/ci.yml
>
> Invalid workflow file
>
> (Line: 21, Col: 1): Unexpected value
...

💬 maflcko commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2323259358)
> We have `self.test_list.pop(0)` at the beginning:

This will already throw an IndexError. There is no need to check it manually twice for it. The check here is for `self.jobs`, to avoid an infinite `while True` loop in case of a coding error.

💬 l0rinc commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2323258878)
what happened here?

👍 willcl-ark approved a pull request: "doc: fix `LIBRARY_PATH` comment"
(https://github.com/bitcoin/bitcoin/pull/33308#pullrequestreview-3186851296)
ACK a2a35b58cb957b7fba113eb4cf933d5e7b9bf8ef

📝 luke-jr opened a pull request: "trace: Workaround GCC bug compiling with old systemtap"
(https://github.com/bitcoin/bitcoin/pull/33310)

💬 l0rinc commented on pull request "doc: archive v29.1 release notes":
(https://github.com/bitcoin/bitcoin/pull/33309#issuecomment-3255286967)
Ah, so that's what the `archive` was referring to, thanks.
Could we add that to the PR description?

📝 laanwj opened a pull request: "net: Quiet down logging when router doesn't support natpmp/pcp"
(https://github.com/bitcoin/bitcoin/pull/33311)
When the router doesn't support natpmp and PCP, one'd normally expect the UDP packet to be ignored, and hit a time out. This logs a warning that is already in the debug category. However, there's also the case in which sending an UDP packet causes a ICMP response. This is returned to user space as "connection refused" (despite UDP having no concept of connections).

Move the warnings from `Send` and `Recv` to debug level too, to reduce log spam in that case.

Closes #33301.

💬 l0rinc commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2323269567)
yes, but that's quadratic and `self.jobs` is not a list anymore, so the error is wrong - isn't it?

👍 darosior approved a pull request: "net: Quiet down logging when router doesn't support natpmp/pcp"
(https://github.com/bitcoin/bitcoin/pull/33311#pullrequestreview-3186874451)
utACK 4f1a4cbccd784e25f7932f1d0293602ef7f3e814

💬 willcl-ark commented on pull request "[29.x] *san CI backports":
(https://github.com/bitcoin/bitcoin/pull/33294#issuecomment-3255304727)
Are you planning on adding these to release notes in this PR?

💬 maflcko commented on pull request "doc: archive v29.1 release notes":
(https://github.com/bitcoin/bitcoin/pull/33309#issuecomment-3255307255)
> Could we add that to the PR description?

It is documented in the process: https://github.com/bitcoin/bitcoin/blob/v29.1/doc/release-process.md#after-6-or-more-people-have-guix-built-and-their-results-match

💬 willcl-ark commented on pull request "net: Quiet down logging when router doesn't support natpmp/pcp":
(https://github.com/bitcoin/bitcoin/pull/33311#issuecomment-3255334801)
utACK 4f1a4cbccd784e25f7932f1d0293602ef7f3e814

Makes sense to demote this message to Debug level.