💬 laanwj commented on issue "importaddress failed, Only legacy wallets are supported by this command.":
(https://github.com/bitcoin/bitcoin/issues/29772#issuecomment-2091363954)
Yes, adding an example would be useful.

💬 instagibbs commented on pull request "doc: replace remaining "520" magic nums with MAX_SCRIPT_ELEMENT_SIZE":
(https://github.com/bitcoin/bitcoin/pull/30024#issuecomment-2091366440)
ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860

💬 sipsorcery commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1588252435)
Does that mean this PR is intended to be a fix for Windows or for wine?

🤔 mzumsande reviewed a pull request: "net: don't lock cs_main while reading blocks in net processing"
(https://github.com/bitcoin/bitcoin/pull/26326#pullrequestreview-2036711916)
Concept ACK

💬 mzumsande commented on pull request "net: don't lock cs_main while reading blocks in net processing":
(https://github.com/bitcoin/bitcoin/pull/26326#discussion_r1588242121)
As written somewhere above, we never prune blocks > 288 blocks from the tip, and if pruning, we should have disconnected any peer requesting older blocks - so it should be impossible to trigger this log even with pruning.
On the other hand, it could still be hit while not pruning due to disk problems - e.g. helping other peers with IBD. Searching for this assert gives me multiple matches, e.g.
* #6263
* #5668
* #12230
* https://bitcoin.stackexchange.com/questions/113218/how-to-fix-the-bitc
...

💬 sipsorcery commented on pull request "refactor, fuzz: Make 64-bit shift explicit":
(https://github.com/bitcoin/bitcoin/pull/30017#issuecomment-2091446475)
utACK b50d127a7710d790c2ba4a08f01b832c2a0b1203

💬 willcl-ark commented on pull request "Fix misleading signmessage error with segwit":
(https://github.com/bitcoin-core/gui/pull/819#issuecomment-2091482504)
Sorry, I've addressed the linter now.

💬 achow101 commented on pull request "refactor: remove remaining unused code from cpp-subprocess":
(https://github.com/bitcoin/bitcoin/pull/29961#issuecomment-2091483193)
ACK 8b52e7f628304e83b0e36fd97e617de0f71c5a62

💬 andrewtoth commented on pull request "net: don't lock cs_main while reading blocks in net processing":
(https://github.com/bitcoin/bitcoin/pull/26326#discussion_r1588303739)
> so it should be impossible to trigger this log even with pruning

Since we have a 2 block buffer, peers can request up to 290 blocks. So one way this can be triggered - a peer requests block 290 and we get the filepos, we release the lock, `pruneblockchain` RPC is called, rpc thread acquires the lock and block 290 is pruned before we can acquire the fd, read now fails.
However, `pruneblockchain` has a 2 hour buffer, so there must also be a long window of no new blocks.
I don't think this i
...

💬 willcl-ark commented on pull request "Multiprocess bitcoin":
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-2091500828)
I've been using this in a side-project and noticed some unintended behaviour, which could be my system; if I `make distclean` (removing the generated files) and then `make -j16` the build fails as it can't find the built artefacts. Running plain `make` works fine every time, and _sometimes_ a low job number works too.

The Makefile [appears](https://github.com/ryanofsky/bitcoin/blob/56ef459b573461087fbe4f39d9b0a7108936335f/src/Makefile.am#L1083-L1098) (to me) to have the dependencies listed co
...

💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1588314813)
For both. Wine is used to run test suite on Linux after cross-compiling for Windows. This need root as Wine and Windows runtime produce different error messages: https://github.com/bitcoin/bitcoin/blob/db73ac8c517f91ed5175b3635eb37ed56852ff91/src/test/system_tests.cpp#L80

💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r1588317380)
FWIW, the same behaviour was before https://github.com/bitcoin/bitcoin/pull/29489.

🚀 achow101 merged a pull request: "refactor: remove remaining unused code from cpp-subprocess"
(https://github.com/bitcoin/bitcoin/pull/29961)

📝 willcl-ark opened a pull request: "doc: fix broken relative md links"
(https://github.com/bitcoin/bitcoin/pull/30025)
These relative links in our documentation are broken, fix them.

💬 achow101 commented on pull request "test: Validate UTXO snapshot with coin height > base height & amount > MAX_MONEY supply":
(https://github.com/bitcoin/bitcoin/pull/29617#issuecomment-2091513768)
ACK ec1f1abfefa281e62bb876aa1c4738d576ef9a47

💬 hebasto commented on pull request "Fix misleading signmessage error with segwit":
(https://github.com/bitcoin-core/gui/pull/819#discussion_r1588327277)
It is https://github.com/bitcoin/bitcoin/pull/29494 goal to include this headers unguarded :)

Effectively, this PR conflicts with it. However, I believe it will be merged soon.

Anyway, it seems reasonable in this PR to use:
```c++
#if defined(HAVE_CONFIG_H)
#include <config/bitcoin-config.h>
#endif
```
or
```c++
#include <config/bitcoin-config.h> // IWYU pragma: keep
```

🚀 achow101 merged a pull request: "test: Validate UTXO snapshot with coin height > base height & amount > MAX_MONEY supply"
(https://github.com/bitcoin/bitcoin/pull/29617)

💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#issuecomment-2091525067)
Rebased on top of the merged #29961.

💬 willcl-ark commented on issue "qa: Support git worktrees when running the linters locally via Docker":
(https://github.com/bitcoin/bitcoin/issues/29972#issuecomment-2091588721)
I think it _can_ be done, although it's quite hacky:

```bash
DOCKER_BUILDKIT=1 docker build -t bitcoin-linter --file "./ci/lint_imagefile" ./ && GIT_DIR=$(git rev-parse --path-format=absolute --git-common-dir) docker run --rm -v $(pwd):/bitcoin -v "$GIT_DIR":"$GIT_DIR" -it bitcoin-linter
```

This mounts the external (real) `.git` directory (which is a symlink in the worktree, and not possible to follow from inside of the container) to the same location inside of the container, making the
...

👍 hebasto approved a pull request: "build: Assume HAVE_CONFIG_H, Add IWYU pragma keep to bitcoin-config.h includes"
(https://github.com/bitcoin/bitcoin/pull/29494#pullrequestreview-2036914913)
re-ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3, only rebased since my recent [review](https://github.com/bitcoin/bitcoin/pull/29494#pullrequestreview-2028864535) (`timedata.cpp` removed in https://github.com/bitcoin/bitcoin/pull/29623).