💬 darosior commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674383009)
Taproot leaves. It's to avoid overcounting if for instance you have `tr(d971308d5463ddc82addd897f6ebfa8a88e86de0664a66b9e9cf5e873459c528,{dvdvdvdvdv:1,1))`.
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674383009)
Taproot leaves. It's to avoid overcounting if for instance you have `tr(d971308d5463ddc82addd897f6ebfa8a88e86de0664a66b9e9cf5e873459c528,{dvdvdvdvdv:1,1))`.
💬 mzumsande commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#issuecomment-2223477561)
[b715a13 ](https://github.com/bitcoin/bitcoin/commit/b715a13d11c6ca7e928b2dee66edcc8c6c30a8d4)to [55b6d7b](https://github.com/bitcoin/bitcoin/commit/55b6d7be68a6f6c3882588ffd5b9349d885ed953): rebased
(https://github.com/bitcoin/bitcoin/pull/30320#issuecomment-2223477561)
[b715a13 ](https://github.com/bitcoin/bitcoin/commit/b715a13d11c6ca7e928b2dee66edcc8c6c30a8d4)to [55b6d7b](https://github.com/bitcoin/bitcoin/commit/55b6d7be68a6f6c3882588ffd5b9349d885ed953): rebased
💬 stickies-v commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674385763)
Okay, thanks. No strong opinion as to which approach, but since I couldn't find any reference to it perhaps good to add this example as a docstring in this else if path?
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674385763)
Okay, thanks. No strong opinion as to which approach, but since I couldn't find any reference to it perhaps good to add this example as a docstring in this else if path?
💬 m3dwards commented on pull request "net: Allow -proxy=[::1] on nodes with IPV6 lo only":
(https://github.com/bitcoin/bitcoin/pull/30245#discussion_r1674387539)
If pursuing this approach do we think it would be prudent to reset `ai_res` to a nullptr before the second call?
(https://github.com/bitcoin/bitcoin/pull/30245#discussion_r1674387539)
If pursuing this approach do we think it would be prudent to reset `ai_res` to a nullptr before the second call?
💬 stickies-v commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674388962)
Or alternatively, add a separate else if branch for ignored characters (e.g. "{"), just for clarity?
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1674388962)
Or alternatively, add a separate else if branch for ignored characters (e.g. "{"), just for clarity?
💬 theuni commented on pull request "contrib: use c++ compiler rather than c compiler for binary checks":
(https://github.com/bitcoin/bitcoin/pull/30387#discussion_r1674399301)
GRRRRR!!!
(https://github.com/bitcoin/bitcoin/pull/30387#discussion_r1674399301)
GRRRRR!!!
💬 ismaelsadeeq commented on issue "ci: failure in `wallet_fundrawtransaction.py --legacy-wallet`":
(https://github.com/bitcoin/bitcoin/issues/30432#issuecomment-2223506811)
This issue is fixed by #30353
(https://github.com/bitcoin/bitcoin/issues/30432#issuecomment-2223506811)
This issue is fixed by #30353
💬 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.
(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)
(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
(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
(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.
(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.
(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;
}
```
(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)
(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)
(https://github.com/bitcoin/bitcoin/pull/30146)
🚀 fanquake merged a pull request: "refactor: modernize-use-equals-default"
(https://github.com/bitcoin/bitcoin/pull/30406)
(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.
(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
...
(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)
(https://github.com/bitcoin/bitcoin/pull/30234)