💬 hebasto commented on issue "cmake: passing options from depends build to main build":
(https://github.com/bitcoin/bitcoin/issues/30813#issuecomment-2388309124)
This discussion might be related: https://discourse.cmake.org/t/is-there-a-downside-for-a-toolchain-to-set-vars-in-cache-instead-of-init/12034.
(https://github.com/bitcoin/bitcoin/issues/30813#issuecomment-2388309124)
This discussion might be related: https://discourse.cmake.org/t/is-there-a-downside-for-a-toolchain-to-set-vars-in-cache-instead-of-init/12034.
💬 stickies-v commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2388310577)
> It would be cleaner _in the implementation_ to use `util::Result` instead of overwriting the returned value with the error message
I don't think that's a good approach for our codebase, even if there was minimal code churn. All format strings are known at compile-time, so any formatting errors are programming errors. Handling that with `util::Result` would be both a cumbersome interface and bad practice imo. We don't need to let callsites handle formatting errors, they should never occur, b
...
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2388310577)
> It would be cleaner _in the implementation_ to use `util::Result` instead of overwriting the returned value with the error message
I don't think that's a good approach for our codebase, even if there was minimal code churn. All format strings are known at compile-time, so any formatting errors are programming errors. Handling that with `util::Result` would be both a cumbersome interface and bad practice imo. We don't need to let callsites handle formatting errors, they should never occur, b
...
✅ maflcko closed an issue: "Intermittent timeout in tsan feature_init.py"
(https://github.com/bitcoin/bitcoin/issues/30586)
(https://github.com/bitcoin/bitcoin/issues/30586)
💬 maflcko commented on issue "Intermittent timeout in tsan feature_init.py":
(https://github.com/bitcoin/bitcoin/issues/30586#issuecomment-2388314370)
Closing for now, but this can be reopened when it happens again the next time.
(https://github.com/bitcoin/bitcoin/issues/30586#issuecomment-2388314370)
Closing for now, but this can be reopened when it happens again the next time.
🚀 fanquake merged a pull request: "doc: add testnet4 section header for config file"
(https://github.com/bitcoin/bitcoin/pull/31007)
(https://github.com/bitcoin/bitcoin/pull/31007)
💬 fanquake commented on pull request "wallet: Filter-out "send" addresses from `listreceivedby*`":
(https://github.com/bitcoin/bitcoin/pull/25973#issuecomment-2388319477)
Some of this was picked up in #30972.
(https://github.com/bitcoin/bitcoin/pull/25973#issuecomment-2388319477)
Some of this was picked up in #30972.
💬 maflcko commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2388324332)
I agree that `util::Result` doesn't make any sense here. Apart from the reasons mentioned, it would also make it harder to switch to `std::format`, as well as being confusing, because no other format lib I am aware of is doing that.
> * using the `tfm::format_raw` functions (only for tinyformat implementation or tests)
In the PR description: I think this should also say `or bitcoin-cli`.
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2388324332)
I agree that `util::Result` doesn't make any sense here. Apart from the reasons mentioned, it would also make it harder to switch to `std::format`, as well as being confusing, because no other format lib I am aware of is doing that.
> * using the `tfm::format_raw` functions (only for tinyformat implementation or tests)
In the PR description: I think this should also say `or bitcoin-cli`.
💬 maflcko commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1784285299)
If `Tinyformat handles string parameters.` is too controversial, I think it can be removed in the follow-up that removes the linter.
Otherwise, if this is replaced with `std::string`, someone else is going to suggest to mention `std::string_view` as well.
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1784285299)
If `Tinyformat handles string parameters.` is too controversial, I think it can be removed in the follow-up that removes the linter.
Otherwise, if this is replaced with `std::string`, someone else is going to suggest to mention `std::string_view` as well.
💬 stickies-v commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2388345105)
> In the PR description: I think this should also say `or bitcoin-cli`.
`bitcoin-cli` wasn't using the `const std::string&` overload which that bulletpoint is about. It is mentioned in the next bulletpoint about compile-time checks:
> Callsites that for some reason don't pass the compile-time checks (such as in `bitcoin-cli.cpp`) can use `tfm::format_raw`.
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2388345105)
> In the PR description: I think this should also say `or bitcoin-cli`.
`bitcoin-cli` wasn't using the `const std::string&` overload which that bulletpoint is about. It is mentioned in the next bulletpoint about compile-time checks:
> Callsites that for some reason don't pass the compile-time checks (such as in `bitcoin-cli.cpp`) can use `tfm::format_raw`.
💬 MrSuddenJoy commented on issue "RFC: Multiprocess binaries and packaging options":
(https://github.com/bitcoin/bitcoin/issues/30983#issuecomment-2388347821)
@ryanofsky Not entirely sure this is way to go imho.
As long as everything works well ( = as expected ) its all ok, but as soon as things go south, problems (especially ones that are hard to solve/complex) start to accumulate/pile up. And thats exactly whats root cause of many bad things, namely frustration, burn-out and things like this.
Having `meta-package` has proven (to me) to be perfect cure for problems described above. If something breaks, its dead simple to: localise where (in cod
...
(https://github.com/bitcoin/bitcoin/issues/30983#issuecomment-2388347821)
@ryanofsky Not entirely sure this is way to go imho.
As long as everything works well ( = as expected ) its all ok, but as soon as things go south, problems (especially ones that are hard to solve/complex) start to accumulate/pile up. And thats exactly whats root cause of many bad things, namely frustration, burn-out and things like this.
Having `meta-package` has proven (to me) to be perfect cure for problems described above. If something breaks, its dead simple to: localise where (in cod
...
💬 maflcko commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#discussion_r1784296416)
maybe I am missing something, but this looks still wrong. I am reading this as fuzzing adding *additional* constraints, which seems the inverse of what we want to achieve.
Also, this could be impossible to satisfy, if `!(m_magic_bytes[0]&1)` holds. What am I missing?
(https://github.com/bitcoin/bitcoin/pull/31012#discussion_r1784296416)
maybe I am missing something, but this looks still wrong. I am reading this as fuzzing adding *additional* constraints, which seems the inverse of what we want to achieve.
Also, this could be impossible to satisfy, if `!(m_magic_bytes[0]&1)` holds. What am I missing?
💬 Sjors commented on issue "depends: llvm-ranlib (etc): "No such file or directory" on Intel macOS 15.0":
(https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2388352764)
I get it every time on master and any branch I test on.
Here's the initial log from a clean run on fc642c33ef28829eda0119a0fe39fd9bc4b84051 on Intel macOS 15.0:
```
git clean -dfx
cd depends
make
/bin/sh: command -v llvm-ranlib: No such file or directory
/bin/sh: command -v llvm-strip: No such file or directory
/bin/sh: command -v llvm-nm: No such file or directory
/bin/sh: command -v llvm-objdump: No such file or directory
/bin/sh: command -v dsymutil: No such file or directory
...
(https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2388352764)
I get it every time on master and any branch I test on.
Here's the initial log from a clean run on fc642c33ef28829eda0119a0fe39fd9bc4b84051 on Intel macOS 15.0:
```
git clean -dfx
cd depends
make
/bin/sh: command -v llvm-ranlib: No such file or directory
/bin/sh: command -v llvm-strip: No such file or directory
/bin/sh: command -v llvm-nm: No such file or directory
/bin/sh: command -v llvm-objdump: No such file or directory
/bin/sh: command -v dsymutil: No such file or directory
...
💬 Sjors commented on issue "depends: llvm-ranlib (etc): "No such file or directory" on Intel macOS 15.0":
(https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2388358515)
Mmm, maybe it's because my 15.2 has the full Xcode install? (not needed for Bitcoin Core)
(https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2388358515)
Mmm, maybe it's because my 15.2 has the full Xcode install? (not needed for Bitcoin Core)
💬 fanquake commented on pull request "build: have "make test" depend on "make all"":
(https://github.com/bitcoin/bitcoin/pull/31015#issuecomment-2388364779)
Concept ACK. Nice that after 15 years there is now a solution (https://cmake.org/Bug/view.php?id=8774, https://gitlab.kitware.com/cmake/cmake/-/issues/8774).
(https://github.com/bitcoin/bitcoin/pull/31015#issuecomment-2388364779)
Concept ACK. Nice that after 15 years there is now a solution (https://cmake.org/Bug/view.php?id=8774, https://gitlab.kitware.com/cmake/cmake/-/issues/8774).
💬 Sjors commented on issue "depends: llvm-ranlib (etc): "No such file or directory" on Intel macOS 15.0":
(https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2388364879)
Deleting Xcode (on the macOS 15.0 machine) doesn't fix the error.
It's possible that the installation made permanent changes, but this does point to a new place:
```
% xcrun -f ranlib
/Library/Developer/CommandLineTools/usr/bin/ranlib
```
I'll try a reboot.
(https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2388364879)
Deleting Xcode (on the macOS 15.0 machine) doesn't fix the error.
It's possible that the installation made permanent changes, but this does point to a new place:
```
% xcrun -f ranlib
/Library/Developer/CommandLineTools/usr/bin/ranlib
```
I'll try a reboot.
💬 brunoerg commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#discussion_r1784313319)
Before, if `hdr.pchMessageStart != m_magic_bytes` we would return an error. That is, fuzzing would have to spend time matching it. Now, we will only invalidate it if `hdr.pchMessageStart != m_magic_bytes` and `(hdr.pchMessageStart[0] & 1) != 0`. So, it will make fuzzing easier by just needing to match `(hdr.pchMessageStart[0] & 1) == 0`.
(https://github.com/bitcoin/bitcoin/pull/31012#discussion_r1784313319)
Before, if `hdr.pchMessageStart != m_magic_bytes` we would return an error. That is, fuzzing would have to spend time matching it. Now, we will only invalidate it if `hdr.pchMessageStart != m_magic_bytes` and `(hdr.pchMessageStart[0] & 1) != 0`. So, it will make fuzzing easier by just needing to match `(hdr.pchMessageStart[0] & 1) == 0`.
👋 brunoerg's pull request is ready for review: "net: fuzz: bypass network magic and checksum validation"
(https://github.com/bitcoin/bitcoin/pull/31012)
(https://github.com/bitcoin/bitcoin/pull/31012)
💬 vasild commented on issue "V2 Only Option":
(https://github.com/bitcoin/bitcoin/issues/29618#issuecomment-2388394165)
> I asked for this because of a friend in a country where they are having issues, very real issues and they did not want to doxx themselves
Even with V2-only your friend will still doxx themselves, see https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2381424649. Running a node thinking they are hidden whereas they are not is dangerous.
To summarize - connecting to publicly known bitcoin nodes is already revealing the fact that one runs a bitcoin node, regardless of encryption. V2
...
(https://github.com/bitcoin/bitcoin/issues/29618#issuecomment-2388394165)
> I asked for this because of a friend in a country where they are having issues, very real issues and they did not want to doxx themselves
Even with V2-only your friend will still doxx themselves, see https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2381424649. Running a node thinking they are hidden whereas they are not is dangerous.
To summarize - connecting to publicly known bitcoin nodes is already revealing the fact that one runs a bitcoin node, regardless of encryption. V2
...
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1784357648)
Thanks! Fixed.
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1784357648)
Thanks! Fixed.
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2388451346)
> Depends builds on macOS now, but the next step fails:
Should be fixed now.
> P.S. it would be nice if `make` in `depends` provides the incantation needed for the configure stage. Back in the day it would give you the `--prefix` argument to use.
Addressed in https://github.com/bitcoin/bitcoin/pull/31008.
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2388451346)
> Depends builds on macOS now, but the next step fails:
Should be fixed now.
> P.S. it would be nice if `make` in `depends` provides the incantation needed for the configure stage. Back in the day it would give you the `--prefix` argument to use.
Addressed in https://github.com/bitcoin/bitcoin/pull/31008.