💬 hebasto commented on pull request "build: Bump minimum supported Clang to clang-10":
(https://github.com/bitcoin/bitcoin/pull/27682#discussion_r1196123867)
The only left usage of `SAVED_LIBS` can de dropped as well:
```diff
--- a/build-aux/m4/l_filesystem.m4
+++ b/build-aux/m4/l_filesystem.m4
@@ -25,8 +25,7 @@ AC_DEFUN([CHECK_FILESYSTEM], [
AC_MSG_RESULT([yes])
],[
AC_MSG_RESULT([no])
- SAVED_LIBS="$LIBS"
- LIBS="$SAVED_LIBS -lstdc++fs"
+ LIBS="$LIBS -lstdc++fs"
AC_MSG_CHECKING([whether std::filesystem needs -lstdc++fs])
AC_LINK_IFELSE([AC_LANG_SOURCE([_CHECK_FILESYSTEM_testbody])],[
...
(https://github.com/bitcoin/bitcoin/pull/27682#discussion_r1196123867)
The only left usage of `SAVED_LIBS` can de dropped as well:
```diff
--- a/build-aux/m4/l_filesystem.m4
+++ b/build-aux/m4/l_filesystem.m4
@@ -25,8 +25,7 @@ AC_DEFUN([CHECK_FILESYSTEM], [
AC_MSG_RESULT([yes])
],[
AC_MSG_RESULT([no])
- SAVED_LIBS="$LIBS"
- LIBS="$SAVED_LIBS -lstdc++fs"
+ LIBS="$LIBS -lstdc++fs"
AC_MSG_CHECKING([whether std::filesystem needs -lstdc++fs])
AC_LINK_IFELSE([AC_LANG_SOURCE([_CHECK_FILESYSTEM_testbody])],[
...
📝 MarcoFalke converted_to_draft a pull request: "build: Bump minimum supported GCC to g++-9"
(https://github.com/bitcoin/bitcoin/pull/27662)
It is a bit frustrating to write valid C++ code only to realize that g++-8 fails to parse it later on.
The only non-EOL operating system still shipping with g++-8 is CentOS Stream 8. I think it is reasonable for users of affected Linux distributions to:
* Upgrade their operating system, or compiler to a supported version.
* Alternatively, stay with a previous release of Bitcoin Core as long as it is supported.
Fixes https://github.com/bitcoin/bitcoin/issues/27537
(https://github.com/bitcoin/bitcoin/pull/27662)
It is a bit frustrating to write valid C++ code only to realize that g++-8 fails to parse it later on.
The only non-EOL operating system still shipping with g++-8 is CentOS Stream 8. I think it is reasonable for users of affected Linux distributions to:
* Upgrade their operating system, or compiler to a supported version.
* Alternatively, stay with a previous release of Bitcoin Core as long as it is supported.
Fixes https://github.com/bitcoin/bitcoin/issues/27537
💬 Sjors commented on issue "Use muhash for assumeUTXO snapshot ":
(https://github.com/bitcoin/bitcoin/issues/27669#issuecomment-1550964299)
For the purpose of verifying a consensus change the ordering doesn't matter. The end user of the snapshot would benefit from a sha256 hash in order to not waste time verifying incorrect snapshots (assuming that sha256 is quicker than muhash here).
A file checksum doesn't need to be in the consensus code, though if we offer an automatic download it may need to be somewhere in the code. The right file checksum may depend on the specific distribution mechanism, e.g. if you use a torrent you'd us
...
(https://github.com/bitcoin/bitcoin/issues/27669#issuecomment-1550964299)
For the purpose of verifying a consensus change the ordering doesn't matter. The end user of the snapshot would benefit from a sha256 hash in order to not waste time verifying incorrect snapshots (assuming that sha256 is quicker than muhash here).
A file checksum doesn't need to be in the consensus code, though if we offer an automatic download it may need to be somewhere in the code. The right file checksum may depend on the specific distribution mechanism, e.g. if you use a torrent you'd us
...
💬 MarcoFalke commented on pull request "build: Bump minimum supported Clang to clang-10":
(https://github.com/bitcoin/bitcoin/pull/27682#discussion_r1196131293)
Ok, will do on the next push. Otherwise it will go away when the file is translated to cmake or when GCC is bumped to g++-9, whichever happens earlier.
(https://github.com/bitcoin/bitcoin/pull/27682#discussion_r1196131293)
Ok, will do on the next push. Otherwise it will go away when the file is translated to cmake or when GCC is bumped to g++-9, whichever happens earlier.
💬 hebasto commented on pull request "build: Bump minimum supported Clang to clang-10":
(https://github.com/bitcoin/bitcoin/pull/27682#issuecomment-1550971166)
Just noting that our current Guix environment uses Clang 10.0.1.
(https://github.com/bitcoin/bitcoin/pull/27682#issuecomment-1550971166)
Just noting that our current Guix environment uses Clang 10.0.1.
💬 Sjors commented on issue "Mac osx 12.6.5 ":
(https://github.com/bitcoin/bitcoin/issues/27681#issuecomment-1550972453)
Also useful to know if this is an Intel mac or the newer M1 / M2.
(https://github.com/bitcoin/bitcoin/issues/27681#issuecomment-1550972453)
Also useful to know if this is an Intel mac or the newer M1 / M2.
🚀 hebasto merged a pull request: "test: Add missed header"
(https://github.com/bitcoin-core/gui/pull/729)
(https://github.com/bitcoin-core/gui/pull/729)
💬 fanquake commented on pull request "msvc: Rename `libbitcoinconsensus` to `libbitcoin_consensus` and other adjustments":
(https://github.com/bitcoin/bitcoin/pull/27615#issuecomment-1551007062)
Other than renaming things, what does this acheive? Unclear it's worth doing if everything done here is going to be thrown away when we switch to CMake.
(https://github.com/bitcoin/bitcoin/pull/27615#issuecomment-1551007062)
Other than renaming things, what does this acheive? Unclear it's worth doing if everything done here is going to be thrown away when we switch to CMake.
💬 MarcoFalke commented on pull request "build: Bump minimum supported Clang to clang-10":
(https://github.com/bitcoin/bitcoin/pull/27682#discussion_r1196171608)
Thanks, done.
(https://github.com/bitcoin/bitcoin/pull/27682#discussion_r1196171608)
Thanks, done.
💬 MarcoFalke commented on pull request "test: Add missed header":
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551017016)
Any understanding why or when this happened?
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551017016)
Any understanding why or when this happened?
💬 fanquake commented on pull request "test: Add missed header":
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551020550)
I think it was https://github.com/bitcoin/bitcoin/pull/26715.
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551020550)
I think it was https://github.com/bitcoin/bitcoin/pull/26715.
💬 MarcoFalke commented on pull request "test: Add missed header":
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551028017)
But CI passed on the pull and merge commit (https://github.com/bitcoin/bitcoin/runs/13481551706), no?
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551028017)
But CI passed on the pull and merge commit (https://github.com/bitcoin/bitcoin/runs/13481551706), no?
💬 hebasto commented on pull request "msvc: Rename `libbitcoinconsensus` to `libbitcoin_consensus` and other adjustments":
(https://github.com/bitcoin/bitcoin/pull/27615#issuecomment-1551029540)
> Other than renaming things, what does this acheive?
It is [required](https://github.com/bitcoin/bitcoin/pull/24773#issuecomment-1541888040) for https://github.com/bitcoin/bitcoin/pull/24773.
> Unclear it's worth doing if everything done here is going to be thrown away when we switch to CMake.
Sure. Not a priority at all.
(https://github.com/bitcoin/bitcoin/pull/27615#issuecomment-1551029540)
> Other than renaming things, what does this acheive?
It is [required](https://github.com/bitcoin/bitcoin/pull/24773#issuecomment-1541888040) for https://github.com/bitcoin/bitcoin/pull/24773.
> Unclear it's worth doing if everything done here is going to be thrown away when we switch to CMake.
Sure. Not a priority at all.
💬 hebasto commented on pull request "test: Add missed header":
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551033551)
The MSVC link issue was kind of intermittent. I guess, it was dependent on the actual order of processing of the source files.
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551033551)
The MSVC link issue was kind of intermittent. I guess, it was dependent on the actual order of processing of the source files.
💬 MarcoFalke commented on pull request "test: Add missed header":
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551036780)
Ok, interesting. Maybe with iwyu this kind of bug will be fixed eventually :smiling_face_with_tear:
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551036780)
Ok, interesting. Maybe with iwyu this kind of bug will be fixed eventually :smiling_face_with_tear:
💬 fanquake commented on pull request "test: Add missed header":
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551037870)
Yea. I guess the compiler is broken? Developers should not have to worry about putting files in the "right order" for it.
(https://github.com/bitcoin-core/gui/pull/729#issuecomment-1551037870)
Yea. I guess the compiler is broken? Developers should not have to worry about putting files in the "right order" for it.
👍 hebasto approved a pull request: "build: Bump minimum supported Clang to clang-10"
(https://github.com/bitcoin/bitcoin/pull/27682#pullrequestreview-1430199164)
ACK fa199ee614a7ed99c6caf329093a3573ea5a664b
(https://github.com/bitcoin/bitcoin/pull/27682#pullrequestreview-1430199164)
ACK fa199ee614a7ed99c6caf329093a3573ea5a664b
💬 jamesob commented on issue "Use muhash for assumeUTXO snapshot ":
(https://github.com/bitcoin/bitcoin/issues/27669#issuecomment-1551051475)
See the proposal for why I didn't use muhash to begin with: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal#how-will-users-and-reviewers-efficiently-verify-hashes-of-a-given-utxo-set
That said, the original rationale (supporting snapshot chunks) doesn't seem as relevant anymore, so this might be worth revisiting...
(https://github.com/bitcoin/bitcoin/issues/27669#issuecomment-1551051475)
See the proposal for why I didn't use muhash to begin with: https://github.com/jamesob/assumeutxo-docs/tree/2019-04-proposal/proposal#how-will-users-and-reviewers-efficiently-verify-hashes-of-a-given-utxo-set
That said, the original rationale (supporting snapshot chunks) doesn't seem as relevant anymore, so this might be worth revisiting...
💬 Sjors commented on issue "Proposal for a new mempool design":
(https://github.com/bitcoin/bitcoin/issues/27677#issuecomment-1551056767)
Nice! It's also worth noting that although this proposal introduces a new limit, for the cluster size, it could potentially supersede some other limits (e.g. `-limitancestorcount`, `-limitancestorsize`, `-limitdescendantcount`, `-limitdescendantsize`) if the maximum cluster size is large enough. Though if the limit is in vbytes, then someone assuming a limit in count might still hit a non backwards compatible policy limit.
(https://github.com/bitcoin/bitcoin/issues/27677#issuecomment-1551056767)
Nice! It's also worth noting that although this proposal introduces a new limit, for the cluster size, it could potentially supersede some other limits (e.g. `-limitancestorcount`, `-limitancestorsize`, `-limitdescendantcount`, `-limitdescendantsize`) if the maximum cluster size is large enough. Though if the limit is in vbytes, then someone assuming a limit in count might still hit a non backwards compatible policy limit.
💬 hebasto commented on pull request "build: Bump minimum supported Clang to clang-10":
(https://github.com/bitcoin/bitcoin/pull/27682#issuecomment-1551090163)
Guix builds:
```
4837adede6c57a98c0bc0ddaa178870a543c3f6ce9c3c8cd47841262937637c7 guix-build-fa199ee614a7/output/arm64-apple-darwin/SHA256SUMS.part
7ff35a219b09300f27a03b00196367ae9f17effab610e440a60322b0dd6a22e8 guix-build-fa199ee614a7/output/arm64-apple-darwin/bitcoin-fa199ee614a7-arm64-apple-darwin-unsigned.dmg
4dcf0408d81b0558f30055749334b966b8b1d2269da32238ff1f219e3e027bfc guix-build-fa199ee614a7/output/arm64-apple-darwin/bitcoin-fa199ee614a7-arm64-apple-darwin-unsigned.tar.gz
169fe
...
(https://github.com/bitcoin/bitcoin/pull/27682#issuecomment-1551090163)
Guix builds:
```
4837adede6c57a98c0bc0ddaa178870a543c3f6ce9c3c8cd47841262937637c7 guix-build-fa199ee614a7/output/arm64-apple-darwin/SHA256SUMS.part
7ff35a219b09300f27a03b00196367ae9f17effab610e440a60322b0dd6a22e8 guix-build-fa199ee614a7/output/arm64-apple-darwin/bitcoin-fa199ee614a7-arm64-apple-darwin-unsigned.dmg
4dcf0408d81b0558f30055749334b966b8b1d2269da32238ff1f219e3e027bfc guix-build-fa199ee614a7/output/arm64-apple-darwin/bitcoin-fa199ee614a7-arm64-apple-darwin-unsigned.tar.gz
169fe
...