💬 stratospher commented on pull request "test: Fix intermittent failure in p2p_v2_misbehaving.py":
(https://github.com/bitcoin/bitcoin/pull/30420#discussion_r1674409130)
good idea, will push an update soon.

🚀 fanquake merged a pull request: "refactor: Use designated initializer in test/util/net.cpp"
(https://github.com/bitcoin/bitcoin/pull/30397)

💬 fanquake 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-2223521496)
https://cirrus-ci.com/task/5777533663182848?logs=ci#L9969

💬 darosior commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674425730)
nit stack overflow: @darosior crashed and won't be able to process any further nit

💬 stickies-v commented on pull request "rpc: Use CHECK_NONFATAL over Assert":
(https://github.com/bitcoin/bitcoin/pull/30429#discussion_r1674427363)
Also, when I run this new `lint-assertions.py` on the old `rpc/blockchain.cpp` it still doesn't seem to catch the issue, I think because of the `\<` prefix? `r"(A|a)ss(ume|ert)\(",` works fine for me.

🤔 stickies-v reviewed a pull request: "rpc: Use CHECK_NONFATAL over Assert"
(https://github.com/bitcoin/bitcoin/pull/30429#pullrequestreview-2172564813)
Approach ACK. Code fixes fa8b587b91dfa8df28fc13589c511b871902670b LGTM but I think the linter needs some further changes.

💬 stickies-v commented on pull request "rpc: Use CHECK_NONFATAL over Assert":
(https://github.com/bitcoin/bitcoin/pull/30429#discussion_r1674412838)
I think assuming no whitespace between a macro and the parenthesis is a regression, since that's allowed? (same for `BOOST_ASSERT`)

```cpp
#include <iostream>
#include <cstdlib>

#define Assume(x) do { if (!(x)) std::terminate(); } while(0)

int main() {
Assume(true); // Without space
Assume (true); // With space

std::cout << "Both assertions passed." << std::endl;
return 0;
}
```

🚀 fanquake merged a pull request: "util: Catch translation string errors at compile time"
(https://github.com/bitcoin/bitcoin/pull/30383)

🚀 fanquake merged a pull request: "Add clang-tidy check for thread_local vars"
(https://github.com/bitcoin/bitcoin/pull/30146)

🚀 fanquake merged a pull request: "refactor: modernize-use-equals-default"
(https://github.com/bitcoin/bitcoin/pull/30406)

💬 darosior commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674457669)
It's in doc/descriptors.md but sure i'll expand a bit in a comment.

📝 theuni opened a pull request: "depends: bump boost to 1.85.0 and use new CMake buildsystem"
(https://github.com/bitcoin/bitcoin/pull/30434)
Marked as draft because this is much more relevant after we've switched to CMake.

This has a few advantages over the old method of simply copying headers:
- Installs proper cmake files which can be picked up by our buildsystem
- Only installs necessary headers, not all of boost

The only drawback is that it builds a few libs that we end up throwing away. date_time and test can both be optionally used header-only (which we do), but boost's CMake buildsystem doesn't expose an option to skip
...

🚀 fanquake merged a pull request: "Enable clang-tidy checks for self-assignment"
(https://github.com/bitcoin/bitcoin/pull/30234)

💬 theuni commented on pull request "depends: bump boost to 1.85.0 and use new CMake buildsystem":
(https://github.com/bitcoin/bitcoin/pull/30434#issuecomment-2223618172)
Ping @hebasto. Tested on `cmake-staging` with `Boost_NO_BOOST_CMAKE` removed. Seems to work as expected. Though ofc we wouldn't be able to remove that until we require boost 1.82 for non-depends builds.

💬 theuni commented on pull request "depends: update doc in Qt pwd patch":
(https://github.com/bitcoin/bitcoin/pull/30336#issuecomment-2223626277)
Nice, utACK. Any reason not to just take their full patch, though? Guessing it doesn't apply cleanly?

💬 fanquake commented on pull request "depends: update doc in Qt pwd patch":
(https://github.com/bitcoin/bitcoin/pull/30336#issuecomment-2223639910)
Yea, iirc. Also didn't want to change anything else here, and possibly break anything else, given we don't need any other changes.

💬 darosior commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674506073)
More seriously: i considered doing it but it would not match the existing style for `HasDeepDerivPath` and would require duplicating the `MAX_SUBS` and `MAX_WRAPPERS` args (once in descriptor and once in descriptor_parse). So IMO it would be less neat and therefore i didn't do it.

🤔 theuni reviewed a pull request: "contrib: simplify `test-security-check`"
(https://github.com/bitcoin/bitcoin/pull/30423#pullrequestreview-2172750410)
Nice, concept ACK. I had the same thought when looking at this for #30387.

I think it should be possible (or maybe this is enough?) to avoid making toolchain assumptions and have this work outside of guix.

👍 theuni approved a pull request: "depends: update doc in Qt pwd patch"
(https://github.com/bitcoin/bitcoin/pull/30336#pullrequestreview-2172752965)
ACK f170fe04ca03fe4021cbff7c5450ce3cc7fda17f

💬 achow101 commented on pull request "test: fix inconsistency in fundrawtransaction weight limits test":
(https://github.com/bitcoin/bitcoin/pull/30353#issuecomment-2223661762)
ACK 00b8e26bd6a47a8207c7571ca7ef9643f0ee2668