💬 murchandamus commented on pull request "wallet: Reset chain notifications handler if AttachChain fails":
(https://github.com/bitcoin/bitcoin/pull/29243#issuecomment-1890247334)
Any idea why this didn’t fail in the CI on https://github.com/bitcoin/bitcoin/pull/28838 before getting merged?

💬 achow101 commented on pull request "wallet: Reset chain notifications handler if AttachChain fails":
(https://github.com/bitcoin/bitcoin/pull/29243#issuecomment-1890247599)
> Huh, any clue as to why this only becomes an issue on Windows?

Presumably only Windows is checking whether other things have the file open before attempting to remove them. Since the other shared_ptr reference was still hanging around in memory, bitcoind still had the database files open when `RestoreWallet` tries to delete the files for its cleanup on failure.

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1890248185)
CI failure is unrelated, see https://github.com/bitcoin/bitcoin/pull/29243

💬 achow101 commented on pull request "wallet: Reset chain notifications handler if AttachChain fails":
(https://github.com/bitcoin/bitcoin/pull/29243#issuecomment-1890248958)
> Any idea why this didn’t fail in the CI on #28838 before getting merged?

Apparently we weren't running on functional tests in the Windows CI until recently: #29059

💬 jamesob commented on pull request "wallet: Reset chain notifications handler if AttachChain fails":
(https://github.com/bitcoin/bitcoin/pull/29243#issuecomment-1890285738)
ACK https://github.com/bitcoin/bitcoin/pull/29243/commits/ea2551e55d260854a5cca8aa95034970d4adca1c

CI is happy.

✅ jamesob closed a pull request: "test: unbreak: exclude windows from wallet_assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/29238)

💬 jamesob commented on pull request "test: unbreak: exclude windows from wallet_assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/29238#issuecomment-1890285982)
Closing in lieu of https://github.com/bitcoin/bitcoin/pull/29243.

💬 maflcko commented on pull request "wallet: Reset chain notifications handler if AttachChain fails":
(https://github.com/bitcoin/bitcoin/pull/29243#issuecomment-1890391142)
Is this for backport?

🤔 hebasto reviewed a pull request: "build: Bump clang minimum supported version to 14"
(https://github.com/bitcoin/bitcoin/pull/29208#pullrequestreview-1819951236)
Post-merge ACK aaaace2fd1299939c755c281b787df0bbf1747a0.

🤔 hebasto reviewed a pull request: "ci: Rename tasks (previous releases, macOS cross)"
(https://github.com/bitcoin/bitcoin/pull/29218#pullrequestreview-1819952922)
Post-merge ACK fa0c594b33970e12d97e6879ab4ca57045453492.

@maflcko

Thank you for addressing https://github.com/bitcoin/bitcoin/pull/29203#discussion_r1446339387.

💬 hebasto commented on pull request "build: Drop `ALLOW_HOST_PACKAGES` support in depends":
(https://github.com/bitcoin/bitcoin/pull/29203#discussion_r1451423102)
Thanks for https://github.com/bitcoin/bitcoin/pull/29218 :)

💬 hebasto commented on pull request "build: Pass sanitize flags to instrument `libsecp256k1` code":
(https://github.com/bitcoin/bitcoin/pull/28875#issuecomment-1890400163)
> It might be better to reverse the order of the commits or squash them to avoid bisect failures in between.

Squashed.

👍 hebasto approved a pull request: "depends: Allow PATH with spaces in directory names."
(https://github.com/bitcoin/bitcoin/pull/29237#pullrequestreview-1819967328)
ACK 4756114e505cff8848fb6344ef9a48d8822066c1, successfully built depends on Ubuntu 22.04.

💬 hebasto commented on pull request "Fix typos":
(https://github.com/bitcoin-core/gui/pull/787#issuecomment-1890409751)
These changes are not GUI-specific. Therefore, they should be submitted to the main repository, i.e., https://github.com/bitcoin/bitcoin.

✅ hebasto closed a pull request: "Fix typos"
(https://github.com/bitcoin-core/gui/pull/787)

💬 TheCharlatan commented on pull request "depends: remove dependency on Darwin libtool":
(https://github.com/bitcoin/bitcoin/pull/29232#issuecomment-1890414477)
Guix builds:
```
73062e70b4ec1fa814c0c25f12cbbcfbabfd3a5e82827a3bfeb2f29b30d0b84f guix-build-a28c6bbde0bc/output/aarch64-linux-gnu/SHA256SUMS.part
0965ed91f84129a64b6e0873b5feb182d83cae0dac6d89a51667d5fbe46fc68c guix-build-a28c6bbde0bc/output/aarch64-linux-gnu/bitcoin-a28c6bbde0bc-aarch64-linux-gnu-debug.tar.gz
87ee5bc2103fc10b8e7acbfa776430918538f05fccf024b624f224f73987f745 guix-build-a28c6bbde0bc/output/aarch64-linux-gnu/bitcoin-a28c6bbde0bc-aarch64-linux-gnu.tar.gz
c9bad44733b9f9a90d2
...

💬 hebasto commented on pull request "depends: remove dependency on Darwin libtool":
(https://github.com/bitcoin/bitcoin/pull/29232#issuecomment-1890414655)
Concept ACK.

👍 TheCharlatan approved a pull request: "depends: Allow PATH with spaces in directory names."
(https://github.com/bitcoin/bitcoin/pull/29237#pullrequestreview-1819985102)
ACK 4756114e505cff8848fb6344ef9a48d8822066c1

💬 darosior commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1451496426)
> nit: I think constants are not supposed to be prefixed with g_

https://github.com/bitcoin/bitcoin/blob/3ba8de1b704d590fa4e1975620bd21d830d11666/doc/developer-notes.md?plain=1#L87-L93

> I think you could also just use the genesis block (Params().GenesisBlock()) for these two values.

Done.

💬 darosior commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1451496443)
Done.