💬 fanquake commented on pull request "build: remove `CHECK_ATOMIC`":
(https://github.com/bitcoin/bitcoin/pull/28777#issuecomment-1791057989)
Although not always, because the 32-bit CentOS build using GCC 11 compiled fine. Will adjust the docs.
(https://github.com/bitcoin/bitcoin/pull/28777#issuecomment-1791057989)
Although not always, because the 32-bit CentOS build using GCC 11 compiled fine. Will adjust the docs.
📝 fanquake converted_to_draft a pull request: "build: remove `CHECK_ATOMIC`"
(https://github.com/bitcoin/bitcoin/pull/28777)
This is not required to perform a Guix build, which uses GCC 10 (our minimum supported GCC), so if it is still needed for a certain platform / target combination:
> Some versions of gcc/libstdc++ require linking with -latomic if
> using the C++ atomic library.
we should clarify where it is needed in the documentation. Otherwise, we should remove it, and save porting this to CMake.
`libatomic.so.1` is still a runtime dependency for the riscv binaries.
```bash
0c1a2d47ea1e798469d9eb6e7
...
(https://github.com/bitcoin/bitcoin/pull/28777)
This is not required to perform a Guix build, which uses GCC 10 (our minimum supported GCC), so if it is still needed for a certain platform / target combination:
> Some versions of gcc/libstdc++ require linking with -latomic if
> using the C++ atomic library.
we should clarify where it is needed in the documentation. Otherwise, we should remove it, and save porting this to CMake.
`libatomic.so.1` is still a runtime dependency for the riscv binaries.
```bash
0c1a2d47ea1e798469d9eb6e7
...
📝 fanquake opened a pull request: "depends: drop -O1 workaround from arm64 apple Qt build"
(https://github.com/bitcoin/bitcoin/pull/28778)
Drop the workaround of setting optimization flags to -O1 for the `arm64-apple-darwin` builds. I no-longer see reproducibility issues when building across `x86_64` and `aarch64`:
```bash
real 7m21.192s
user 67m41.047s
sys 5m8.596s
fedora-32gb-hel1-1 bitcoin]# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
62373549d2884e8ef8f46a77b9a93f64ebfc88603569e9d33b68fc67beaf2226 guix-build-664c87354f9e/output/arm64-apple-darwin/S
...
(https://github.com/bitcoin/bitcoin/pull/28778)
Drop the workaround of setting optimization flags to -O1 for the `arm64-apple-darwin` builds. I no-longer see reproducibility issues when building across `x86_64` and `aarch64`:
```bash
real 7m21.192s
user 67m41.047s
sys 5m8.596s
fedora-32gb-hel1-1 bitcoin]# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
62373549d2884e8ef8f46a77b9a93f64ebfc88603569e9d33b68fc67beaf2226 guix-build-664c87354f9e/output/arm64-apple-darwin/S
...
💬 hebasto commented on pull request "depends: drop -O1 workaround from arm64 apple Qt build":
(https://github.com/bitcoin/bitcoin/pull/28778#issuecomment-1791078896)
Concept ACK. Nice :)
(https://github.com/bitcoin/bitcoin/pull/28778#issuecomment-1791078896)
Concept ACK. Nice :)
👋 fanquake's pull request is ready for review: "doc: update docs for `CHECK_ATOMIC` macro"
(https://github.com/bitcoin/bitcoin/pull/28777)
(https://github.com/bitcoin/bitcoin/pull/28777)
💬 fanquake commented on pull request "doc: update docs for `CHECK_ATOMIC` macro":
(https://github.com/bitcoin/bitcoin/pull/28777#issuecomment-1791118146)
Clang prior to version 15, when compiling for 32-bit, is the culprit here (I misread the CI). Have updated the docs.
(https://github.com/bitcoin/bitcoin/pull/28777#issuecomment-1791118146)
Clang prior to version 15, when compiling for 32-bit, is the culprit here (I misread the CI). Have updated the docs.
💬 maflcko commented on pull request "doc: update docs for `CHECK_ATOMIC` macro":
(https://github.com/bitcoin/bitcoin/pull/28777#issuecomment-1791174436)
Jup, it is still needed. Steps to reproduce on a fresh install of bullseye:
`export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git --depth=1 && cd bitcoin && apt install build-essential libtool autotools-dev automake pkg-config bsdmainutils python3-zmq make automake cmake curl clang-13 llvm-13 g++-multilib libtool binutils-gold bsdmainutils pkg-config python3 patch bison -y && ( cd depends && ma
...
(https://github.com/bitcoin/bitcoin/pull/28777#issuecomment-1791174436)
Jup, it is still needed. Steps to reproduce on a fresh install of bullseye:
`export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git --depth=1 && cd bitcoin && apt install build-essential libtool autotools-dev automake pkg-config bsdmainutils python3-zmq make automake cmake curl clang-13 llvm-13 g++-multilib libtool binutils-gold bsdmainutils pkg-config python3 patch bison -y && ( cd depends && ma
...
💬 achow101 commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#issuecomment-1791255104)
ACK a5b6481b94580ef2cc2d9c45ac1c1959e79a82b2
(https://github.com/bitcoin/bitcoin/pull/28762#issuecomment-1791255104)
ACK a5b6481b94580ef2cc2d9c45ac1c1959e79a82b2
💬 achow101 commented on pull request "Replace RecursiveMutex m_cs_banned with Mutex, and rename it":
(https://github.com/bitcoin/bitcoin/pull/24097#issuecomment-1791258148)
ACK 37d150d8c5ffcb2bddcd99951a739e97571194c7
(https://github.com/bitcoin/bitcoin/pull/24097#issuecomment-1791258148)
ACK 37d150d8c5ffcb2bddcd99951a739e97571194c7
🚀 achow101 merged a pull request: "Replace RecursiveMutex m_cs_banned with Mutex, and rename it"
(https://github.com/bitcoin/bitcoin/pull/24097)
(https://github.com/bitcoin/bitcoin/pull/24097)
💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for Spend":
(https://github.com/bitcoin/bitcoin/pull/28236#issuecomment-1791356580)
The `fuzzer` logs are unclear, I'm not sure why it failed suddenly. Maybe some issue with the Fuzzer again?
(https://github.com/bitcoin/bitcoin/pull/28236#issuecomment-1791356580)
The `fuzzer` logs are unclear, I'm not sure why it failed suddenly. Maybe some issue with the Fuzzer again?
💬 maflcko commented on pull request "fuzz: wallet, add target for Spend":
(https://github.com/bitcoin/bitcoin/pull/28236#issuecomment-1791381061)
You'll need to rebase on current master in any case
(https://github.com/bitcoin/bitcoin/pull/28236#issuecomment-1791381061)
You'll need to rebase on current master in any case
⚠️ hebasto opened an issue: "`libbitcoinconsensus.a` is unusable"
(https://github.com/bitcoin/bitcoin/issues/28779)
On Ubuntu 22.04, in the master branch @ 0857f2935f90df9c3d303582e5b62a9c8dedd9d7:
```
$ ./autogen.sh
$ ./configure --without-daemon --without-gui --without-utils --disable-tests --disable-bench --disable-fuzz-binary --disable-shared --prefix=/
$ make
$ make DESTDIR=~/CONSENSUS install
$ g++ testconsensus.cpp -o testconsensus -I ~/CONSENSUS/include -L ~/CONSENSUS/lib -l:libbitcoinconsensus.a
/usr/bin/ld: /home/hebasto/CONSENSUS/lib/libbitcoinconsensus.a(libbitcoinconsensus_la-pubkey.o): in
...
(https://github.com/bitcoin/bitcoin/issues/28779)
On Ubuntu 22.04, in the master branch @ 0857f2935f90df9c3d303582e5b62a9c8dedd9d7:
```
$ ./autogen.sh
$ ./configure --without-daemon --without-gui --without-utils --disable-tests --disable-bench --disable-fuzz-binary --disable-shared --prefix=/
$ make
$ make DESTDIR=~/CONSENSUS install
$ g++ testconsensus.cpp -o testconsensus -I ~/CONSENSUS/include -L ~/CONSENSUS/lib -l:libbitcoinconsensus.a
/usr/bin/ld: /home/hebasto/CONSENSUS/lib/libbitcoinconsensus.a(libbitcoinconsensus_la-pubkey.o): in
...
💬 hebasto commented on issue "`libbitcoinconsensus.a` is unusable":
(https://github.com/bitcoin/bitcoin/issues/28779#issuecomment-1791383638)
cc @theuni @TheCharlatan @fanquake @ryanofsky
(https://github.com/bitcoin/bitcoin/issues/28779#issuecomment-1791383638)
cc @theuni @TheCharlatan @fanquake @ryanofsky
💬 achow101 commented on pull request "test: add coverage to rpc_blockchain.py":
(https://github.com/bitcoin/bitcoin/pull/27852#issuecomment-1791405002)
ACK 376dc2cfb32806a8aa450589effe4d384e648398
(https://github.com/bitcoin/bitcoin/pull/27852#issuecomment-1791405002)
ACK 376dc2cfb32806a8aa450589effe4d384e648398
🚀 achow101 merged a pull request: "test: add coverage to rpc_blockchain.py"
(https://github.com/bitcoin/bitcoin/pull/27852)
(https://github.com/bitcoin/bitcoin/pull/27852)
🤔 kevkevinpal reviewed a pull request: "MiniMiner changes for package linearization"
(https://github.com/bitcoin/bitcoin/pull/28762#pullrequestreview-1709331080)
ACK a5b6481b94580ef2cc2d9c45ac1c1959e79a82b2
added few small nits but feel free to ignore
(https://github.com/bitcoin/bitcoin/pull/28762#pullrequestreview-1709331080)
ACK a5b6481b94580ef2cc2d9c45ac1c1959e79a82b2
added few small nits but feel free to ignore
💬 kevkevinpal commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1379509252)
nit: Should we make some of these values into a variable instead of hardcoding the number?
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1379509252)
nit: Should we make some of these values into a variable instead of hardcoding the number?
💬 kevkevinpal commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1379505261)
nit:
I just noticed `CENT/100` is used in this file for tx 5's fee aswell, not sure if it makes sense to make a common variable to use for both
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1379505261)
nit:
I just noticed `CENT/100` is used in this file for tx 5's fee aswell, not sure if it makes sense to make a common variable to use for both
💬 kevkevinpal commented on pull request "wallet: allowing walletpassphrase timeout to be MAX_SLEEP_TIME if set to -1":
(https://github.com/bitcoin/bitcoin/pull/28454#issuecomment-1791427196)
> rebased to [76b1c4c](https://github.com/bitcoin/bitcoin/pull/28454/commits/76b1c4ca6eea327328c7a0b035e6f2b1a2211e51) and since this [PR 28617](https://github.com/bitcoin/bitcoin/pull/28617) has been merged only the test-framework will be modified so I am not sure if this change is needed anymore.
>
> Thoughts @maflcko? I am happy to close this
closing this pull request
(https://github.com/bitcoin/bitcoin/pull/28454#issuecomment-1791427196)
> rebased to [76b1c4c](https://github.com/bitcoin/bitcoin/pull/28454/commits/76b1c4ca6eea327328c7a0b035e6f2b1a2211e51) and since this [PR 28617](https://github.com/bitcoin/bitcoin/pull/28617) has been merged only the test-framework will be modified so I am not sure if this change is needed anymore.
>
> Thoughts @maflcko? I am happy to close this
closing this pull request