💬 maflcko commented on pull request "ci: Run unit tests parallel with functional tests":
(https://github.com/bitcoin/bitcoin/pull/33000#issuecomment-3158236096)
> Concept ACK. Not sure about the approach of more Bash, to call new Python, that wraps more Bash.

Yeah, it is ugly. I think I'll drop some of the (actually redundant) Bash in https://github.com/bitcoin/bitcoin/pull/33136 first. Then, this can be done in pure Python, possibly.

💬 fanquake commented on pull request "Add Docker support with multi-arch build and user permissions handling":
(https://github.com/bitcoin/bitcoin/pull/33139#issuecomment-3158289885)
Concept NACK. The benefits of this are unclear, this seems to just add a new dependency/abstraction layer (for development?), without solving any specific problem. It's also trying to do three things at once, and there already exist maintained Dockerfiles, for the deployment scenario, i.e https://github.com/willcl-ark/bitcoin-core-docker.

> developers can edit the Dockerfile or mount their source with custom flags.

This is more work than just using CMake and setting options?

> Provide a
...

💬 maflcko commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-3158322672)
> Ah, I guess on my machine, these jobs are the slowest:
>
> ```
> 143/144 Test #4: secp256k1_tests ...................... Passed 217.55 sec
> 144/144 Test #6: bench_sanity_check ................... Passed 433.08 sec
> ```
>
> In the ci environment, we see this:
>
> ```
> [17:30:48.523] 143/144 Test #4: secp256k1_tests ...................... Passed 1134.59 sec
> [17:51:54.170] 144/144 Test #6: bench_sanity_check ...................***Timeout 2400.23 sec
> ```

I
...

💬 maflcko commented on issue "Slow unit tests delay functional tests and leave CPU unused":
(https://github.com/bitcoin/bitcoin/issues/32770#issuecomment-3158330108)
`bench_sanity_check` could make sense to split up as well. With sanitizers enabled, it takes several minutes, even on fast hardware. See https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-3146639801. So I created https://github.com/bitcoin/bitcoin/pull/33142

💬 maflcko commented on pull request "Add Docker support with multi-arch build and user permissions handling":
(https://github.com/bitcoin/bitcoin/pull/33139#issuecomment-3158495632)
> > developers can edit the Dockerfile or mount their source with custom flags.
>
> This is more work than just using CMake and setting options?

Yeah, for devs it could make sense to have an image with just the build deps, similar to the lint ci image:

```
DOCKER_BUILDKIT=1 docker build -t bitcoin-core-build-deps --file "./contrib/dev-build-deps_imagefile" ./ && docker run --rm -v $(pwd):/bitcoin -it bitcoin-core-build-deps
... then run cmake, etc...
```

However, I am not sure if
...

🤔 fanquake reviewed a pull request: "ci: Pass CI_FAILFAST_TEST_LEAVE_DANGLING into container"
(https://github.com/bitcoin/bitcoin/pull/33138#pullrequestreview-3091670370)
ACK fa1d2f63803e71d21b470cf4eb52fbe941aa0b28

🚀 fanquake merged a pull request: "ci: Pass CI_FAILFAST_TEST_LEAVE_DANGLING into container"
(https://github.com/bitcoin/bitcoin/pull/33138)

🤔 fanquake reviewed a pull request: "cmake: Switch to generated `ts_files.cmake` file"
(https://github.com/bitcoin/bitcoin/pull/33115#pullrequestreview-3091739994)
ACK a26fbee38f95e71fbbeb6cf09e18ed7fff089ec8

✅ fanquake closed an issue: "cmake: Replace f`ile(GLOB ...)` command with an explicit list of `*.ts` files"
(https://github.com/bitcoin/bitcoin/issues/32653)

🚀 fanquake merged a pull request: "cmake: Switch to generated `ts_files.cmake` file"
(https://github.com/bitcoin/bitcoin/pull/33115)

💬 fanquake commented on issue "Slow unit tests delay functional tests and leave CPU unused":
(https://github.com/bitcoin/bitcoin/issues/32770#issuecomment-3158930887)
`coinselector_tests` is a good candidate for speedup, under Valgrind, it is the slowest after `bench_sanity_check`:
```bash
141/144 Test #78: random_tests ......................... Passed 227.62 sec
142/144 Test #30: coins_tests_dbbase ................... Passed 336.61 sec
143/144 Test #130: coinselector_tests ................... Passed 459.29 sec
144/144 Test #6: bench_sanity_check ................... Passed 793.02 sec
```

Same for `wallet_miniscript.py` in the functional tests
...

🚀 fanquake merged a pull request: "test: remove duplicated code in test/functional/wallet_migration.py"
(https://github.com/bitcoin/bitcoin/pull/33122)

🚀 fanquake merged a pull request: "refactor: Use immediate lambda to work around GCC bug 117966"
(https://github.com/bitcoin/bitcoin/pull/33113)

🚀 fanquake merged a pull request: "rpc: Fix 'getdescriptoractivity' RPCHelpMan, add test to verify fix"
(https://github.com/bitcoin/bitcoin/pull/33119)

💬 fanquake commented on pull request "rpc: Fix 'getdescriptoractivity' RPCHelpMan, add test to verify fix":
(https://github.com/bitcoin/bitcoin/pull/33119#issuecomment-3159184025)
Backported to `29.x` in #33074.

🤔 fanquake reviewed a pull request: "cmake: Proactively avoid use of `SECP256K1_DISABLE_SHARED`"
(https://github.com/bitcoin/bitcoin/pull/33101#pullrequestreview-3091896535)
ACK b093a19ae2eff306b8ed6ce74d133e3ec53ef64e

🚀 fanquake merged a pull request: "cmake: Proactively avoid use of `SECP256K1_DISABLE_SHARED`"
(https://github.com/bitcoin/bitcoin/pull/33101)

💬 ajtowns commented on pull request "p2p: never check tx rejections by txid":
(https://github.com/bitcoin/bitcoin/pull/33066#issuecomment-3159450863)
> I still think this PR is preferable to #33105. The latter will inevitably introduce more false-positive witness stripped errors,

I don't think false-positive witness stripped errors are harmful, apart from the CPU they required to be detected in the first place.

Only "misbehaving" peers will send us witness stripped txs in the first place, so the only benefit caching an error does (which is what we'd do if we replaced the "false" witness stripped result with something else) is only to pr
...

💬 fanquake commented on pull request "cmake: Proactively avoid use of `SECP256K1_DISABLE_SHARED`":
(https://github.com/bitcoin/bitcoin/pull/33101#issuecomment-3159505982)
Guix Build (aarch64):
```bash
1a4f3c804b729fdc5a3d4831239b6bd7e34e0ca9061395578dfce07ffa8ea1cf guix-build-b093a19ae2ef/output/aarch64-linux-gnu/SHA256SUMS.part
03563287d5fd2f87829e2569e2fc02e8e6c0f9dc3db22d196fe442b852b242f0 guix-build-b093a19ae2ef/output/aarch64-linux-gnu/bitcoin-b093a19ae2ef-aarch64-linux-gnu-debug.tar.gz
5aee57e7e430fe8c7d0214dcc5717a1e50992fa9a28252804658fe262d6cde0e guix-build-b093a19ae2ef/output/aarch64-linux-gnu/bitcoin-b093a19ae2ef-aarch64-linux-gnu.tar.gz
d61969
...

💬 pinheadmz commented on pull request "Add Docker support with multi-arch build and user permissions handling":
(https://github.com/bitcoin/bitcoin/pull/33139#issuecomment-3159533044)
concept NACK
willcl_ark's dockerhub repo is already outstanding: https://hub.docker.com/r/bitcoin/bitcoin even providing nightly builds!