👍 dergoegge approved a pull request: "contrib: Add deterministic-unittest-coverage"
(https://github.com/bitcoin/bitcoin/pull/31901#pullrequestreview-2644695039)
tACK fa99c3b544b631cfe34d52fb5e71636aedb1b423

💬 maflcko commented on pull request "ci: limit max stack size to 512 KiB":
(https://github.com/bitcoin/bitcoin/pull/31367#issuecomment-2685182068)
If someone want to pick this up, the correct diff would likely look something like:

```diff
diff --git a/ci/test/00_setup_env_mac_native.sh b/ci/test/00_setup_env_mac_native.sh
index e01a56895b..ab1b78b14d 100755
--- a/ci/test/00_setup_env_mac_native.sh
+++ b/ci/test/00_setup_env_mac_native.sh
@@ -15,3 +15,4 @@ export BITCOIN_CONFIG="-DBUILD_GUI=ON -DWITH_ZMQ=ON -DREDUCE_EXPORTS=ON"
export CI_OS_NAME="macos"
export NO_DEPENDS=1
export OSX_SDK=""
+export CI_LIMIT_STACK_SIZE=1
diff
...

💬 maflcko commented on pull request "contrib: fix `test_deterministic_coverage.sh` script for out-of-tree builds":
(https://github.com/bitcoin/bitcoin/pull/31588#issuecomment-2685198391)
Maybe this can be closed, given that https://github.com/bitcoin/bitcoin/pull/31901 has 3 acks and is likely close to merge?

💬 laanwj commented on pull request "init: Handle dropped UPnP support more gracefully":
(https://github.com/bitcoin/bitcoin/pull/31916#issuecomment-2685202796)
> I'm a little confused by discussion above about options being overridden on the command line or configuration file, but maybe I need to think about it more. In general options being specified on command line should override GUI settings, but options specified in the GUI should override options specified in the config file.

It's all working as expected. The only thing is that the "Map port using PCP or NAT-PMP" checkbox isn't checked when `upnp=1` is specified in the configuration file.


...

✅ fanquake closed a pull request: "contrib: fix `test_deterministic_coverage.sh` script for out-of-tree builds"
(https://github.com/bitcoin/bitcoin/pull/31588)

💬 fanquake commented on pull request "contrib: fix `test_deterministic_coverage.sh` script for out-of-tree builds":
(https://github.com/bitcoin/bitcoin/pull/31588#issuecomment-2685216783)
Closing in favour of #31901.

👍 brunoerg approved a pull request: "test: add coverage for abandoning unconfirmed transaction"
(https://github.com/bitcoin/bitcoin/pull/31943#pullrequestreview-2644733871)
utACK 073a017016e62c7d52562aa61d942024379c110d

🚀 fanquake merged a pull request: "test: add coverage for abandoning unconfirmed transaction"
(https://github.com/bitcoin/bitcoin/pull/31943)

💬 maflcko commented on issue "[rfc] build: Reject unclean configure?":
(https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2685336869)
cc @ryanofsky I believe this is an alternative way to ease your concern from https://github.com/bitcoin/bitcoin/pull/31161#pullrequestreview-2644640989, assuming that 31161 triggers a re-configure.

💬 ryanofsky commented on pull request "init: Handle dropped UPnP support more gracefully":
(https://github.com/bitcoin/bitcoin/pull/31916#issuecomment-2685343640)
> The only thing is that the "Map port using PCP or NAT-PMP" checkbox isn't checked (in the options dialog) when upnp=1 is specified in the configuration file.

I see. Agree this is definitely an edge case. I think it could be fixed by replacing `SoftSetBoolArg("-natpmp", *arg)` with something like `if (!IsArgSet("-natpmp") settings.rw_settings[""]["natpmp"] = *upnp;` but this might have other problems and current version does seem ok.

💬 laanwj commented on pull request "init: Handle dropped UPnP support more gracefully":
(https://github.com/bitcoin/bitcoin/pull/31916#issuecomment-2685373064)
> with something like if (!IsArgSet("-natpmp") settings.rw_settings[""]["natpmp"] = *upnp;
> but this might have other problems and current version does seem ok.

Doesn't that make them saved options?

Have thought about that when i started, but seems undesirable to have settings from the command line / config file "leak" into `settings.json`.

⚠️ maflcko opened an issue: "callgrind_annotate broken after cmake migration?"
(https://github.com/bitcoin/bitcoin/issues/31957)
Steps to reproduce on a fresh install of Ubuntu LTS 24.04:

* Install packages and clone repo: `( export DEBIAN_FRONTEND=noninteractive && apt update && apt install git ccache make build-essential libtool cmake autotools-dev automake pkg-config bsdmainutils python3 libevent-dev libboost-dev libsqlite3-dev valgrind -y && git clone https://github.com/bitcoin/bitcoin.git b-c ) && cd b-c`
* `git checkout 338bc2cd261ba3daf7fb494f8cb4a534762e292c # cmake migration (allows to test autotools and
...

💬 tnndbtc commented on issue "build: macOS fuzz instructions broken using latest macOS linker":
(https://github.com/bitcoin/bitcoin/issues/31049#issuecomment-2685490090)
@maflcko Did you test it? _Actually, re-reading the thread, it looks like the simplest solution would be to install lld and pass -DCMAKE_CXX_FLAGS="-fuse-ld=lld"_

I tested on Mac (Apple M1 chipset, OS: Sequoia 15.1.1) with following commands and it failed on building

% brew install llvm # this will install llvm version 19
% brew install lld
% cmake --preset=libfuzzer -DCMAKE_C_COMPILER="$(brew --prefix llvm)/bin/clang" -DCMAKE_CXX_COMPILER="$(brew --prefix llvm)/bin/clang++" -DAPPEND_LDFLAGS=
...

💬 maflcko commented on issue "build: macOS fuzz instructions broken using latest macOS linker":
(https://github.com/bitcoin/bitcoin/issues/31049#issuecomment-2685512872)
> [@maflcko](https://github.com/maflcko) Did you test it? _Actually, re-reading the thread, it looks like the simplest solution would be to install lld and pass -DCMAKE_CXX_FLAGS="-fuse-ld=lld"_

I don't have a mac, so no. Theoretically, I also would have expected `-DAPPEND_LDFLAGS='-fuse-ld=lld'` instead, but I haven't tested this either.

💬 janb84 commented on pull request "doc: Update documentation to include Clang/llvm based coverage report generation":
(https://github.com/bitcoin/bitcoin/pull/31933#issuecomment-2685582112)
Concept ACK [5f7b426](https://github.com/bitcoin/bitcoin/pull/31933/commits/5f7b42642ad83faf23c8e75511f2379f9eae25ad)

Using the new instructions of the document on MacOS using Nix-shell (so using the non mac instructions) I do endup gettting the warning that was mentioned before:
**"warning: 2 functions have mismatched data"**

The rapport is still generated.

Tested both under clang 19.1.7 / 18.1.8 and LLVM 19.1.7 / 18.1.8

This is an improvement because I cannot get the "old way"
...

💬 ryanofsky commented on pull request "init: Handle dropped UPnP support more gracefully":
(https://github.com/bitcoin/bitcoin/pull/31916#issuecomment-2685607333)
> Doesn't that make them saved options?

Sorry that was just a typo, that was supposed to say `ro_config` not `rw_settings`. Again though current approach seems ok. Only suggesting if we want to address issue of checkbox showing up as unchecked in this corner case.

💬 pablomartin4btc commented on pull request "doc: Update translation generation instructions":
(https://github.com/bitcoin/bitcoin/pull/31930#issuecomment-2685717665)
_Updates_:
- Addressed @ryanofsky's feedback and updated the code with his suggestion (Thanks for the review!).

💬 Christewart commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2685732624)
thanks for this @theStack ! This was very helpful in analyzing the existence of certain types of utxos for this analysis! https://delvingbitcoin.org/t/great-consensus-cleanup-revival/710/73?u=chris_stewart_5

💬 AlexWater123456789 commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2685861086)
> Great to see that this was merged!

💬 AlexWater123456789 commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2685861769)

Perfect