💬 Sjors commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1568433362)
Happy to leave that to later PR.

tACK e82e73103ce81159f6f1a51408ce5411b88a12b2

💬 TheCharlatan commented on issue "Libbitcoinkernel Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27587#issuecomment-1568433462)
Re https://github.com/bitcoin/bitcoin/issues/27587#issuecomment-1568419077

> Would it make sense to update the project page to https://github.com/orgs/bitcoin/projects/3/views/1 (which if I'm not mistaken seems to be the current one)?

Thanks for the notice, updated to the new one.

> Could keep a link to the old project https://github.com/bitcoin/bitcoin/projects/18 for reference?

There is no useful extra information on the old board that is not in the new one, so I don't thi
...

💬 fanquake commented on pull request "wallet, bench: Move commonly used functions to their own file and fix a bug":
(https://github.com/bitcoin/bitcoin/pull/27666#issuecomment-1568469989)
@furszy looks like you ACK'd the wrong commit here.

💬 instagibbs commented on issue "24.0.1 crash on restart":
(https://github.com/bitcoin/bitcoin/issues/27088#issuecomment-1568481549)
no intention to chase after this, close?

🚀 fanquake merged a pull request: "kernel: Remove util/system from kernel library, interface_ui from validation."
(https://github.com/bitcoin/bitcoin/pull/27636)

💬 furszy commented on pull request "wallet, bench: Move commonly used functions to their own file and fix a bug":
(https://github.com/bitcoin/bitcoin/pull/27666#issuecomment-1568486957)
oops, fixed. That was the one from the comment. Thanks @fanquake.

💬 MarcoFalke commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#discussion_r1210324682)
Do dangling image (layer)s have a label? Anyway, this isn't for local runs, only for the persistent runners, guarded via `RESTART_CI_DOCKER_BEFORE_RUN`. Happy to rename `RESTART_CI_DOCKER_BEFORE_RUN` to `DANGER_RESTART_CI_DOCKER_BEFORE_RUN`?

✅ MarcoFalke closed an issue: "24.0.1 crash on restart"
(https://github.com/bitcoin/bitcoin/issues/27088)

👍 dergoegge approved a pull request: "ci: Enable float-divide-by-zero check"
(https://github.com/bitcoin/bitcoin/pull/27778#pullrequestreview-1451049584)
ACK fa3ab4520317f48d4700b81dab023c4e639bbd68

💬 fanquake commented on pull request "ci, iwyu: Update mappings":
(https://github.com/bitcoin/bitcoin/pull/27710#discussion_r1210341923)
What's the reason for leaving certain mappings out of this list, i.e `#include <boost/operators.hpp>`?

I'm guessing it's because those same includes are recommended for non-test code, and mapping them to Boost Test here would just produce nonsense recommendations for other files? If so, how are you planning on working around that in future? If not, or that isn't a problem, why not do the mapping now?

👍 stickies-v approved a pull request: "refactor: Add [[nodiscard]] where ignoring a Result return type is an error"
(https://github.com/bitcoin/bitcoin/pull/27774#pullrequestreview-1451069282)
ACK fa5680b7520c340952239c4e25ebe505d16c7c39

I looked at all sites where `Result` is either of type `void` or some other descriptive/status type, and could not find any other places where the return value can never be ignored.

👍 stickies-v approved a pull request: "refactor: Add [[nodiscard]] where ignoring a Result return type is an error"
(https://github.com/bitcoin/bitcoin/pull/27774#pullrequestreview-1451069356)
ACK fa5680b7520c340952239c4e25ebe505d16c7c39

I looked at all sites where `Result` is either of type `void` or some other descriptive/status type, and could not find any other places where the return value can never be ignored.

👍 theStack approved a pull request: "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`"
(https://github.com/bitcoin/bitcoin/pull/26261#pullrequestreview-1451070881)
re-ACK 5c832c3820253affc65c0ed490e26e5b0a4d5c9b

> Maybe done with nits, and we can have it merged?

Agree!

🚀 fanquake merged a pull request: "refactor: Add [[nodiscard]] where ignoring a Result return type is an error"
(https://github.com/bitcoin/bitcoin/pull/27774)

💬 Sjors commented on pull request "assumeutxo (2)":
(https://github.com/bitcoin/bitcoin/pull/27596#discussion_r1210397176)
Which version of Python are you using?

💬 mzumsande commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1210400306)
Not really sure, I've never looked into these options much and don't know about best practices - I think that if you use `-whitebind` with non-local addresses, you'd at least have to make sure that that address is not self-advertised. I guess that this applies more to `-whitebind` than `-whitelist`, so that the preferred approach in case of a non-local connection would be to use `-whitelist`?
fyi @vasild, do you have an opinion on this?

💬 stickies-v commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#discussion_r1210393080)
I'm not sure this works? Even though the CLIs have a compatible syntax, I think they operate on their own set of resources. I'm pretty sure `podman ps` right before this line won't show anything, so I think no containers are killed after this line? At least, that's what I get when I try locally spinning up a container with `docker run` and then try to show/kill it with `podman ps` or `podman container kill --all`.

💬 stickies-v commented on pull request "ci: Prune dangling images on RESTART_CI_DOCKER_BEFORE_RUN":
(https://github.com/bitcoin/bitcoin/pull/27777#discussion_r1210393258)
> Do dangling image (layer)s have a label?

Yeah they do, just tested it to confirm.

> guarded via RESTART_CI_DOCKER_BEFORE_RUN

Fair enough, looks like this is unlikely to be hit locally. I don't think renaming to `DANGER_RESTART_CI_DOCKER_BEFORE_RUN` is an improvement, so happy to keep as is. Just thought adding a label and only applying batch operations to our own images is a good practice, e.g. if in the future we remove the `RESTART_CI_DOCKER_BEFORE_RUN` condition, given that it's al
...

💬 dergoegge commented on pull request "refactor: Guard TxRequestTracker by its own lock instead of cs_main":
(https://github.com/bitcoin/bitcoin/pull/26151#issuecomment-1568587465)
Closing this for now, can be marked up for grabs.

https://github.com/bitcoin/bitcoin/pull/26151#discussion_r1003216463 needs to be addressed for this to move forward. Imo, the interface of `TxRequestTracker` should change to internally enforce the `MAX_PEER_TX_ANNOUNCEMENTS` and `MAX_PEER_TX_REQUEST_IN_FLIGHT` limits.

✅ dergoegge closed a pull request: "refactor: Guard TxRequestTracker by its own lock instead of cs_main"
(https://github.com/bitcoin/bitcoin/pull/26151)