💬 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

💬 darosior commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#issuecomment-2223670550)
Thank you both for the review. I addressed your comments and heavily documented the code. I think it can be helpful as it indeed relies on being familiar with the descriptor/miniscript syntax. If only to make the assumptions in the counting logic explicit, or for when we'll all have forgotten about the details of this syntax.

I also modified two small things:
- I introduced a limit to the number of nested sub-fragments a fragment may have. Since we are pushing an element on top of a stack fo
...

💬 maflcko commented on pull request "rpc: Use CHECK_NONFATAL over Assert":
(https://github.com/bitcoin/bitcoin/pull/30429#discussion_r1674529679)
> I think assuming no whitespace between a macro and the parenthesis is a regression, since that's allowed?

Yes, also a newline, but both are "forbidden" by clang-format. If you want to be more accurate you'll have to write a clang-tidy plugin, but I won't be doing this myself.



> I think because of the `\<` prefix?

Keep in mind that macOS is not supported. What is `grep --version | grep GNU` on your machine?

💬 mzumsande commented on issue "intermittent issue in feature_reindex.py: Reindex fails to continue after restart; `initload` thread hangs forever":
(https://github.com/bitcoin/bitcoin/issues/30424#issuecomment-2223676434)
I now have a hypothesis what might be happening. In `Shutdown()`, we first stop the scheduler thread and then the import blocks thread [here](https://github.com/bitcoin/bitcoin/blob/00feabf6c59c954c1e3a5a46e58cb80cd8d55911/src/init.cpp#L301-L302).
When the import blocks thread calls `ActivateBestChain()`, this can call `LimitValidationInterfaceQueue` [here](https://github.com/bitcoin/bitcoin/blob/00feabf6c59c954c1e3a5a46e58cb80cd8d55911/src/validation.cpp#L3487). In a case of unfortunate timing
...

💬 maflcko commented on pull request "wallet: BIP 326 sequence based anti-fee-snipe for taproot inputs":
(https://github.com/bitcoin/bitcoin/pull/24128#issuecomment-2223678147)
One-line force-push to relax the v2 assumption to allow v3/TRUC transactions, to avoid having to touch the code again in the future.

🚀 achow101 merged a pull request: "test: fix inconsistency in fundrawtransaction weight limits test"
(https://github.com/bitcoin/bitcoin/pull/30353)

✅ achow101 closed an issue: "ci: failure in `wallet_fundrawtransaction.py --legacy-wallet`"
(https://github.com/bitcoin/bitcoin/issues/30432)