💬 fanquake commented on issue "ci: Lower and unify default stack size":
(https://github.com/bitcoin/bitcoin/issues/29840#issuecomment-3118322437)
Any opinions on doing this via link flags? We can instruct the linker to set stack size limits on the executable during linking. One advantage might be that we can set varying limits, per build, but that might not be useful.

If not, I'll pickup the ulimit approach.

👍 darosior approved a pull request: "[29.x] final changes for v29.1rc1"
(https://github.com/bitcoin/bitcoin/pull/33056#pullrequestreview-3055779227)
ACK 4c2d285b706058ece7092d03f0e5ad1112c2097f

✅ Sjors closed a pull request: "subprocess: always check result of close()"
(https://github.com/bitcoin/bitcoin/pull/33061)

💬 Sjors commented on pull request "subprocess: always check result of close()":
(https://github.com/bitcoin/bitcoin/pull/33061#issuecomment-3118343686)
It appears that underlying issue is now understood to be signal safety, see #33063. So improving `close` error reporting and handling isn't a priority.

💬 maflcko commented on pull request "util: Revert "common: Close non-std fds before exec in RunCommandJSON"":
(https://github.com/bitcoin/bitcoin/pull/33063#issuecomment-3118364788)
> it's just `throw` and `sysconf` that are the problem?

No, all of the C++ standard libary is problematic as well. (See the pull description and the bt)



> How do you verify a reverted merge commit? I guess this worked:

I typed `git revert -m1 31d3eebfb92ae0521e18225d69be95e78fb02672`, but `git` is very flexible and you can type many different things to arrive at the same result.

💬 darosior commented on pull request "util: Revert "common: Close non-std fds before exec in RunCommandJSON"":
(https://github.com/bitcoin/bitcoin/pull/33063#issuecomment-3118381836)
Concept ACK...

👍 darosior approved a pull request: "util: Revert "common: Close non-std fds before exec in RunCommandJSON""
(https://github.com/bitcoin/bitcoin/pull/33063#pullrequestreview-3055837132)
ACK faa1c3e80d95552bdc2c0e717065ebf8d510138f

💬 jonatack commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#discussion_r2231370673)
Done

💬 jonatack commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#discussion_r2231372594)
Reworked it 👍

🤔 mzumsande reviewed a pull request: "p2p: rename GetAddresses -> GetAddressesUnsafe"
(https://github.com/bitcoin/bitcoin/pull/32994#pullrequestreview-3055853811)
Code review ACK 1cb23997033c395d9ecd7bf2f54787b134485f41

🚀 fanquake merged a pull request: "p2p: rename GetAddresses -> GetAddressesUnsafe"
(https://github.com/bitcoin/bitcoin/pull/32994)

💬 fanquake commented on pull request "[29.x] final changes for v29.1rc1":
(https://github.com/bitcoin/bitcoin/pull/33056#issuecomment-3118510216)
ACK 4c2d285b706058ece7092d03f0e5ad1112c2097f

🚀 fanquake merged a pull request: "[29.x] final changes for v29.1rc1"
(https://github.com/bitcoin/bitcoin/pull/33056)

💬 pinheadmz commented on pull request "rpc: use CScheduler for relocking wallet and remove RPCTimer":
(https://github.com/bitcoin/bitcoin/pull/32862#issuecomment-3118790215)
I'm unable to reproduce so far building master at 6cdc5a90cffe9ced4ec50d2028c9896d25a9cb6a


on Debian:

`cmake -B ./bld-cmake -DCMAKE_C_COMPILER='clang-16' -DCMAKE_CXX_COMPILER='clang++-16' -DCMAKE_CXX_FLAGS='-std=c++20' -DBUILD_GUI=OFF -DBUILD_TESTS=OFF -DSANITIZERS=thread && cmake --build ./bld-cmake --parallel $( nproc ) && TSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/tsan:halt_on_error=1:second_deadlock_stack=1" ./bld-cmake/test/functional/wallet_multiwallet.py`

on
...

💬 olegrok commented on issue "ci: don't pass GIT_EXEC_PATH inside test container":
(https://github.com/bitcoin/bitcoin/issues/32935#issuecomment-3119155140)
> [@olegrok](https://github.com/olegrok) If you want, you can test [#33002](https://github.com/bitcoin/bitcoin/pull/33002)

It looks a bit tricky (however I can't suggest solution that will be better) but this fixes an issue.

💬 ishaanam commented on pull request "package validation: relax the package-not-child-with-unconfirmed-parents rule":
(https://github.com/bitcoin/bitcoin/pull/31385#issuecomment-3119410214)
re-utACK ea17a9423fb431a86d36927b02d3624f654fd867

I used range_diff to compare to my previous review. The changes were just to address review comments and rebase.

📝 theStack opened a pull request: "rpc, wallet: replace remaining hardcoded output types with `FormatAllOutputTypes`"
(https://github.com/bitcoin/bitcoin/pull/33065)
This PR takes use of the `FormatAllOutputTypes` helper (introduced in PR #32432, commit 8cc9845b8ddf4f93a02c622e7df8d1095dc1a640) to get rid of the remaining hardcoded output types in wallet RPC documentation [1]. Note that it can't be used in the [`createmultisig` RPC](https://github.com/bitcoin/bitcoin/blob/fc162299f0cc5b61bbb55736fe85e27b705f78cc/src/rpc/output_script.cpp#L100), as this one is only for pre-taproot output types and hence doesn't contain "bech32m" in the list.

[1] instances
...

🤔 jonatack reviewed a pull request: "ci: Run unit test parallel with functional tests"
(https://github.com/bitcoin/bitcoin/pull/33000#pullrequestreview-3056247871)
I don't know how much variance we typically see in CI run times, but light lgtm ACK fab25c6f5803ae8699f068d089f1c6e4f1387743

```
$ /test_runner.py -h
--valgrind run nodes under the valgrind memory error detector: expect at least a ~10x slowdown. valgrind 3.14 or later required. Does not apply to previous release
binaries.
--bash-cmd-extra-tests BASH_CMD_EXTRA_TESTS
Run an arbitrary string as Bash command in parallel to the fu
...

🤔 LarryRuane reviewed a pull request: "test: revive test verifying that `GetCoinsCacheSizeState` switches from OK→LARGE→CRITICAL"
(https://github.com/bitcoin/bitcoin/pull/33021#pullrequestreview-3056300082)
tACK b2d82668a57d4ab4eef310c25a2aebda18563677
I thoroughly reviewed and understand all code changes. This test perfectly tests `GetCoinsCacheSizeState()` and is much simpler and less fragile. (Also, I learned a few C++ tricks, very nice.)

I ran the modified test using the debugger (on Ubuntu), and it behaves as designed. The only very minor suggestion if you need to retouch is to consider adding a comment explaining why it's not necessary to verify that the loops didn't exceed `MAX_ATTEMPTS`
...

💬 glozow commented on pull request "p2p: stop special-casing witness-stripped error for unconfirmed transactions":
(https://github.com/bitcoin/bitcoin/pull/32379#issuecomment-3119690432)
I ran the above patch for a while and added logging to collect some stats and see how much more redownloading we’d do. TLDR it seems like we can definitely go ahead with it.

Caveats: Traffic has been very low so I set my mempool expiry to 1 hour to try to induce more orphan-fetching. I’m a default policy node so I don’t really expect to be relayed many transactions I’ll reject. There isn’t much opportunity to test reconsiderable rejections right now because of how low transaction traffic is.
...