💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1115361129)
Closing as I think this is resolved

💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1115385741)
Updated in 56e370fbb9413260723c598048392219b1055ad0

💬 willcl-ark commented on pull request "docs: add ramdisk guide for running tests on OSX":
(https://github.com/bitcoin/bitcoin/pull/27124#issuecomment-1441415357)
ACK 2f84ad7b9e62dd710940c2f265b65973b94864d7

💬 fanquake commented on pull request "Fix various libbitcoinkernel DLL build problems":
(https://github.com/bitcoin/bitcoin/pull/27146#issuecomment-1441455198)
Concept ACK

💬 fanquake commented on pull request "build: Build `libbitcoinconsensus` from its own convenience library":
(https://github.com/bitcoin/bitcoin/pull/24994#issuecomment-1441457340)
Concept NACK. In general, I don't like this approach of refactoring code, and adding libraries, to workaround issues with build tooling, especially when its a problem that only exists for a single platform (let alone the DEBUG build, and ). I think this should just be closed in light of #27146 and the migration towards CMake.

🤔 TheCharlatan requested changes to a pull request: "Fix various libbitcoinkernel DLL build problems"
(https://github.com/bitcoin/bitcoin/pull/27146)

💬 TheCharlatan commented on pull request "Fix various libbitcoinkernel DLL build problems":
(https://github.com/bitcoin/bitcoin/pull/27146#discussion_r1115408165)
Can you comment on what this `-static` flag is here for?

💬 TheCharlatan commented on pull request "Fix various libbitcoinkernel DLL build problems":
(https://github.com/bitcoin/bitcoin/pull/27146#discussion_r1115441530)
Is there a way to check this? I tried running:
```
make distclean
./autogen.sh
CONFIG_SITE=~/bitcoin/depends/x86_64-w64-mingw32/share/config.site ./configure \
--with-experimental-kernel-lib \
--enable-experimental-util-chainstate \
--with-experimental \
--prefix ~/bitcoin/install_dir \
--disable-static
make install
```
But I'm getting a bunch of linker errors:
```
/usr/bin/x86_64-w64-mingw32-ld: /usr/lib/gcc/x86_64-w64-mingw32/10-posix/libssp.a(ssp.o):(.text+0xe0): multiple defi
...

💬 fanquake commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1441472941)
> Maybe keep this PR diff CI-driven? (I mean, the CI [task](https://cirrus-ci.com/task/4897437224009728) passes now)

If there are additional "fixes" being pointed out, which are correct, and based on the same rules that we are trying to apply, I don't really see the point of leaving those changes out, just to have to fix them later (upgrading our tooling here is another point).

I've been a little bit concerned that a lot of these clang-tidy changes have just been blindly following the tool
...

🚀 fanquake merged a pull request: "docs: add ramdisk guide for running tests on OSX"
(https://github.com/bitcoin/bitcoin/pull/27124)

✅ hebasto closed a pull request: "build: Build `libbitcoinconsensus` from its own convenience library"
(https://github.com/bitcoin/bitcoin/pull/24994)

💬 Ely1969 commented on pull request "build: Build `libbitcoinconsensus` from its own convenience library":
(https://github.com/bitcoin/bitcoin/pull/24994#issuecomment-1441549203)
Very nicr

💬 Sjors commented on pull request "wallet: skip R-value signature grinding for external signers":
(https://github.com/bitcoin/bitcoin/pull/26032#issuecomment-1441567529)
@S3RK done!

💬 vasild commented on pull request "doc: clarify that LOCK() does AssertLockNotHeld() internally":
(https://github.com/bitcoin/bitcoin/pull/27116#discussion_r1115561797)
Yes, if `LOCK()` is moved, then it would have to be reviewed either way.

💬 Ely1969 commented on pull request "doc: clarify that LOCK() does AssertLockNotHeld() internally":
(https://github.com/bitcoin/bitcoin/pull/27116#discussion_r1115568025)
Hoe to merge

💬 naumenkogs commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1441727779)
Concept ACK

⚠️ MarcoFalke opened an issue: "miniscript_stable fuzz timeout"
(https://github.com/bitcoin/bitcoin/issues/27147)
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56270

👍 furszy approved a pull request: "wallet: skip R-value signature grinding for external signers"
(https://github.com/bitcoin/bitcoin/pull/26032)
ACK 302f07ee
Looking quite straightforward now.

💬 furszy commented on pull request "wallet: skip R-value signature grinding for external signers":
(https://github.com/bitcoin/bitcoin/pull/26032#discussion_r1115670527)
tiny nit:
could cache this result into a variable outside of the loop (after the `only_safe` variable)

💬 furszy commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#issuecomment-1441833350)
All good about the none usage of the "-" prefix on the GUI 👍🏼 .

> The provided method executeConsoleOnlyCommand is almost identical to the suggested ExecuteCliCommand and is also ready for potential future new console only commands, the only difference is that it detects ( like IsCliOnlyCommand ) and executes commands in a single method.

I'm not fan of the current method mostly because of all the `!parsedCommand.empty()` and `parsedCommand.size() > something` and `parsedCommand.size() <
...