💬 MarcoFalke commented on pull request "ci: document that -Wreturn-type has been fixed upstream (mingw-w64)":
(https://github.com/bitcoin/bitcoin/pull/28092#issuecomment-1641704810)
> once the time-machine is bumped.
Doesn't this break other builds? So I was wondering if the time machine could be bumped (for mingw), but the Linux tasks be kept as-is for now?
(https://github.com/bitcoin/bitcoin/pull/28092#issuecomment-1641704810)
> once the time-machine is bumped.
Doesn't this break other builds? So I was wondering if the time machine could be bumped (for mingw), but the Linux tasks be kept as-is for now?
💬 MarcoFalke commented on pull request "ci: document that -Wreturn-type has been fixed upstream (mingw-w64)":
(https://github.com/bitcoin/bitcoin/pull/28092#issuecomment-1641709498)
(I mean, obviously my preference would be to use clang for Windows cross-builds, because mingw doesn't seem to be used by any large/relevant software project anymore, so using it at all is becoming increasingly fragile, but doing another bump shouldn't hurt too much for now)
(https://github.com/bitcoin/bitcoin/pull/28092#issuecomment-1641709498)
(I mean, obviously my preference would be to use clang for Windows cross-builds, because mingw doesn't seem to be used by any large/relevant software project anymore, so using it at all is becoming increasingly fragile, but doing another bump shouldn't hurt too much for now)
🤔 jonasnick reviewed a pull request: "subtree: update libsecp256k1 to latest master"
(https://github.com/bitcoin/bitcoin/pull/28093#pullrequestreview-1536639285)
ACK 5080c9c25f44ae9d16a69d41f0da1d1e06483bf7
(https://github.com/bitcoin/bitcoin/pull/28093#pullrequestreview-1536639285)
ACK 5080c9c25f44ae9d16a69d41f0da1d1e06483bf7
🚀 fanquake merged a pull request: "test: move remaining rand code from util/setup_common to util/random"
(https://github.com/bitcoin/bitcoin/pull/27425)
(https://github.com/bitcoin/bitcoin/pull/27425)
📝 fanquake opened a pull request: "build: promote -Wunused-member-function to -Wunused"
(https://github.com/bitcoin/bitcoin/pull/28102)
This doesn't produce any additional warnings for me under GCC 13 or Clang 16, so it'd seem useful to use the less restricted unused code warning.
`-Wno-unused-parameter` is still required.
(https://github.com/bitcoin/bitcoin/pull/28102)
This doesn't produce any additional warnings for me under GCC 13 or Clang 16, so it'd seem useful to use the less restricted unused code warning.
`-Wno-unused-parameter` is still required.
💬 ajtowns commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1267822413)
I think I messed this up in the [bad rebase](https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1253703097) :( Should be fixed now.
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1267822413)
I think I messed this up in the [bad rebase](https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1253703097) :( Should be fixed now.
💬 ajtowns commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1267822689)
Dropped
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1267822689)
Dropped
💬 kristapsk commented on pull request "During IBD, prune as much as possible until we get close to where we will eventually keep blocks":
(https://github.com/bitcoin/bitcoin/pull/20827#issuecomment-1641766302)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/20827#issuecomment-1641766302)
Concept ACK
📝 MarcoFalke opened a pull request: " lint: Add missing `set -ex` to ci/lint/06_script.sh "
(https://github.com/bitcoin/bitcoin/pull/28103)
Requested in https://github.com/bitcoin/bitcoin/pull/28083#pullrequestreview-1535304219.
Also, one doc commit.
(https://github.com/bitcoin/bitcoin/pull/28103)
Requested in https://github.com/bitcoin/bitcoin/pull/28083#pullrequestreview-1535304219.
Also, one doc commit.
💬 MarcoFalke commented on pull request "ci: Use DOCKER_BUILDKIT for lint image":
(https://github.com/bitcoin/bitcoin/pull/28083#issuecomment-1641769027)
Done in https://github.com/bitcoin/bitcoin/pull/28103
(https://github.com/bitcoin/bitcoin/pull/28083#issuecomment-1641769027)
Done in https://github.com/bitcoin/bitcoin/pull/28103
👍 hebasto approved a pull request: "depends: xcb-proto 1.15.2"
(https://github.com/bitcoin/bitcoin/pull/28097#pullrequestreview-1536745294)
ACK 7cb88c8b46723d306b96953a6a60c90a4ab211e3, tested on Ubuntu Mantic with:
```
$ python3 --version
Python 3.12.0b4
```
(https://github.com/bitcoin/bitcoin/pull/28097#pullrequestreview-1536745294)
ACK 7cb88c8b46723d306b96953a6a60c90a4ab211e3, tested on Ubuntu Mantic with:
```
$ python3 --version
Python 3.12.0b4
```
💬 vasild commented on pull request "net: disconnect inside AttemptToEvictConnection":
(https://github.com/bitcoin/bitcoin/pull/27912#issuecomment-1641806637)
Here is a patch on top of this PR that should address all concerns above about thread safetyness.
<details>
<summary>[patch] thread safe</summary>
```diff
diff --git i/src/net.cpp w/src/net.cpp
index c2160e945f..058f615dd5 100644
--- i/src/net.cpp
+++ w/src/net.cpp
@@ -886,12 +886,14 @@ size_t CConnman::SocketSendData(CNode& node) const
* to forge. In order to partition a node the attacker must be
* simultaneously better at all of them than honest peers.
* If we find
...
(https://github.com/bitcoin/bitcoin/pull/27912#issuecomment-1641806637)
Here is a patch on top of this PR that should address all concerns above about thread safetyness.
<details>
<summary>[patch] thread safe</summary>
```diff
diff --git i/src/net.cpp w/src/net.cpp
index c2160e945f..058f615dd5 100644
--- i/src/net.cpp
+++ w/src/net.cpp
@@ -886,12 +886,14 @@ size_t CConnman::SocketSendData(CNode& node) const
* to forge. In order to partition a node the attacker must be
* simultaneously better at all of them than honest peers.
* If we find
...
💬 MarcoFalke commented on pull request "build: promote -Wunused-member-function to -Wunused":
(https://github.com/bitcoin/bitcoin/pull/28102#issuecomment-1641808324)
For gcc this is already in Wall. Not sure for clang.
Not sure if related: https://cirrus-ci.com/task/6059497324544000?logs=ci#L2338
(https://github.com/bitcoin/bitcoin/pull/28102#issuecomment-1641808324)
For gcc this is already in Wall. Not sure for clang.
Not sure if related: https://cirrus-ci.com/task/6059497324544000?logs=ci#L2338
💬 hebasto commented on pull request "build: promote -Wunused-member-function to -Wunused":
(https://github.com/bitcoin/bitcoin/pull/28102#issuecomment-1641810745)
For clang, [`-Wunused`](https://clang.llvm.org/docs/DiagnosticsReference.html#wunused) does not control `-Wunused-member-function`, no?
(https://github.com/bitcoin/bitcoin/pull/28102#issuecomment-1641810745)
For clang, [`-Wunused`](https://clang.llvm.org/docs/DiagnosticsReference.html#wunused) does not control `-Wunused-member-function`, no?
💬 hebasto commented on pull request "depends: xcb-proto 1.15.2":
(https://github.com/bitcoin/bitcoin/pull/28097#issuecomment-1641813666)
Guix builds:
```
6d1725c9346bdf04e1eeec2deffda90957bd081700ba896834a143756410fb0e guix-build-7cb88c8b4672/output/aarch64-linux-gnu/SHA256SUMS.part
34486c39daea8a3ce8d92e9c1501f26e5f5e300d9612a25bd56c48ab56353329 guix-build-7cb88c8b4672/output/aarch64-linux-gnu/bitcoin-7cb88c8b4672-aarch64-linux-gnu-debug.tar.gz
cb0080d75d0fc4fc5c7fc022b5771470c60609c35836e7b66c4f479a2edcd098 guix-build-7cb88c8b4672/output/aarch64-linux-gnu/bitcoin-7cb88c8b4672-aarch64-linux-gnu.tar.gz
395c332f83206125819
...
(https://github.com/bitcoin/bitcoin/pull/28097#issuecomment-1641813666)
Guix builds:
```
6d1725c9346bdf04e1eeec2deffda90957bd081700ba896834a143756410fb0e guix-build-7cb88c8b4672/output/aarch64-linux-gnu/SHA256SUMS.part
34486c39daea8a3ce8d92e9c1501f26e5f5e300d9612a25bd56c48ab56353329 guix-build-7cb88c8b4672/output/aarch64-linux-gnu/bitcoin-7cb88c8b4672-aarch64-linux-gnu-debug.tar.gz
cb0080d75d0fc4fc5c7fc022b5771470c60609c35836e7b66c4f479a2edcd098 guix-build-7cb88c8b4672/output/aarch64-linux-gnu/bitcoin-7cb88c8b4672-aarch64-linux-gnu.tar.gz
395c332f83206125819
...
💬 fanquake commented on pull request "Add `UNREACHABLE` macro":
(https://github.com/bitcoin/bitcoin/pull/26504#issuecomment-1641813690)
> I don't care about updating the existing code,
To be clear, I agree, and am still a NACK to any code changes here.
(https://github.com/bitcoin/bitcoin/pull/26504#issuecomment-1641813690)
> I don't care about updating the existing code,
To be clear, I agree, and am still a NACK to any code changes here.
💬 fanquake commented on pull request "I2P: also sleep after errors in Accept() & destroy the session if we get "Session was closed"":
(https://github.com/bitcoin/bitcoin/pull/28077#issuecomment-1641828054)
> I should report this upstream as per https://geti2p.net/en/faq#bug.
Great. Can you link to the issue once it's reported upstream, as it'd be good to know which (if any) of the changes here are required just to work around buggy router software.
(https://github.com/bitcoin/bitcoin/pull/28077#issuecomment-1641828054)
> I should report this upstream as per https://geti2p.net/en/faq#bug.
Great. Can you link to the issue once it's reported upstream, as it'd be good to know which (if any) of the changes here are required just to work around buggy router software.
🤔 hebasto reviewed a pull request: "validation: use noexcept instead of deprecated throw()"
(https://github.com/bitcoin/bitcoin/pull/28090#pullrequestreview-1536785324)
Post-merge ACK 047daad4f59942488163c6be8516a69291646294.
(https://github.com/bitcoin/bitcoin/pull/28090#pullrequestreview-1536785324)
Post-merge ACK 047daad4f59942488163c6be8516a69291646294.
💬 fanquake commented on pull request "build: promote -Wunused-member-function to -Wunused":
(https://github.com/bitcoin/bitcoin/pull/28102#issuecomment-1641843137)
> Not sure if related: https://cirrus-ci.com/task/6059497324544000?logs=ci#L2338
I thought we'd cleaned these up at some point, can be left as-is in any case.
(https://github.com/bitcoin/bitcoin/pull/28102#issuecomment-1641843137)
> Not sure if related: https://cirrus-ci.com/task/6059497324544000?logs=ci#L2338
I thought we'd cleaned these up at some point, can be left as-is in any case.
✅ fanquake closed a pull request: "build: promote -Wunused-member-function to -Wunused"
(https://github.com/bitcoin/bitcoin/pull/28102)
(https://github.com/bitcoin/bitcoin/pull/28102)