💬 glozow commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1380289810)
9b0843042ab5c30e6ef4753d590c69c4811d9a70 seems to be doing multiple things: (1) making `CBlockPolicyEstimator` react to validationinterface and (2) delegating the `validForFeeEstimation` from validation to the `CBlockPolicyEstimator` by providing these bools in the `TransactionAddedToMempool` signal (but this isn't fully done until the next commit which cleans things up).

These should be separate commits, e.g. (2) and then (1).

💬 glozow commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1380307655)
This is quite verbose and repeated in several places. Why not create a dedicated constructor?

💬 maflcko commented on issue "Undefined behavior in AutoFile::write (gcc only)":
(https://github.com/bitcoin/bitcoin/issues/28761#issuecomment-1790993773)
Also, a suppression can be added in the meantime, if anyone is using g++

💬 glozow commented on pull request "Fuzz: Check individual and package transaction invariants":
(https://github.com/bitcoin/bitcoin/pull/28764#issuecomment-1791000546)
reACK fcb3069fa307942cf7f3edabcda1be96d615c91f, only whitespace changes

💬 maflcko commented on pull request "build: remove `CHECK_ATOMIC`":
(https://github.com/bitcoin/bitcoin/pull/28777#issuecomment-1791001127)
It may be good to get matching guix hashes from different arches before merging this?

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1380403135)
these fields were added to solve silent merge conflict https://github.com/bitcoin/bitcoin/pull/25380#issuecomment-1713512238 , but you are right they should be added when needed.

💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1380403540)
This is fixed in latest push.

💬 fanquake commented on pull request "build: remove `CHECK_ATOMIC`":
(https://github.com/bitcoin/bitcoin/pull/28777#issuecomment-1791049082)
Looks like it's still required for at least 32-bit Linux, using GCC 11.

💬 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.

📝 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
...

📝 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
...

💬 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 :)

👋 fanquake's pull request is ready for review: "doc: update docs for `CHECK_ATOMIC` macro"
(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.

💬 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
...

💬 achow101 commented on pull request "MiniMiner changes for package linearization":
(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

🚀 achow101 merged a pull request: "Replace RecursiveMutex m_cs_banned with Mutex, and rename it"
(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?

💬 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