💬 fanquake commented on pull request "guix: document when certain patches can be dropped":
(https://github.com/bitcoin/bitcoin/pull/27668#discussion_r1194938523)
I don't think that gains us anything here, as it's implied we can only use `--with-nonshared-cflags` when it's actually available.
(https://github.com/bitcoin/bitcoin/pull/27668#discussion_r1194938523)
I don't think that gains us anything here, as it's implied we can only use `--with-nonshared-cflags` when it's actually available.
🚀 fanquake merged a pull request: "Build: Improve handling of suppressed logging in Makefiles"
(https://github.com/bitcoin/bitcoin/pull/27041)
(https://github.com/bitcoin/bitcoin/pull/27041)
👍 hebasto approved a pull request: "guix: document when certain patches can be dropped"
(https://github.com/bitcoin/bitcoin/pull/27668#pullrequestreview-1428237915)
ACK a09269a146b1e32a0e7979692f4455cc2f6faeae, I have reviewed the changes and they look OK.
(https://github.com/bitcoin/bitcoin/pull/27668#pullrequestreview-1428237915)
ACK a09269a146b1e32a0e7979692f4455cc2f6faeae, I have reviewed the changes and they look OK.
💬 fanquake commented on issue "Use muhash for assumeUTXO snapshot ":
(https://github.com/bitcoin/bitcoin/issues/27669#issuecomment-1549397147)
cc @jamesob
(https://github.com/bitcoin/bitcoin/issues/27669#issuecomment-1549397147)
cc @jamesob
💬 kouloumos commented on pull request "Introduce `MockableDatabase` for wallet unit tests":
(https://github.com/bitcoin/bitcoin/pull/26715#issuecomment-1549400101)
While reviewing https://github.com/bitcoin/bitcoin/pull/27666, I observed that this PR changed the performance of the `WalletLoadingLegacy` benchmark:
PR branch (33e2b82a4fc990253ff77655f437c7aed336bc55)
| ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:-
...
(https://github.com/bitcoin/bitcoin/pull/26715#issuecomment-1549400101)
While reviewing https://github.com/bitcoin/bitcoin/pull/27666, I observed that this PR changed the performance of the `WalletLoadingLegacy` benchmark:
PR branch (33e2b82a4fc990253ff77655f437c7aed336bc55)
| ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:-
...
👍 fanquake approved a pull request: "build: Use newest `config.{guess,sub}` available"
(https://github.com/bitcoin/bitcoin/pull/26422#pullrequestreview-1428252489)
ACK ea7b8528490d330f0f4e34e9b26ab00ba528f546
(https://github.com/bitcoin/bitcoin/pull/26422#pullrequestreview-1428252489)
ACK ea7b8528490d330f0f4e34e9b26ab00ba528f546
🚀 fanquake merged a pull request: "build: Use newest `config.{guess,sub}` available"
(https://github.com/bitcoin/bitcoin/pull/26422)
(https://github.com/bitcoin/bitcoin/pull/26422)
💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1194955775)
Yeah
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1194955775)
Yeah
📝 fanquake opened a pull request: "guix: remove redundant glibc patches"
(https://github.com/bitcoin/bitcoin/pull/27670)
These should only be relevant for a glibc that is built as part of a Guix system, and should not be required for a glibc that is just being built to compile our binaries against. A x86_64 linux bitcoind produced with Guix using master vs this change has no difference. i.e:
```diff
@@ -20311,15 +20311,15 @@
This is experimental software.
The source code is available from %s.
Please contribute if you find %s useful. Visit %s for further information about the software.
The %s developers
...
(https://github.com/bitcoin/bitcoin/pull/27670)
These should only be relevant for a glibc that is built as part of a Guix system, and should not be required for a glibc that is just being built to compile our binaries against. A x86_64 linux bitcoind produced with Guix using master vs this change has no difference. i.e:
```diff
@@ -20311,15 +20311,15 @@
This is experimental software.
The source code is available from %s.
Please contribute if you find %s useful. Visit %s for further information about the software.
The %s developers
...
💬 Sjors commented on pull request "assumeutxo (2)":
(https://github.com/bitcoin/bitcoin/pull/27596#issuecomment-1549423309)
Here's a signet snapshot torrent: `magnet:?xt=urn:btih:0e1a61b4ed9a57c41d98f1ddd5061bc088c857b6&dn=utxo-signet-140000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
Can be added to `kernel/chainparams.cpp` with:
```cpp
m_assumeutxo_data = MapAssumeutxo{
{
140'000,
{AssumeutxoHash{uint256S("0x91412d7018c8564beef1938eef2bfe18feee53674fd26bea425daa4280437641")}, 2233882},
},
};
```
(https://github.com/bitcoin/bitcoin/pull/27596#issuecomment-1549423309)
Here's a signet snapshot torrent: `magnet:?xt=urn:btih:0e1a61b4ed9a57c41d98f1ddd5061bc088c857b6&dn=utxo-signet-140000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969`
Can be added to `kernel/chainparams.cpp` with:
```cpp
m_assumeutxo_data = MapAssumeutxo{
{
140'000,
{AssumeutxoHash{uint256S("0x91412d7018c8564beef1938eef2bfe18feee53674fd26bea425daa4280437641")}, 2233882},
},
};
```
💬 josibake commented on pull request "[23.2] Final Changes":
(https://github.com/bitcoin/bitcoin/pull/27663#issuecomment-1549434174)
ACK https://github.com/bitcoin/bitcoin/pull/27663/commits/7ae937326ae009aa825c5d855b20e79ab1a7e5dc
(https://github.com/bitcoin/bitcoin/pull/27663#issuecomment-1549434174)
ACK https://github.com/bitcoin/bitcoin/pull/27663/commits/7ae937326ae009aa825c5d855b20e79ab1a7e5dc
📝 fanquake opened a pull request: "depends: Boost 1.82.0"
(https://github.com/bitcoin/bitcoin/pull/27671)
https://www.boost.org/users/history/version_1_82_0.html
Relevant changelogs:
https://github.com/boostorg/core/compare/boost-1.81.0...boost-1.82.0
https://github.com/boostorg/multi_index/compare/boost-1.81.0...boost-1.82.0
https://github.com/boostorg/test/compare/boost-1.81.0...boost-1.82.0
https://github.com/boostorg/process/compare/boost-1.81.0...boost-1.82.0
Other modules, i.e date_time, signals2, etc, had no changes.
(https://github.com/bitcoin/bitcoin/pull/27671)
https://www.boost.org/users/history/version_1_82_0.html
Relevant changelogs:
https://github.com/boostorg/core/compare/boost-1.81.0...boost-1.82.0
https://github.com/boostorg/multi_index/compare/boost-1.81.0...boost-1.82.0
https://github.com/boostorg/test/compare/boost-1.81.0...boost-1.82.0
https://github.com/boostorg/process/compare/boost-1.81.0...boost-1.82.0
Other modules, i.e date_time, signals2, etc, had no changes.
💬 Sjors commented on issue "--with-sanitizers=float-divide-by-zero crash with -debug=bench in Chainstate::ConnectTip":
(https://github.com/bitcoin/bitcoin/issues/27635#issuecomment-1549439436)
The potential divide by zero was introduced here:
https://github.com/bitcoin/bitcoin/pull/24216/files#diff-97c3a52bc5fad452d82670a7fd291800bae20c7bc35bb82686c2c0a4ea7b5b98L2607-R2612
Let me see if I remember why…
(https://github.com/bitcoin/bitcoin/issues/27635#issuecomment-1549439436)
The potential divide by zero was introduced here:
https://github.com/bitcoin/bitcoin/pull/24216/files#diff-97c3a52bc5fad452d82670a7fd291800bae20c7bc35bb82686c2c0a4ea7b5b98L2607-R2612
Let me see if I remember why…
💬 hebasto commented on pull request "depends: Boost 1.82.0":
(https://github.com/bitcoin/bitcoin/pull/27671#issuecomment-1549442652)
Skimming changelogs did not reveal a reason why we need this update.
Did I miss something important?
(https://github.com/bitcoin/bitcoin/pull/27671#issuecomment-1549442652)
Skimming changelogs did not reveal a reason why we need this update.
Did I miss something important?
📝 MarcoFalke opened a pull request: "fuzz: Print error message when FUZZ is missing"
(https://github.com/bitcoin/bitcoin/pull/27672)
Some trivial UX improvements.
* Change the exit code for `PRINT_ALL_FUZZ_TARGETS_AND_ABORT` and `WRITE_ALL_FUZZ_TARGETS_AND_ABORT` to `EXIT_SUCCESS` instead of `Aborted (core dumped)`.
* Print readable error message when `FUZZ` is missing instead of `Aborted (core dumped)`.
* Clarify that a fuzz target needs to be compiled into the executable.
(https://github.com/bitcoin/bitcoin/pull/27672)
Some trivial UX improvements.
* Change the exit code for `PRINT_ALL_FUZZ_TARGETS_AND_ABORT` and `WRITE_ALL_FUZZ_TARGETS_AND_ABORT` to `EXIT_SUCCESS` instead of `Aborted (core dumped)`.
* Print readable error message when `FUZZ` is missing instead of `Aborted (core dumped)`.
* Clarify that a fuzz target needs to be compiled into the executable.
💬 MarcoFalke commented on pull request "fuzz: Print error message when FUZZ is missing":
(https://github.com/bitcoin/bitcoin/pull/27672#issuecomment-1549450433)
(Also contains some no-op code to allow for easier hacking)
(https://github.com/bitcoin/bitcoin/pull/27672#issuecomment-1549450433)
(Also contains some no-op code to allow for easier hacking)
💬 MarcoFalke commented on pull request "depends: Boost 1.82.0":
(https://github.com/bitcoin/bitcoin/pull/27671#issuecomment-1549456481)
Seems a bit early to bump, given that no distro ships with that version, and there is no need for the Bitcoin Core release binaries to be the first ones to test the new boost release.
(https://github.com/bitcoin/bitcoin/pull/27671#issuecomment-1549456481)
Seems a bit early to bump, given that no distro ships with that version, and there is no need for the Bitcoin Core release binaries to be the first ones to test the new boost release.
💬 sipa commented on issue "Allow getblockfrompeer to use any peer":
(https://github.com/bitcoin/bitcoin/issues/27652#issuecomment-1549464215)
I don't think this is hard to implement, and doesn't need #27626 (it wouldn't use compact blocks).
We effectively already have a background block download fetching algorithm, that regularly decides which blocks should be fetched from which peers (see `FindNextBlocksToDownload` in net_processing.cpp). I think it should be possible to have a list/set somewhere in the net processing state of CBlockIndexes/uint256s/... of blocks to-redownload. If any peer served by `FindNextBlocksToDownload` happ
...
(https://github.com/bitcoin/bitcoin/issues/27652#issuecomment-1549464215)
I don't think this is hard to implement, and doesn't need #27626 (it wouldn't use compact blocks).
We effectively already have a background block download fetching algorithm, that regularly decides which blocks should be fetched from which peers (see `FindNextBlocksToDownload` in net_processing.cpp). I think it should be possible to have a list/set somewhere in the net processing state of CBlockIndexes/uint256s/... of blocks to-redownload. If any peer served by `FindNextBlocksToDownload` happ
...
💬 fanquake commented on pull request "depends: Boost 1.82.0":
(https://github.com/bitcoin/bitcoin/pull/27671#issuecomment-1549464475)
> given that no distro ships with that version
Pretty much all of the rolling/modern distros, i.e Alpine, FreeBSD, Gentoo, Tumbleweed, are already shipping this.
(https://github.com/bitcoin/bitcoin/pull/27671#issuecomment-1549464475)
> given that no distro ships with that version
Pretty much all of the rolling/modern distros, i.e Alpine, FreeBSD, Gentoo, Tumbleweed, are already shipping this.
📝 Sjors opened a pull request: "ConnectTip: don't log total disk read time in bench"
(https://github.com/bitcoin/bitcoin/pull/27673)
This incorrectly assumed num_blocks_total would be greater than 0. This is not guaranteed until the ConnectBlock call right below it.
The total and average metric is not very useful because it does not distinguish between blocks read from disk and those loaded from memory. So rather than fixing the divide by zero issue, we just drop the metric.
Fixes #27635
(https://github.com/bitcoin/bitcoin/pull/27673)
This incorrectly assumed num_blocks_total would be greater than 0. This is not guaranteed until the ConnectBlock call right below it.
The total and average metric is not very useful because it does not distinguish between blocks read from disk and those loaded from memory. So rather than fixing the divide by zero issue, we just drop the metric.
Fixes #27635