🚀 fanquake merged a pull request: "fuzz: More accurate coverage reports"
(https://github.com/bitcoin/bitcoin/pull/30156)
(https://github.com/bitcoin/bitcoin/pull/30156)
💬 maflcko commented on pull request "test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1618469965)
nit: Not sure about using the global for new code. I'd say it is cleaner to replace the SeedInsecureRand with a local FastRandomContext and then call the member methods directly. E.g `frc.randrange(64)`, etc
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1618469965)
nit: Not sure about using the global for new code. I'd say it is cleaner to replace the SeedInsecureRand with a local FastRandomContext and then call the member methods directly. E.g `frc.randrange(64)`, etc
🚀 fanquake merged a pull request: "build: LLD based macOS toolchain"
(https://github.com/bitcoin/bitcoin/pull/21778)
(https://github.com/bitcoin/bitcoin/pull/21778)
💬 fanquake commented on pull request "fuzz: Handle missing BDBRO errors":
(https://github.com/bitcoin/bitcoin/pull/30172#discussion_r1618477590)
@achow101 can you followup here? Would be good to merge this to avoid the fuzzing issues.
(https://github.com/bitcoin/bitcoin/pull/30172#discussion_r1618477590)
@achow101 can you followup here? Would be good to merge this to avoid the fuzzing issues.
💬 fanquake commented on pull request "lint/contrib/doc updates":
(https://github.com/bitcoin/bitcoin/pull/30084#issuecomment-2136876648)
What is the status of this? I think it'd be easiest to just drop the contrib changes. Also no PR description now?
(https://github.com/bitcoin/bitcoin/pull/30084#issuecomment-2136876648)
What is the status of this? I think it'd be easiest to just drop the contrib changes. Also no PR description now?
👋 fanquake's pull request is ready for review: "build: use `-no_exported_symbols` on macOS"
(https://github.com/bitcoin/bitcoin/pull/29072)
(https://github.com/bitcoin/bitcoin/pull/29072)
💬 achow101 commented on pull request "fuzz: Handle missing BDBRO errors":
(https://github.com/bitcoin/bitcoin/pull/30172#discussion_r1618509267)
Removed
(https://github.com/bitcoin/bitcoin/pull/30172#discussion_r1618509267)
Removed
👍 dergoegge approved a pull request: "fuzz: Handle missing BDBRO errors"
(https://github.com/bitcoin/bitcoin/pull/30172#pullrequestreview-2084802181)
reACK 9ddf39dd87a3729ceedaa05a207621a02c532536
(https://github.com/bitcoin/bitcoin/pull/30172#pullrequestreview-2084802181)
reACK 9ddf39dd87a3729ceedaa05a207621a02c532536
💬 vasild commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1618525186)
The newly added `int auto_outbounds = std::min(MAX_OUTBOUND_FULL_RELAY_CONNECTIONS + MAX_BLOCK_RELAY_ONLY_CONNECTIONS + MAX_FEELER_CONNECTIONS, nUserMaxConnections);` seems to be duplicating this logic, but not exactly:
https://github.com/bitcoin/bitcoin/blob/be100cf4c77a3e45750773689e0a396fda39d8a7/src/net.h#L1069-L1073
and I think this is getting complicated again. I would suggest to keep it simple - drop the last commit f569b06a53851f844219cd80cd33676470e3b776 `init: fix min required fd
...
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1618525186)
The newly added `int auto_outbounds = std::min(MAX_OUTBOUND_FULL_RELAY_CONNECTIONS + MAX_BLOCK_RELAY_ONLY_CONNECTIONS + MAX_FEELER_CONNECTIONS, nUserMaxConnections);` seems to be duplicating this logic, but not exactly:
https://github.com/bitcoin/bitcoin/blob/be100cf4c77a3e45750773689e0a396fda39d8a7/src/net.h#L1069-L1073
and I think this is getting complicated again. I would suggest to keep it simple - drop the last commit f569b06a53851f844219cd80cd33676470e3b776 `init: fix min required fd
...
👍 TheCharlatan approved a pull request: "fuzz: Handle missing BDBRO errors"
(https://github.com/bitcoin/bitcoin/pull/30172#pullrequestreview-2084829846)
ACK 9ddf39dd87a3729ceedaa05a207621a02c532536
(https://github.com/bitcoin/bitcoin/pull/30172#pullrequestreview-2084829846)
ACK 9ddf39dd87a3729ceedaa05a207621a02c532536
💬 paplorinc commented on pull request "test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1618551037)
Nice, thanks!
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1618551037)
Nice, thanks!
✅ fanquake closed an issue: "fuzz, wallet_bdb_parser: BDB builtin encryption is not supported"
(https://github.com/bitcoin/bitcoin/issues/30166)
(https://github.com/bitcoin/bitcoin/issues/30166)
🚀 fanquake merged a pull request: "fuzz: Handle missing BDBRO errors"
(https://github.com/bitcoin/bitcoin/pull/30172)
(https://github.com/bitcoin/bitcoin/pull/30172)
🚀 fanquake merged a pull request: "bench: enable wallet creation benchmarks on all platforms"
(https://github.com/bitcoin/bitcoin/pull/30122)
(https://github.com/bitcoin/bitcoin/pull/30122)
💬 fanquake commented on pull request "build: use `-no_exported_symbols` on macOS":
(https://github.com/bitcoin/bitcoin/pull/29072#issuecomment-2136977600)
Binary size comparison for `arm64-apple-darwin` of a Guix build of master (be100cf4c77a) vs this PR (6213ac97c6b5):
| Binary | master | PR |
| ------ | ------- | --- |
| bitcoin-cli | 481248 | 477056 |
| bitcoin-qt | 30559072 | 30511316 |
| bitcoin-tx | 2246456 | 2245808 |
| bitcoin-util | 324200 | 324168 |
| bitcoin-wallet | 5091904 | 5054440 |
| bitcoind | 11867476 | 11820592 |
| test_bitcoin | 20097440 | 20054668 |
(https://github.com/bitcoin/bitcoin/pull/29072#issuecomment-2136977600)
Binary size comparison for `arm64-apple-darwin` of a Guix build of master (be100cf4c77a) vs this PR (6213ac97c6b5):
| Binary | master | PR |
| ------ | ------- | --- |
| bitcoin-cli | 481248 | 477056 |
| bitcoin-qt | 30559072 | 30511316 |
| bitcoin-tx | 2246456 | 2245808 |
| bitcoin-util | 324200 | 324168 |
| bitcoin-wallet | 5091904 | 5054440 |
| bitcoind | 11867476 | 11820592 |
| test_bitcoin | 20097440 | 20054668 |
💬 maflcko commented on pull request "releases: use LLVM 18 for macOS":
(https://github.com/bitcoin/bitcoin/pull/30022#discussion_r1618584825)
It should be possible now to nuke this and require the user (if they want to cross-compile to macOS) to type `apt install clang-18` instead?
I presume leaving this to the user will allow to run the cross-compile on more distros, as opposed to being possibly confined to `x86_64-linux-gnu-ubuntu` when using the llvm artefacts?
(https://github.com/bitcoin/bitcoin/pull/30022#discussion_r1618584825)
It should be possible now to nuke this and require the user (if they want to cross-compile to macOS) to type `apt install clang-18` instead?
I presume leaving this to the user will allow to run the cross-compile on more distros, as opposed to being possibly confined to `x86_64-linux-gnu-ubuntu` when using the llvm artefacts?
💬 fanquake commented on pull request "releases: use LLVM 18 for macOS":
(https://github.com/bitcoin/bitcoin/pull/30022#discussion_r1618590069)
Yea, the plan is to switch away from vendoring Clang entirely, which is more straightforward now that #21778 is merged. I'll be followingup.
(https://github.com/bitcoin/bitcoin/pull/30022#discussion_r1618590069)
Yea, the plan is to switch away from vendoring Clang entirely, which is more straightforward now that #21778 is merged. I'll be followingup.
💬 vasild commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1618645388)
This is the upstream fix: https://github.com/freebsd/freebsd-src/pull/1070.
The suggested workaround to define `typeof` is ok:
```diff
diff --git i/src/util/netif.cpp w/src/util/netif.cpp
index 845b8aed1d..1840974a83 100644
--- i/src/util/netif.cpp
+++ w/src/util/netif.cpp
@@ -9,18 +9,24 @@
#include <logging.h>
#include <netbase.h>
#include <util/check.h>
#include <util/sock.h>
#include <util/syserror.h>
+#ifdef __FreeBSD__
+#include <osreldate.h>
+#endif
+
// Linux
...
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1618645388)
This is the upstream fix: https://github.com/freebsd/freebsd-src/pull/1070.
The suggested workaround to define `typeof` is ok:
```diff
diff --git i/src/util/netif.cpp w/src/util/netif.cpp
index 845b8aed1d..1840974a83 100644
--- i/src/util/netif.cpp
+++ w/src/util/netif.cpp
@@ -9,18 +9,24 @@
#include <logging.h>
#include <netbase.h>
#include <util/check.h>
#include <util/sock.h>
#include <util/syserror.h>
+#ifdef __FreeBSD__
+#include <osreldate.h>
+#endif
+
// Linux
...
🤔 willcl-ark reviewed a pull request: "contrib: Fixup verify-binaries OS platform parsing"
(https://github.com/bitcoin/bitcoin/pull/30147#pullrequestreview-2085013260)
I think the version parser here is is pretty good shape generally, or at least improved from its current state.
I am not convinced yet that it is handling `rc` correctly yet though, see results of this additional test i added:
```log
₿ ./contrib/verify-binaries/test2.py
Checking 27.0
Found RC version: version_base=27.0, version_rc= os_filter=
Checking 27.0-x86_64
Found RC version: version_base=27.0, version_rc= os_filter=x86_64
Checking 27.0-linux
Found RC version: version_base=27.0
...
(https://github.com/bitcoin/bitcoin/pull/30147#pullrequestreview-2085013260)
I think the version parser here is is pretty good shape generally, or at least improved from its current state.
I am not convinced yet that it is handling `rc` correctly yet though, see results of this additional test i added:
```log
₿ ./contrib/verify-binaries/test2.py
Checking 27.0
Found RC version: version_base=27.0, version_rc= os_filter=
Checking 27.0-x86_64
Found RC version: version_base=27.0, version_rc= os_filter=x86_64
Checking 27.0-linux
Found RC version: version_base=27.0
...
💬 willcl-ark commented on pull request "contrib: Fixup verify-binaries OS platform parsing":
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1618650022)
Not sure if we want this to specify bitcoincore.org, `bitcoin.org` does host some binaries too (and recently even added a few which aren't totally out of date!), and we do try both hosts. On the other hand, this advice is actually more useful, accurate and likely to be correct, so 🤷🏼♂️
(https://github.com/bitcoin/bitcoin/pull/30147#discussion_r1618650022)
Not sure if we want this to specify bitcoincore.org, `bitcoin.org` does host some binaries too (and recently even added a few which aren't totally out of date!), and we do try both hosts. On the other hand, this advice is actually more useful, accurate and likely to be correct, so 🤷🏼♂️