π¬ l0rinc commented on pull request "build: Minimize I/O operations in GenerateHeaderFromRaw.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1757406164)
Done
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1757406164)
Done
π¬ maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2347020039)
I recreated a CI timeout and the machine's CPU was idle flat at 0% for most of the part for 2 hours, but I didn't look where it spent so much time "doing nothing".
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2347020039)
I recreated a CI timeout and the machine's CPU was idle flat at 0% for most of the part for 2 hours, but I didn't look where it spent so much time "doing nothing".
π¬ l0rinc commented on issue "cmake: GenerateHeaderFrom very slow":
(https://github.com/bitcoin/bitcoin/issues/30881#issuecomment-2347020740)
Fixed in https://github.com/bitcoin/bitcoin/pull/30888
(https://github.com/bitcoin/bitcoin/issues/30881#issuecomment-2347020740)
Fixed in https://github.com/bitcoin/bitcoin/pull/30888
π jonatack opened a pull request: "doc: unit test runner help fixup"
(https://github.com/bitcoin/bitcoin/pull/30890)
Running `test_bitcoin --help` prints the list of arguments that may be passed, not the list of tests, so fix that.
An optional second commit improves the clarity and organization of the "Running individual tests" section.
(https://github.com/bitcoin/bitcoin/pull/30890)
Running `test_bitcoin --help` prints the list of arguments that may be passed, not the list of tests, so fix that.
An optional second commit improves the clarity and organization of the "Running individual tests" section.
π¬ l0rinc commented on pull request "build: optimize .h generation in GenerateHeaderFrom{Raw,Json}.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#issuecomment-2347026457)
> it would be good to test this on Windows and Linux with the minimum and maximum supported cmake version to make sure this is really the last time this needs to be touched.
While the speed difference is algorithmic, shouldn't be OS dependent, I would appreciate if someone would do that.
> would be more consistent and easier to maintain to also transform the json one
pushed
(https://github.com/bitcoin/bitcoin/pull/30888#issuecomment-2347026457)
> it would be good to test this on Windows and Linux with the minimum and maximum supported cmake version to make sure this is really the last time this needs to be touched.
While the speed difference is algorithmic, shouldn't be OS dependent, I would appreciate if someone would do that.
> would be more consistent and easier to maintain to also transform the json one
pushed
π¬ ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1757414433)
> The argument list is really difficult to read due to the nature of the arguments. However, I understand that it can be no better than that anyway!
To be fair, I think this could be significantly simpler now that we are using c++20, which supports [concepts](https://en.cppreference.com/w/cpp/language/constraints). In general there should be no reason to use std::enable_if anymore now that concepts are available. There's a lot of older code like this that could be switched to use concepts, an
...
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1757414433)
> The argument list is really difficult to read due to the nature of the arguments. However, I understand that it can be no better than that anyway!
To be fair, I think this could be significantly simpler now that we are using c++20, which supports [concepts](https://en.cppreference.com/w/cpp/language/constraints). In general there should be no reason to use std::enable_if anymore now that concepts are available. There's a lot of older code like this that could be switched to use concepts, an
...
π¬ achow101 commented on pull request "test: Wait for local services to update in feature_assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/30880#issuecomment-2347033456)
Backported in #30827
(https://github.com/bitcoin/bitcoin/pull/30880#issuecomment-2347033456)
Backported in #30827
π maflcko approved a pull request: "build: optimize .h generation in GenerateHeaderFrom{Raw,Json}.cmake"
(https://github.com/bitcoin/bitcoin/pull/30888#pullrequestreview-2301157792)
lgtm
(https://github.com/bitcoin/bitcoin/pull/30888#pullrequestreview-2301157792)
lgtm
π¬ maflcko commented on pull request "build: optimize .h generation in GenerateHeaderFrom{Raw,Json}.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1757413579)
Same. `file(WRITE ${HEADER_PATH}...` should be self-documenting already.
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1757413579)
Same. `file(WRITE ${HEADER_PATH}...` should be self-documenting already.
π¬ maflcko commented on pull request "build: optimize .h generation in GenerateHeaderFrom{Raw,Json}.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1757418027)
The comment is wrong. There is no `std::byte` in a string's char type.
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1757418027)
The comment is wrong. There is no `std::byte` in a string's char type.
π¬ maflcko commented on pull request "build: optimize .h generation in GenerateHeaderFrom{Raw,Json}.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1757416946)
nit: Instead of describing what the code does, it would be better to explain *why* it does something, where relevant. Here, it could say that string-REGEX-REPLACE` was found to be faster than file-APPEND or string-APPEND. But just a style nit, up to you.
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1757416946)
nit: Instead of describing what the code does, it would be better to explain *why* it does something, where relevant. Here, it could say that string-REGEX-REPLACE` was found to be faster than file-APPEND or string-APPEND. But just a style nit, up to you.
π¬ maflcko commented on pull request "build: optimize .h generation in GenerateHeaderFrom{Raw,Json}.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#issuecomment-2347038904)
> > it would be good to test this on Windows and Linux with the minimum and maximum supported cmake version to make sure this is really the last time this needs to be touched.
>
> While the speed difference is algorithmic, shouldn't be OS dependent, I would appreciate if someone would do that.
Some people claimed that string-APPEND was faster on Windows than file-APPEND, but it was the inverse on Linux. So I think it would be good to check on both platforms. (I can do Linux, but not Window
...
(https://github.com/bitcoin/bitcoin/pull/30888#issuecomment-2347038904)
> > it would be good to test this on Windows and Linux with the minimum and maximum supported cmake version to make sure this is really the last time this needs to be touched.
>
> While the speed difference is algorithmic, shouldn't be OS dependent, I would appreciate if someone would do that.
Some people claimed that string-APPEND was faster on Windows than file-APPEND, but it was the inverse on Linux. So I think it would be good to check on both platforms. (I can do Linux, but not Window
...
β οΈ fabioBaraDev opened an issue: "No such file or directory: bitcoind Error"
(https://github.com/bitcoin/bitcoin/issues/30891)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo
- [X] I still think this issue should be opened here
### Report
Hi guys, I am using a macbook Sonoma 14.0 and I am following the compilation tutorial, when I tried to run the Functional tests, I get this error:
```
FileNotFoundError: [Errno 2] No such file or directory: '/Users/user/Documents/personalProjects/bitcoin/src/bitcoind'
```
I already tried to run `make clean` and `make` again bu
...
(https://github.com/bitcoin/bitcoin/issues/30891)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo
- [X] I still think this issue should be opened here
### Report
Hi guys, I am using a macbook Sonoma 14.0 and I am following the compilation tutorial, when I tried to run the Functional tests, I get this error:
```
FileNotFoundError: [Errno 2] No such file or directory: '/Users/user/Documents/personalProjects/bitcoin/src/bitcoind'
```
I already tried to run `make clean` and `make` again bu
...
π¬ glozow commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2347058915)
> It's a regression fuzz test for https://github.com/bitcoin/bitcoin/pull/26355
Should this fuzzer fail if I make this line return the result from `IsContinuationOfLowWorkHeadersSync`?
https://github.com/bitcoin/bitcoin/blob/cf0120ff024aa73a56f2975c832fda6aa8146dfa/src/net_processing.cpp#L2900
(https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2347058915)
> It's a regression fuzz test for https://github.com/bitcoin/bitcoin/pull/26355
Should this fuzzer fail if I make this line return the result from `IsContinuationOfLowWorkHeadersSync`?
https://github.com/bitcoin/bitcoin/blob/cf0120ff024aa73a56f2975c832fda6aa8146dfa/src/net_processing.cpp#L2900
π¬ maflcko commented on issue "No such file or directory: bitcoind Error":
(https://github.com/bitcoin/bitcoin/issues/30891#issuecomment-2347065300)
What are the exact steps to reproduce?
(https://github.com/bitcoin/bitcoin/issues/30891#issuecomment-2347065300)
What are the exact steps to reproduce?
π€ murchandamus reviewed a pull request: "doc: clarify loadwallet path loading for wallets"
(https://github.com/bitcoin/bitcoin/pull/30302#pullrequestreview-2301194468)
utACK 69bf58dc0e25897e9fde435c9823a921590a90dc
Thanks for the additional examples and improving the documentation
(https://github.com/bitcoin/bitcoin/pull/30302#pullrequestreview-2301194468)
utACK 69bf58dc0e25897e9fde435c9823a921590a90dc
Thanks for the additional examples and improving the documentation
π¬ murchandamus commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1757439318)
I agree with @hodlinator, and this may be a bit verbose for something that is output to the console. How about:
```suggestion
{"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The path of the wallet directory (or a legacy walletβs .dat file). Takes an absolute path or a path relative to the default wallet directory."},
```
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1757439318)
I agree with @hodlinator, and this may be a bit verbose for something that is output to the console. How about:
```suggestion
{"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The path of the wallet directory (or a legacy walletβs .dat file). Takes an absolute path or a path relative to the default wallet directory."},
```
π¬ hebasto commented on issue "No such file or directory: bitcoind Error":
(https://github.com/bitcoin/bitcoin/issues/30891#issuecomment-2347066598)
> I am following the compilation tutorial, when I tried to run the Functional tests, I get this error:
>
> I also using v27.1
Are you using documentation for the same version you are building?
(https://github.com/bitcoin/bitcoin/issues/30891#issuecomment-2347066598)
> I am following the compilation tutorial, when I tried to run the Functional tests, I get this error:
>
> I also using v27.1
Are you using documentation for the same version you are building?
π¬ jonatack commented on pull request "cli: rework -addrinfo cli to use addresses which arenβt filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-2347070928)
@stratospher (needs rebase) happy to do a call and look at this together to move it forward if you like.
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-2347070928)
@stratospher (needs rebase) happy to do a call and look at this together to move it forward if you like.
π¬ ryanofsky commented on pull request "log: Use ConstevalFormatString":
(https://github.com/bitcoin/bitcoin/pull/30889#issuecomment-2347083912)
Code review ACK 555513ac4e40f25b54d5f0473904adb23f4e3df7. This is great and I'm surprised it could be implemented so easily.
I do have questions about the second commit "log: Make format errors visible in debug builds" (fa0383761696d2c5c2e88208676cc993694fc1d4). I understand compile time checking should prevent most runtime errors, so the behavior in that commit of aborting the program when an invalid format string is specified is not as annoying for log-print debugging as it would be otherwi
...
(https://github.com/bitcoin/bitcoin/pull/30889#issuecomment-2347083912)
Code review ACK 555513ac4e40f25b54d5f0473904adb23f4e3df7. This is great and I'm surprised it could be implemented so easily.
I do have questions about the second commit "log: Make format errors visible in debug builds" (fa0383761696d2c5c2e88208676cc993694fc1d4). I understand compile time checking should prevent most runtime errors, so the behavior in that commit of aborting the program when an invalid format string is specified is not as annoying for log-print debugging as it would be otherwi
...