💬 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.
💬 maflcko commented on pull request "test: Prove+document ConstevalFormatString/tinyformat parity":
(https://github.com/bitcoin/bitcoin/pull/30933#issuecomment-2388482275)
Are you still working on this?
I am asking now, because there shouldn't be any conflicts after rebase, I think.
(https://github.com/bitcoin/bitcoin/pull/30933#issuecomment-2388482275)
Are you still working on this?
I am asking now, because there shouldn't be any conflicts after rebase, I think.
💬 l0rinc commented on pull request "dbwrapper: Bump LevelDB max file size to 128 MiB to avoid system slowdown from high disk cache flush rate":
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2388503588)
I did a few benchmarks on HDD and SSD separately (no raspberry pi yet, but I understood @davidgumberg did some of those and saw a significant speedup), to see the effect of the different values on IBD.
I have tried different values via https://github.com/bitcoin/bitcoin/pull/30059 (rebased), namely DBFILESIZE 1,__2__,4,8,16,32,64,128,256,512 (current value is 2) with default `dbcache`, until 600k blocks using real nodes (which introduces some randomness, but the repeated runs should still ind
...
(https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2388503588)
I did a few benchmarks on HDD and SSD separately (no raspberry pi yet, but I understood @davidgumberg did some of those and saw a significant speedup), to see the effect of the different values on IBD.
I have tried different values via https://github.com/bitcoin/bitcoin/pull/30059 (rebased), namely DBFILESIZE 1,__2__,4,8,16,32,64,128,256,512 (current value is 2) with default `dbcache`, until 600k blocks using real nodes (which introduces some randomness, but the repeated runs should still ind
...
🤔 l0rinc reviewed a pull request: "Add option dbfilesize to control LevelDB target ("max") file size"
(https://github.com/bitcoin/bitcoin/pull/30059#pullrequestreview-2342721830)
I've rebased this and ran a few benchmarks with different values.
Will do a few more measurements, but so far I'm rather leaning on finding a better value than the default instead of making this confuggurable.
See: https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2388503588
(https://github.com/bitcoin/bitcoin/pull/30059#pullrequestreview-2342721830)
I've rebased this and ran a few benchmarks with different values.
Will do a few more measurements, but so far I'm rather leaning on finding a better value than the default instead of making this confuggurable.
See: https://github.com/bitcoin/bitcoin/pull/30039#issuecomment-2388503588
💬 l0rinc commented on pull request "Add option dbfilesize to control LevelDB target ("max") file size":
(https://github.com/bitcoin/bitcoin/pull/30059#discussion_r1784374410)
setting the option here after we've partially constructed it seems inconsequential, could we add it as a parameter instead?
```diff
diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp
--- a/src/dbwrapper.cpp (revision 5f69dc11cb53c1794e9e71e6f398b95c1037cdc2)
+++ b/src/dbwrapper.cpp (date 1727870700414)
@@ -134,9 +134,10 @@
options->max_open_files, default_open_files);
}
-static leveldb::Options GetOptions(size_t nCacheSize)
+static leveldb::Options GetOptions(size_t nC
...
(https://github.com/bitcoin/bitcoin/pull/30059#discussion_r1784374410)
setting the option here after we've partially constructed it seems inconsequential, could we add it as a parameter instead?
```diff
diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp
--- a/src/dbwrapper.cpp (revision 5f69dc11cb53c1794e9e71e6f398b95c1037cdc2)
+++ b/src/dbwrapper.cpp (date 1727870700414)
@@ -134,9 +134,10 @@
options->max_open_files, default_open_files);
}
-static leveldb::Options GetOptions(size_t nCacheSize)
+static leveldb::Options GetOptions(size_t nC
...
💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2388526922)
Building 291ec5e2a876622a051baf6e33a4d4d592e6568e (on arm64):
```bash
make -C depends HOST=x86_64-apple-darwin qt
Building native_qt...
[13/312] Building CXX object qtbase/src/tools/macdeployqt/macdeployqt/CMakeFiles/macdeployqt.dir/__/shared/shared.cpp.o
FAILED: qtbase/src/tools/macdeployqt/macdeployqt/CMakeFiles/macdeployqt.dir/__/shared/shared.cpp.o
/Library/Developer/CommandLineTools/usr/bin/clang++ -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk -DGL_SILENCE_DEPRECATION
...
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2388526922)
Building 291ec5e2a876622a051baf6e33a4d4d592e6568e (on arm64):
```bash
make -C depends HOST=x86_64-apple-darwin qt
Building native_qt...
[13/312] Building CXX object qtbase/src/tools/macdeployqt/macdeployqt/CMakeFiles/macdeployqt.dir/__/shared/shared.cpp.o
FAILED: qtbase/src/tools/macdeployqt/macdeployqt/CMakeFiles/macdeployqt.dir/__/shared/shared.cpp.o
/Library/Developer/CommandLineTools/usr/bin/clang++ -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk -DGL_SILENCE_DEPRECATION
...