💬 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?
(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
(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?
(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
(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.
(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.
(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
(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.
(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.
(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
(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
```
(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
```
💬 instagibbs commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2323344088)
probably should be explicit in comments that this is a memory optimization, letting hole-having clusters be reconstructed without holes
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2323344088)
probably should be explicit in comments that this is a memory optimization, letting hole-having clusters be reconstructed without holes
💬 instagibbs commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2323362751)
this in unconditionally called I think
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2323362751)
this in unconditionally called I think
💬 instagibbs commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2323384935)
not sure I track why the command values are being mapped to specific series of actions. Writing it out in case it ends up helpful:
0:
a. AddTransaction if txns below set limit
b. AddDependency if any txns exist
c. RemoveTransactions if any txns exist
d. Compact
1:
a. AddTransaction if no transaction exists
b. AddDependency (transaction will exist)
c. RemoveTransactions (transaction will exist)
d. Compact
2:
a. AddTransaction if no transaction exists
b. RemoveTransactions (trans
...
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2323384935)
not sure I track why the command values are being mapped to specific series of actions. Writing it out in case it ends up helpful:
0:
a. AddTransaction if txns below set limit
b. AddDependency if any txns exist
c. RemoveTransactions if any txns exist
d. Compact
1:
a. AddTransaction if no transaction exists
b. AddDependency (transaction will exist)
c. RemoveTransactions (transaction will exist)
d. Compact
2:
a. AddTransaction if no transaction exists
b. RemoveTransactions (trans
...
🤔 mzumsande reviewed a pull request: "net: Quiet down logging when router doesn't support natpmp/pcp"
(https://github.com/bitcoin/bitcoin/pull/33311#pullrequestreview-3187161713)
utACK 4f1a4cbccd784e25f7932f1d0293602ef7f3e814
(https://github.com/bitcoin/bitcoin/pull/33311#pullrequestreview-3187161713)
utACK 4f1a4cbccd784e25f7932f1d0293602ef7f3e814
💬 hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3255622984)
Rebased and updated using the [latest version](https://github.com/bitcoin-core/bitcoin-maintainer-tools/pull/188) of the tool.
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3255622984)
Rebased and updated using the [latest version](https://github.com/bitcoin-core/bitcoin-maintainer-tools/pull/188) of the tool.
💬 TheCharlatan commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2323536325)
How does this condition get fulfilled? I set `uses_wallet` manually in the `set_test_params`, but then the assertion on line 161 fails, which is a bit puzzling, because that would mean something changed in the template in the meantime.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2323536325)
How does this condition get fulfilled? I set `uses_wallet` manually in the `set_test_params`, but then the assertion on line 161 fails, which is a bit puzzling, because that would mean something changed in the template in the meantime.
💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2323550726)
It's set by `skip_if_no_wallet`.
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2323550726)
It's set by `skip_if_no_wallet`.
💬 sipa commented on pull request "net: Quiet down logging when router doesn't support natpmp/pcp":
(https://github.com/bitcoin/bitcoin/pull/33311#issuecomment-3255678817)
utACK 4f1a4cbccd784e25f7932f1d0293602ef7f3e814
(https://github.com/bitcoin/bitcoin/pull/33311#issuecomment-3255678817)
utACK 4f1a4cbccd784e25f7932f1d0293602ef7f3e814
💬 davidgumberg commented on pull request "net: Quiet down logging when router doesn't support natpmp/pcp":
(https://github.com/bitcoin/bitcoin/pull/33311#issuecomment-3255679309)
Tested ACK https://github.com/bitcoin/bitcoin/commit/4f1a4cbccd784e25f7932f1d0293602ef7f3e814
Ran this branch on a network where the default gateway does not support NatPMP, with `-natpmp=1` and `pcp: Could not receive response: Connection refused` is not printed, on master it is.
```console
$ mkdir -p /tmp/datadir && ./build/bin/bitcoind -regtest -natpmp=1 | grep -i pcp
```
On master:
```console
2025-09-04T21:03:19Z [net:warning] pcp: Could not receive response: Connection refu
...
(https://github.com/bitcoin/bitcoin/pull/33311#issuecomment-3255679309)
Tested ACK https://github.com/bitcoin/bitcoin/commit/4f1a4cbccd784e25f7932f1d0293602ef7f3e814
Ran this branch on a network where the default gateway does not support NatPMP, with `-natpmp=1` and `pcp: Could not receive response: Connection refused` is not printed, on master it is.
```console
$ mkdir -p /tmp/datadir && ./build/bin/bitcoind -regtest -natpmp=1 | grep -i pcp
```
On master:
```console
2025-09-04T21:03:19Z [net:warning] pcp: Could not receive response: Connection refu
...