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

💬 maflcko commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2323334963)
> quadratic

Ok, I see. Though, that seems unrelated? I am not touching `test_list` in this pull request (and also not in the line you commented on), so you should be able to create a separate pull (happy to review).



> so the error is wrong - isn't it?

Yes, the message is "wrong". Though, it always was "wrong" (intentionally). The only thing that matters is the `raise`, the message is mostly irrelevant.

💬 l0rinc commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2323365026)
> so you should be able to create a separate pull

yes, I think it should be done in a separate PR, should have clarified that.

> Yes, the message is "wrong".

I'm not blocking, I don't think it's important, but if you edit again, maybe we can remove/update the message to remove the confusion

💬 l0rinc commented on pull request "doc: archive v29.1 release notes":
(https://github.com/bitcoin/bitcoin/pull/33309#issuecomment-3255407984)
I am aware of the doc, just confused which step of the release process this was - it's why I suggested adding a description to the PR to help other reviewers. I don't see why that's a controversial take, I reviewed this because I want to help and documented the parts that I think could be helpful for other reviewers.

💬 hebasto commented on pull request "doc: fix `LIBRARY_PATH` comment":
(https://github.com/bitcoin/bitcoin/pull/33308#discussion_r2323416795)
While the comment's amendment is correct, the comment itself should by applied to both `darwin*` and `mingw*` hosts.

🤔 instagibbs reviewed a pull request: "cluster mempool: control/optimize TxGraph memory usage"
(https://github.com/bitcoin/bitcoin/pull/33157#pullrequestreview-3186772025)
concept ACK, I refuse to do math

💬 instagibbs commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2323216351)
```Suggestion
* removed), return the Cluster it is in and the level the Cluster has. Otherwise, return
```