💬 TheCharlatan commented on pull request "guix: pass `--enable-initfini-array` to release GCC":
(https://github.com/bitcoin/bitcoin/pull/27153#issuecomment-1450562166)
I can confirm that this adds relevant `.fini_array`sections and gets rid of the `.ctors` and `.dtors` sections in the bitcoin x86 _64 linux binaries by comparing the binaries of this PR's `HEAD` and its `HEAD~1`.

`objdump -h bitcoind` on fc733e9 (HEAD) :
```
bitcoind: file format elf64-x86-64
Sections:
Idx Name Size VMA LMA File off Algn
0 .interp 0000001c 0000000000000350 0000000000000350 00000350 2**0
CONTENT
...

💬 TheCharlatan commented on pull request "Deduplicate bitcoind and bitcoin-qt init code":
(https://github.com/bitcoin/bitcoin/pull/27150#issuecomment-1450575944)
ACK 802cc1ef536e11944608fe9ab782d3e962037703

💬 jonatack commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1450599651)
@pinheadmz Thanks for working on unit test coverage -- will circle back soon.

💬 hernanmarino commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1122161205)
I ' ll actually remove the variable `executableCommand` since it's only used once, I prefer avoiding memory overhead and better speed execution (althoug minimal). Okey with the other 2 variables

💬 hernanmarino commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1122164062)
Indentation was introduced after running the script suggested by @ jarolrod and are apparently ok according to clang format standard.

💬 hernanmarino commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1122165135)
Removed extra parenthesis, and I agree with extra long lines, but as I said before, indentation was introduced after running the script suggested by @ jarolrod and are apparently ok according to clang format standard.

💬 furszy commented on pull request "Mask values on Transactions View":
(https://github.com/bitcoin-core/gui/pull/708#discussion_r1122170162)
If the view is not accessible anymore, this changes aren't needed.

(same for the transaction table model ones)

💬 furszy commented on pull request "rpc: make importaddress compatible with descriptors wallet":
(https://github.com/bitcoin/bitcoin/pull/27034#discussion_r1122178962)
> > upgraded the legacy wallet into a descriptors wallet
>
> I think it's best to have only one way to go from a legacy wallet to a descriptor wallet (the migration RPC), so in the long run it's easier to keep testing this stuff.

yeah, agree.

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1450711073)
@willcl-ark thanks, I added comments and release notes.

I also wrote a tiny testing package using [libjson-rpc-cpp](https://github.com/cinemast/libjson-rpc-cpp) to check against an "outside" JSON-RPC 2.0 implementation. The package is https://github.com/pinheadmz/jsonrpc-bitcoin and [seems to like the 2.0 implementation so far.](https://github.com/pinheadmz/jsonrpc-bitcoin#examples)

💬 pablomartin4btc commented on pull request "Mask values on Transactions View":
(https://github.com/bitcoin-core/gui/pull/708#discussion_r1122210010)
yeah, I'll do.

💬 mzumsande commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1122020763)
if this is only necessary for windows, would it be possible to hide it behind `#ifdef WIN32` so that the unit tests don't run 3*2=6 seconds longer for everyone else (which is quite a lot)?

💬 fanquake commented on issue "miniscript_stable fuzz timeout":
(https://github.com/bitcoin/bitcoin/issues/27147#issuecomment-1450764766)
Verified as fixed: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56332#c3

💬 fanquake commented on pull request "Make miniscript_{stable,smart} fuzzers avoid too large scripts":
(https://github.com/bitcoin/bitcoin/pull/27165#issuecomment-1450765037)
Verified as fixed: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56332#c3.

💬 sipa commented on issue "miniscript_stable fuzz timeout":
(https://github.com/bitcoin/bitcoin/issues/27147#issuecomment-1450771004)
@fanquake Wrong issue, actually; the original was about `miniscript_stable`: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56270

The one you link to was about `miniscript_smart`, but that is in fact also addressed by #27165.

💬 fanquake commented on issue "miniscript_stable fuzz timeout":
(https://github.com/bitcoin/bitcoin/issues/27147#issuecomment-1450773295)
> Wrong issue, actually;

Whoops. Yes, both ended up being addressed here.

💬 fanquake commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1450801900)
Rebased past #27172.

💬 fanquake commented on pull request "build: LLVM 15 & LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1450803042)
Rebased past #27172.

💬 pinheadmz commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1122285875)
Thanks, I guarded the sleep for WIN32, cleaned up the lint and mentioned the undo data in OP.

💬 vostrnad commented on pull request "BIP324: Enable v2 P2P encrypted transport":
(https://github.com/bitcoin/bitcoin/pull/24545#issuecomment-1450843657)
Is there a reason the `v2transport` option should be off by default? Does it even need to exist at all? I was accidentally running this for a few days with v2 turned off and was confused why nodes advertising `P2P_V2` weren't actually using the v2 protocol. Everything works fine now.

Also, it would be nice to have the `transport_protocol_type` and `v2_session_id` fields displayed in the GUI, although that might possibly belong to a separate PR in the GUI repo (not sure what the development pr
...

💬 TheCharlatan commented on pull request "guix: pass `--enable-initfini-array` to release GCC":
(https://github.com/bitcoin/bitcoin/pull/27153#issuecomment-1450910713)
Guix builds [127c637](https://github.com/bitcoin/bitcoin/pull/27153/commits/127c637cf0a80e0ea68a7c5aaa088e5ccc9d3d13)

```
ae145877193079ceb71b86582b86294efba941d8898480e16920f8588bb25fca guix-build-127c637cf0a8/output/aarch64-linux-gnu/SHA256SUMS.part
b50822b488f79eba521dca2538b98ced15da3f175deeccce4030a6690bd4b077 guix-build-127c637cf0a8/output/aarch64-linux-gnu/bitcoin-127c637cf0a8-aarch64-linux-gnu-debug.tar.gz
a04d495f8338dc30afdc258df91708c20197f74fe7c84070d8e9ebc9d3924780 guix-bui
...

💬 ryanofsky commented on pull request "Fix BaseIndex::Commit false error":
(https://github.com/bitcoin/bitcoin/pull/26903#issuecomment-1450928417)
I agree 1c67b495cb2de76723c404f42d0bee219cc765c3 would fix the problem, but I think it suppresses errors too broadly. The `Commit()` function is called many places, so changing it to silently skip the `CustomCommit()` call and return `true` when it doesn't do anything could make it harder debug other problems in the future, or even know about them.

I think it would be better to apply a more targeted fix of just not calling `Commit()` when there is nothing to commit in the sync thread:

```d
...