The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33411.

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

The runtime isn't too bad, and it's just blown out by wallet_miniscript.py, which is ~4 times slower than the next slowest running test (signet_miner.py):

tool_signet_miner.py                                   | ✓ Passed  | 536 s
wallet_miniscript.py                                   | ✓ Passed  | 2086 s

With wallet_miniscript skipped, this finishes faster than the ASAN job, and ~roughly the same time as msan & fuzzer.

Concept ACK to re-introducing this job.

Wondering whether it's worth adding an extra runner if we add a new lg-sized job? On the whole CI queue sizes/times have been reasonable I think since the migration, and we did reduce a few job sizes recently. But still might be worth considering.

The 16th seemed unusually active, most days top out around 20 min max queue time AFAIK, but for reference here's the previous 7 days:

Adjusted to remove the conflict with #31425 and add context to the final commit.

No objection, but are there any real issues that this task has found in the last years? The only ones I am aware of are false-positives around valgrind and questions around updating the task config itself for a newly added package. If those are the only reasons to add it, the reasoning seems circular and pull request authors having to deal with valgrind false positives directly in their pull requests may even be counter-productive? For example, https://github.com/bitcoin/bitcoin/pull/31282/files was open for 4 months, and #29635 is still open with no clear solution. I am not sure if it is beneficial to block or delay a perfectly good pull requests based on a false-positive red valgrind CI.

Again, this seems fine, and I don't want to raise an objection. However, the general question of how to deal with the false-positives (see my previous comment) here is still unanswered. IIUC, valgrind may not work under a given compiler (version). For example, currently it doesn't work under clang out of the box. There are workarounds needed, but they come with drawbacks:

• Add a suppression, but if this one is too broad, it decreases the value of running valgrind
• Use -O1, but valgrind is already slow, and this will make it even slower
• Refactor the code somehow randomly, until it isn't hit, but that is tedious and unclear, because it isn't trivially actionable

Generally, I have the impression that the existing asan and msan task find all memory issues before valgrind could find them. If there was one, it would be good to know it.

Can leave it to *san for now.

Maybe we can add it to just run under the default branch (master) on merges? Brought up in the context of #33509 (comment), but i guess the same approach could be used for several tasks? Maybe a brainstorming issue?