💬 TheCharlatan commented on pull request "depends: suggest GNU patch for macOS <= 13":
(https://github.com/bitcoin/bitcoin/pull/29814#issuecomment-2041347112)
Found this https://github.com/bazelbuild/bazel/issues/16550 issue that seems to describe what might be going. Sounds like it is only a macos 13 issue?
(https://github.com/bitcoin/bitcoin/pull/29814#issuecomment-2041347112)
Found this https://github.com/bazelbuild/bazel/issues/16550 issue that seems to describe what might be going. Sounds like it is only a macos 13 issue?
💬 maflcko commented on pull request "test: remove immediate tx relay workaround in wallet_groups.py":
(https://github.com/bitcoin/bitcoin/pull/29822#issuecomment-2041373716)
lgtm ACK 93fae5ae7c31fa1b1095770f00adeac1cfeda4b9
(https://github.com/bitcoin/bitcoin/pull/29822#issuecomment-2041373716)
lgtm ACK 93fae5ae7c31fa1b1095770f00adeac1cfeda4b9
💬 maflcko commented on pull request "fuzz: Some `test/fuzz/test_runner.py` improvements":
(https://github.com/bitcoin/bitcoin/pull/29821#issuecomment-2041376353)
lgtm ACK 47cedee776c6253232beb6039ea708c578211327
(https://github.com/bitcoin/bitcoin/pull/29821#issuecomment-2041376353)
lgtm ACK 47cedee776c6253232beb6039ea708c578211327
💬 maflcko commented on pull request "ci: remove --with-asm=no (secp256k1) from MSAN jobs":
(https://github.com/bitcoin/bitcoin/pull/29742#issuecomment-2041376657)
lgtm ACK 61641e2466768e128fef995e9fcb24cad90e527d
(https://github.com/bitcoin/bitcoin/pull/29742#issuecomment-2041376657)
lgtm ACK 61641e2466768e128fef995e9fcb24cad90e527d
💬 maflcko commented on pull request "refactor, bench, fuzz: Drop unneeded `UCharCast` calls":
(https://github.com/bitcoin/bitcoin/pull/29820#issuecomment-2041377755)
lgtm ACK 56e1e5dd10cbe51d3abc3fbf532b6b41bf62a889
A (more invasive) alternative would be to use `Set(Span<std::byte>)`, or `Set(Span<BasicByte>)`
(https://github.com/bitcoin/bitcoin/pull/29820#issuecomment-2041377755)
lgtm ACK 56e1e5dd10cbe51d3abc3fbf532b6b41bf62a889
A (more invasive) alternative would be to use `Set(Span<std::byte>)`, or `Set(Span<BasicByte>)`
💬 fanquake commented on pull request "test: Bump timeouts in feature_index_prune and wallet_importdescriptors":
(https://github.com/bitcoin/bitcoin/pull/29791#issuecomment-2041390720)
cc @m3dwards, as I think you had been seeing these timeouts as well.
(https://github.com/bitcoin/bitcoin/pull/29791#issuecomment-2041390720)
cc @m3dwards, as I think you had been seeing these timeouts as well.
🚀 fanquake merged a pull request: "test: Bump timeouts in feature_index_prune and wallet_importdescriptors"
(https://github.com/bitcoin/bitcoin/pull/29791)
(https://github.com/bitcoin/bitcoin/pull/29791)
💬 fanquake commented on pull request "depends: add new LLVM debug macro":
(https://github.com/bitcoin/bitcoin/pull/29781#issuecomment-2041397702)
> utACK. Should darwin get this too though since it also uses libc++ ?
Maybe we could look into this once we switch the Darwin cross builds to use LLVM 18 (currently 17)? At this point the newest Apple Clang is still also based on LLVM 16.
(https://github.com/bitcoin/bitcoin/pull/29781#issuecomment-2041397702)
> utACK. Should darwin get this too though since it also uses libc++ ?
Maybe we could look into this once we switch the Darwin cross builds to use LLVM 18 (currently 17)? At this point the newest Apple Clang is still also based on LLVM 16.
🚀 fanquake merged a pull request: "ci: remove --with-asm=no (secp256k1) from MSAN jobs"
(https://github.com/bitcoin/bitcoin/pull/29742)
(https://github.com/bitcoin/bitcoin/pull/29742)
🚀 fanquake merged a pull request: "test: remove immediate tx relay workaround in wallet_groups.py"
(https://github.com/bitcoin/bitcoin/pull/29822)
(https://github.com/bitcoin/bitcoin/pull/29822)
📝 theStack opened a pull request: "test: p2p: add test for rejected tx request logic (`m_recent_rejects` filter)"
(https://github.com/bitcoin/bitcoin/pull/29827)
Motivated by the discussion in #28970 (https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1553911167), this PR adds test coverage for the logic around the `m_recent_rejects` filter, in particular that the filter is cleared after a new block comes in:
https://github.com/bitcoin/bitcoin/blob/f0794cbd405636a7f528a60f2873050b865cf7e8/src/net_processing.cpp#L2199-L2206
As expected, the second part of the test fails if the following patch is applied:
```diff
diff --git a/src/net_processin
...
(https://github.com/bitcoin/bitcoin/pull/29827)
Motivated by the discussion in #28970 (https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1553911167), this PR adds test coverage for the logic around the `m_recent_rejects` filter, in particular that the filter is cleared after a new block comes in:
https://github.com/bitcoin/bitcoin/blob/f0794cbd405636a7f528a60f2873050b865cf7e8/src/net_processing.cpp#L2199-L2206
As expected, the second part of the test fails if the following patch is applied:
```diff
diff --git a/src/net_processin
...
👍 AngusP approved a pull request: "test: Cleans some manual checks/drops when using wait_for_getdata"
(https://github.com/bitcoin/bitcoin/pull/29748#pullrequestreview-1985044436)
ACK df90ea6c73412736608ffc6074b73da980ad25a5
(https://github.com/bitcoin/bitcoin/pull/29748#pullrequestreview-1985044436)
ACK df90ea6c73412736608ffc6074b73da980ad25a5
👍 AngusP approved a pull request: "test: refactor: introduce and use `calculate_input_weight` helper"
(https://github.com/bitcoin/bitcoin/pull/29777#pullrequestreview-1985067835)
tACK 6d91cb781c30966963f28e7577c7aa3829fa9390
All functional tests pass for me including affected `rpc_psbt` and `wallet_send`
(https://github.com/bitcoin/bitcoin/pull/29777#pullrequestreview-1985067835)
tACK 6d91cb781c30966963f28e7577c7aa3829fa9390
All functional tests pass for me including affected `rpc_psbt` and `wallet_send`
💬 dergoegge commented on pull request "clang-tidy: Enable misc-no-recursion":
(https://github.com/bitcoin/bitcoin/pull/29690#issuecomment-2041464695)
Rebased due to conflict
(https://github.com/bitcoin/bitcoin/pull/29690#issuecomment-2041464695)
Rebased due to conflict
👍 TheCharlatan approved a pull request: "clang-tidy: Enable misc-no-recursion"
(https://github.com/bitcoin/bitcoin/pull/29690#pullrequestreview-1985077942)
Re-ACK 78407b99ed6dd17f687fcbfb0486ecc433302287
(https://github.com/bitcoin/bitcoin/pull/29690#pullrequestreview-1985077942)
Re-ACK 78407b99ed6dd17f687fcbfb0486ecc433302287
👍 fanquake approved a pull request: "clang-tidy: Enable misc-no-recursion"
(https://github.com/bitcoin/bitcoin/pull/29690#pullrequestreview-1985080437)
ACK 78407b99ed6dd17f687fcbfb0486ecc433302287
(https://github.com/bitcoin/bitcoin/pull/29690#pullrequestreview-1985080437)
ACK 78407b99ed6dd17f687fcbfb0486ecc433302287
📝 fanquake opened a pull request: "guix: remove `gcc-toolchain static` from Windows build"
(https://github.com/bitcoin/bitcoin/pull/29828)
The libs in this dir are the following:
```bash
ls /gnu/store/2vnbkrdin4rrf7ygnr80mlcglin4qqa4-gcc-toolchain-12.3.0-static/lib/
libanl.a libc.a libdl.a libm.a
libBrokenLocale.a libcrypt.a libg.a libmcheck.a
libpthread.a librt.a
libresolv.a libutil.a
```
These do not need to be propogated into the Windows build environment.
Guix Build (aarch64):
```bash
450c0c4f45f9cb7ed7fc2ef6e7557b6a23004b82c951399da3b7635e8451a076 gui
...
(https://github.com/bitcoin/bitcoin/pull/29828)
The libs in this dir are the following:
```bash
ls /gnu/store/2vnbkrdin4rrf7ygnr80mlcglin4qqa4-gcc-toolchain-12.3.0-static/lib/
libanl.a libc.a libdl.a libm.a
libBrokenLocale.a libcrypt.a libg.a libmcheck.a
libpthread.a librt.a
libresolv.a libutil.a
```
These do not need to be propogated into the Windows build environment.
Guix Build (aarch64):
```bash
450c0c4f45f9cb7ed7fc2ef6e7557b6a23004b82c951399da3b7635e8451a076 gui
...
💬 TheCharlatan commented on pull request "guix: remove `gcc-toolchain static` from Windows build":
(https://github.com/bitcoin/bitcoin/pull/29828#issuecomment-2041477098)
Nice, Concept ACK
(https://github.com/bitcoin/bitcoin/pull/29828#issuecomment-2041477098)
Nice, Concept ACK