💬 l0rinc commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1846343700)
Looking at the actual usage, `ExpectSuccess` doesn't need to be a variadic template function:
```C++
ExpectSuccess(IntFn(5, true), {}, 5);
ExpectSuccess(NoCopyFn(5, true), {}, 5);
ExpectSuccess(StrFn(Untranslated("S"), true), {}, Untranslated("S"));
```
, we might as well simplify to something like:
```C++
template <typename T, typename U>
void ExpectSuccess(const util::Result<T>& result, const bilingual_str& str, U&& expected)
{
ExpectResult(result, true, str);
BOOST_CHECK_E
...

💬 m3dwards commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#issuecomment-2482647517)
Concept ACK

I like the motivation to cleanup the logs and remove the cascade of exceptions.

To test, I modified the included test to see what errors a user would have seen without the assertion:

```diff
--- a/test/functional/feature_framework_rpc_failure.py
+++ b/test/functional/feature_framework_rpc_failure.py
@@ -23,7 +23,8 @@ class FeatureFrameworkRPCFailure(BitcoinTestFramework):
self.log.info(f"Setting RPC timeout to 0 to simulate an unresponsive bitcoind, expect one w
...

💬 l0rinc commented on pull request "refactor: Clamp worker threads in ChainstateManager constructor":
(https://github.com/bitcoin/bitcoin/pull/31313#discussion_r1846350891)
Is script verification completely CPU bound? If so, this limit may still be reasonable for a few more years - but unlike in 2013, it's not an unreasonably big value.

💬 TheCharlatan commented on pull request "refactor: Clamp worker threads in ChainstateManager constructor":
(https://github.com/bitcoin/bitcoin/pull/31313#issuecomment-2482649498)
Updated 09c9ba846ec44ba903de2b32a1cb0f49c7f93cb0 -> 8f85d36d68ab33ba237407a2ed16667eb149d61f ([chainstatemanagerArgs_0](https://github.com/TheCharlatan/bitcoin/tree/chainstatemanagerArgs_0) -> [chainstatemanagerArgs_1](https://github.com/TheCharlatan/bitcoin/tree/chainstatemanagerArgs_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainstatemanagerArgs_0..chainstatemanagerArgs_1))

* Addressed @maflcko's [comment](https://github.com/bitcoin/bitcoin/pull/31313#discussion_r1846233
...

💬 l0rinc commented on pull request "refactor: Clamp worker threads in ChainstateManager constructor":
(https://github.com/bitcoin/bitcoin/pull/31313#discussion_r1846353272)
Do we expect [every call-site](https://github.com/bitcoin/bitcoin/pull/31313#discussion_r1846266844) to do the clamping themselves?

✅ BrandonOdiwuor closed a pull request: "test: run functional tests from ctest"
(https://github.com/bitcoin/bitcoin/pull/31312)

💬 TheCharlatan commented on pull request "refactor: Clamp worker threads in ChainstateManager constructor":
(https://github.com/bitcoin/bitcoin/pull/31313#discussion_r1846366581)
I don't think this really matters, but seems consistent to control the arguments in the same place, just like with the batch size.

🤔 BrandonOdiwuor reviewed a pull request: "Bugfix: Correct first-run free space checks"
(https://github.com/bitcoin/bitcoin/pull/29678#pullrequestreview-2442281522)
Concept ACK using `GB` to represent 10<sup>3</sup> bytes for consistency with GUI

💬 Sjors commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#discussion_r1846404405)
435934fa020cec584b49c6151aa6b0d858054b37 oops, that should have been `&&`

💬 Sjors commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#issuecomment-2482738819)
Pushed a bug fix, but still need to address the main feedback.

💬 maflcko commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#issuecomment-2482760476)
Also, it would be good to add test coverage?

💬 maflcko commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1846425672)
re https://github.com/bitcoin/bitcoin/pull/30660#discussion_r1834551086

Are you still working on this?

✅ maflcko closed a pull request: "nomerge: test malicious bidi char"
(https://github.com/bitcoin/bitcoin/pull/31314)

💬 Sjors commented on pull request "Add multiprocess binaries to release build":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2482785475)
I disabled ARM and macOS-cross as well, added a link to their failures to PR description TODO.

I think the `p2p_i2p_ports.py` failure in the previous releases job is spurious, so leaving that job in.

💬 Sjors commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#issuecomment-2482789520)
@maflcko it's implicitly tested by the shutdown test, which is how I found this bug. Not sure how to explicitly test an early return?

💬 maflcko commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#issuecomment-2482799390)
Are you sure it is covered, because the code is reported as uncovered in https://corecheck.dev/bitcoin/bitcoin/pulls/31297?

💬 Sjors commented on pull request "mining: add early return to waitTipChanged()":
(https://github.com/bitcoin/bitcoin/pull/31297#issuecomment-2482804679)
@maflcko `feature_shutdown.py` only check that the early return is _not_ used. It makes sense to test the scenario where it is used too.

💬 maflcko commented on issue "Intermittent issue in p2p_i2p_ports.py AssertionError: [node 0] Expected messages "['Error connecting to [...].b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not partially match log:":
(https://github.com/bitcoin/bitcoin/issues/30030#issuecomment-2482805416)
https://cirrus-ci.com/task/5063913392308224?logs=ci#L6167

💬 maflcko commented on pull request "test: Fix RANDOM_CTX_SEED use with parallel tests":
(https://github.com/bitcoin/bitcoin/pull/30737#issuecomment-2482830018)
For reference, the fuzz issue can be hit more frequent on machines that do not have nanosecond precision. GCE may not have it, according to the oss-fuzz crashes of the form:

```
libc++abi: terminating due to uncaught exception of type std::__1::__fs::filesystem::filesystem_error: filesystem error: in remove_all: No such file or directory ["/tmp/test_common bitcoin/1731742732339168000/regtest/fuzzed_i2p_private_key"]
```

Ref: https://oss-fuzz.com/testcase-detail/5425475725361152

https:
...

💬 willcl-ark commented on pull request "Add `contrib/justfile` containing useful development workflow commands.":
(https://github.com/bitcoin/bitcoin/pull/31292#issuecomment-2482850113)
I really enjoy making justfiles for projects I work on.

And, whilst I would love to see some kind of community justfile added to this project, I can appreciate the hesitation to do so per the rationale given by @achow101 above.

I've therefore taken a different approach myself, hosting a justfile at [https://github.com/bitcoin-dev-tools/dotfiles](https://github.com/bitcoin-dev-tools/dotfiles/blob/master/justfile) which is designed to sit one directory higher than this repository's code, spe
...