Bitcoin Core Github
44 subscribers
121K links
Download Telegram
📝 fanquake opened a pull request: "ci: allow for any libc++ intrumentation & use it for TSAN"
(https://github.com/bitcoin/bitcoin/pull/33099)
Allow for instrumenting libc++ with a sanitizer other than MemoryWithOrigins.

Would also close #33087, as with the extra instrumentation, the issue from https://github.com/bitcoin/bitcoin/pull/32862#issuecomment-3114706601 is avoided (also see https://github.com/bitcoin/bitcoin/pull/33081), and we can drop `DEBUG_LOCKORDER`.
👋 fanquake's pull request is ready for review: "ci: allow for any libc++ intrumentation & use it for TSAN"
(https://github.com/bitcoin/bitcoin/pull/33099)
💬 fanquake commented on pull request "rpc: use CScheduler for relocking wallet and remove RPCTimer":
(https://github.com/bitcoin/bitcoin/pull/32862#issuecomment-3135990566)
> the problem seems to be that libc++ itself isn't instrumented.

Following up on this in #33099.
📝 fanquake opened a pull request: "ci: remove `ninja-build` from MSAN jobs"
(https://github.com/bitcoin/bitcoin/pull/33100)
It is part of CI_BASE_PACKAGES.
👍 hebasto approved a pull request: "ci: remove `ninja-build` from MSAN jobs"
(https://github.com/bitcoin/bitcoin/pull/33100#pullrequestreview-3071150626)
ACK cab6736b701f203d6e823e1b5d619368d8d4c5e0, I have reviewed the code and it looks OK.
💬 ajtowns commented on pull request "Move `FreespaceChecker` class into its own module":
(https://github.com/bitcoin-core/gui/pull/881#issuecomment-3136166736)
ACK 3a03f075606b19e411b8bd19870242e0e0b58fcb
📝 hebasto opened a pull request: "cmake: Proactively avoid use of `SECP256K1_DISABLE_SHARED`"
(https://github.com/bitcoin/bitcoin/pull/33101)
The `SECP256K1_DISABLE_SHARED` CMake variable has been [removed](https://github.com/bitcoin-core/secp256k1/pull/1688) upstream.

This PR removes its usage ahead of the next `secp256k1` subtree update to prevent breakage and simplify integration.
📝 brunoerg opened a pull request: "fuzz: cover BanMan::IsDiscouraged"
(https://github.com/bitcoin/bitcoin/pull/33102)
This PR adds fuzz coverage for the `IsDiscouraged` function in the banman target. This is the only function missing from `BanMan`.
💬 maflcko commented on pull request "fuzz: cover BanMan::IsDiscouraged":
(https://github.com/bitcoin/bitcoin/pull/33102#issuecomment-3136269998)
lgtm ACK c2ed576d2caf282a51aa9df5df55d844ab1da794
💬 TheBlueMatt commented on pull request "[POC] wallet: Add Support for BIP-353 DNS-Based Bitcoin Address via External Resolver":
(https://github.com/bitcoin/bitcoin/pull/33069#issuecomment-3136293939)
> The multiprocess binaries are produced by this project, built using the same deterministic build system, using the same code as the monolithic binaries. There are already concrete ways that the binaries can be distributed with minimal changes to our own release process. Users who run the multiprocess binaries don't have to do any special configuration for them to work.

Right, I was ancitipating that, ultimately, an external-process DNS resolution thing would be built and dstributed the same
...
💬 maflcko commented on pull request "ci: allow for any libc++ intrumentation & use it for TSAN":
(https://github.com/bitcoin/bitcoin/pull/33099#discussion_r2242662552)
```suggestion
export USE_INSTRUMENTED_LIBCPP="Thread"
```

Can flatten the option into a single one?

Also, adjust the check below to

```sh
if [ -n "${USE_INSTRUMENTED_LIBCPP}" ]; then
💬 maflcko commented on pull request "ci: allow for any libc++ intrumentation & use it for TSAN":
(https://github.com/bitcoin/bitcoin/pull/33099#issuecomment-3136330481)
lgtm, but the tsan depends build seems to be taking significantly longer now for some reason?
💬 fanquake commented on pull request "ci: allow for any libc++ intrumentation & use it for TSAN":
(https://github.com/bitcoin/bitcoin/pull/33099#issuecomment-3136334394)
> lgtm, but the tsan depends build seems to be taking significantly longer now for some reason?

Yea, that QT build has started taking a ridiculous amount of time. Maybe we can just disable it in this job?
🤔 marcofleon reviewed a pull request: "fuzz: cover BanMan::IsDiscouraged"
(https://github.com/bitcoin/bitcoin/pull/33102#pullrequestreview-3071548512)
ACK c2ed576d2caf282a51aa9df5df55d844ab1da794
💬 maflcko commented on pull request "ci: allow for any libc++ intrumentation & use it for TSAN":
(https://github.com/bitcoin/bitcoin/pull/33099#issuecomment-3136364439)
Yeah, I guess it may be best to disable it for now, if the instrumented libc++ turns the qt depends build into a time sink. For reference, the task never in history took that long, looking at https://0xb10c.github.io/bitcoin-core-ci-stats/graph/build/#TSan,%20depends,%20gui.

Obviously, it would be good to retain depends-qt-tsan as a nightly CI task, but I can open a general brainstorming issue about it.
🚀 fanquake merged a pull request: "fuzz: cover BanMan::IsDiscouraged"
(https://github.com/bitcoin/bitcoin/pull/33102)
💬 fanquake commented on pull request "ci: allow for any libc++ intrumentation & use it for TSAN":
(https://github.com/bitcoin/bitcoin/pull/33099#discussion_r2242730767)
Squashed this down.
💬 fanquake commented on pull request "ci: allow for any libc++ intrumentation & use it for TSAN":
(https://github.com/bitcoin/bitcoin/pull/33099#issuecomment-3136406144)
Squashed into a single option. Should have fixed the linter. Dropped Qt.
💬 maflcko commented on pull request "ci: Only pass documented env vars":
(https://github.com/bitcoin/bitcoin/pull/33002#discussion_r2242739108)
Thx, done in a move-only doc-comment-only force push
🤔 marcofleon reviewed a pull request: "[28.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33076#pullrequestreview-3071658257)
lgtm ACK 4d145f9f20a333cbe22579db525292e956af66c9