💬 tobtoht commented on pull request "guix: Remove librt usage from release binaries":
(https://github.com/bitcoin/bitcoin/pull/28069#issuecomment-1635059758)
Guix builds:

```
$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
67078bddd5dc32801b8c916c3bc12f1404da572312f0158a89b9603c1f753969 guix-build-8f6f0d81ee3a/output/aarch64-linux-gnu/SHA256SUMS.part
794dd00009860fd67d7e51463ee1c5ea9677dfff1c739dd0b91cf73136deb655 guix-build-8f6f0d81ee3a/output/aarch64-linux-gnu/bitcoin-8f6f0d81ee3a-aarch64-linux-gnu-debug.tar.gz
eb9cf3f472ffbc37446fe4d80fe81dc62cf1c28c4d57dd8a7b7176e654
...

💬 Crypt-iQ commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#discussion_r1263162187)
I think because the only call to `g_requests_cv.wait(...)` has a predicate that waits for `g_requests.empty()`. I've removed the dependency in the latest patch

💬 Crypt-iQ commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#issuecomment-1635090604)
I've removed the `assert(n == 1)` in `evhttp_request_set_on_complete_cb` because of this comment: https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1611979215. It appears that both `evhttp_connection_set_closecb` and `evhttp_request_set_on_complete_cb` can run which causes a crash. I wasn't able to reproduce it and still want to know what exactly happened, but I think removing the assert here is certainly better than having a crash.

💬 Crypt-iQ commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#discussion_r1263163161)
done

💬 Crypt-iQ commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#discussion_r1263163190)
done

✅ JayBitron closed a pull request: "exclude ipc scheme from port check"
(https://github.com/bitcoin/bitcoin/pull/28020)

💬 MarcoFalke commented on pull request "refactor: Continue moving application data from CNode to Peer":
(https://github.com/bitcoin/bitcoin/pull/26621#issuecomment-1635315780)
Needs rebase if still relevant

🤔 MarcoFalke reviewed a pull request: "test: add end-to-end tests for CConnman and PeerManager"
(https://github.com/bitcoin/bitcoin/pull/26812#pullrequestreview-1529671710)
Not sure about changing the framework.

Also, needs rebase if still relevant.

💬 MarcoFalke commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1263332063)
Not sure about changing this. It just makes it harder to use. If you prefer to not have an exception, this can use an `Assert`? Also, in the commit description, it would be good to refer to documentation why it is "not allowed", and/or include an example of what happens.

💬 MarcoFalke commented on pull request "net: Use serialization parameters for CAddress serialization":
(https://github.com/bitcoin/bitcoin/pull/25284#issuecomment-1635587006)
rebased

💬 dergoegge commented on pull request "refactor: Continue moving application data from CNode to Peer":
(https://github.com/bitcoin/bitcoin/pull/26621#issuecomment-1635588195)
> Needs rebase if still relevant

Thanks, rebased

💬 fanquake commented on pull request "ci: build Valgrind (3.21) from source":
(https://github.com/bitcoin/bitcoin/pull/27992#issuecomment-1635607860)
I'll follow up with just adding more suppressions until things work on aarch64.

✅ fanquake closed a pull request: "ci: build Valgrind (3.21) from source"
(https://github.com/bitcoin/bitcoin/pull/27992)

💬 fanquake commented on pull request "ci: Add missing -O2 to valgrind tasks":
(https://github.com/bitcoin/bitcoin/pull/28071#issuecomment-1635610985)
> Right. Probably not worth to spend time here,

I'll follow up with this, because it certainly shouldn't be broken, and if it is, it's not specific to valgrind etc.

🚀 fanquake merged a pull request: "ci: Add missing -O2 to valgrind tasks"
(https://github.com/bitcoin/bitcoin/pull/28071)

👍 hebasto approved a pull request: "kernel: Remove StartShutdown calls from validation code"
(https://github.com/bitcoin/bitcoin/pull/28048#pullrequestreview-1530008568)
ACK 31eca93a9eb8e54f856d3f558aa3c831ef181d37, I have reviewed the code and it looks OK.

💬 hebasto commented on pull request "kernel: Remove StartShutdown calls from validation code":
(https://github.com/bitcoin/bitcoin/pull/28048#discussion_r1263547071)
This requires to `#include "kernel/notifications_interface.h"`, no?

💬 MarcoFalke commented on pull request "ci: build Valgrind (3.21) from source":
(https://github.com/bitcoin/bitcoin/pull/27992#issuecomment-1635633574)
I haven't tried, but is this still an issue on aarch64 on current master?

💬 MarcoFalke commented on issue "Spurious (?) valgrind failure for p2p_compactblocks.py":
(https://github.com/bitcoin/bitcoin/issues/27741#issuecomment-1635635435)
I haven't tried, but an alternative to using `clang` would be to try to set `-O1` for `gcc`, but I am not sure on how to do this cleanly?

💬 hebasto commented on pull request "blockstorage: Return on fatal flush errors":
(https://github.com/bitcoin/bitcoin/pull/27866#issuecomment-1635642358)
Concept ACK.