🚀 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.
💬 vasild commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2388454981)
A V2-only option will:
* not hide the fact that one runs bitcoin node (that is true regardless of encryption) and
* not stop even moderately motivated actor from spying for transaction origin (either by opening a P2P connection to the target or bricking V2 encryption or outright redirecting connections to their node)
but it will give the false impression that it does those two things. Further, it will make it more difficult for old nodes to find peers to connect to.
Concept NACK becaus
...
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2388454981)
A V2-only option will:
* not hide the fact that one runs bitcoin node (that is true regardless of encryption) and
* not stop even moderately motivated actor from spying for transaction origin (either by opening a P2P connection to the target or bricking V2 encryption or outright redirecting connections to their node)
but it will give the false impression that it does those two things. Further, it will make it more difficult for old nodes to find peers to connect to.
Concept NACK becaus
...
💬 hebasto commented on pull request "qt6, test: Handle deprecated code":
(https://github.com/bitcoin-core/gui/pull/839#issuecomment-2388460921)
Friendly ping @Sjors @furszy @ryanofsky @jarolrod ;)
(https://github.com/bitcoin-core/gui/pull/839#issuecomment-2388460921)
Friendly ping @Sjors @furszy @ryanofsky @jarolrod ;)
💬 hebasto commented on pull request "qt6: Handle deprecated `QLocale::nativeCountryName`":
(https://github.com/bitcoin-core/gui/pull/838#issuecomment-2388461752)
Friendly ping @Sjors @furszy @achow101 @jarolrod ;)
(https://github.com/bitcoin-core/gui/pull/838#issuecomment-2388461752)
Friendly ping @Sjors @furszy @achow101 @jarolrod ;)
💬 maflcko commented on pull request "test: Fix RANDOM_CTX_SEED use with parallel tests":
(https://github.com/bitcoin/bitcoin/pull/30737#discussion_r1784377196)
> However, if you want to go down the route of requiring to set a different testdatadir when wanting to run the same test in parallel, it may be best to fully go that route. Instead of a hash, just use the test name as-is? This would also be parallel to the functional tests.
For reference, https://github.com/bitcoin/bitcoin/pull/31000 should be one step into that direction.
(https://github.com/bitcoin/bitcoin/pull/30737#discussion_r1784377196)
> However, if you want to go down the route of requiring to set a different testdatadir when wanting to run the same test in parallel, it may be best to fully go that route. Instead of a hash, just use the test name as-is? This would also be parallel to the functional tests.
For reference, https://github.com/bitcoin/bitcoin/pull/31000 should be one step into that direction.