💬 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).

💬 Christewart commented on pull request "tests: fix `OP_1NEGATE` handling in `CScriptOp`":
(https://github.com/bitcoin/bitcoin/pull/29589#issuecomment-2091723640)
I've updated this PR to take @achow101 's suggestion. After 08f2c78658d8261199c75c7956fecc26efc17f3a the behavior between the c++ and python codebase is the same.

As mentioned by @dgpv and @petertodd , this now touches consensus code. Here is what I see for the implications. I'm not super familar with the c++ codebase, so please let me know if I am missing some occurrences.

### `EncodeOP_N()`

<img width="245" alt="Screenshot 2024-05-02 at 4 18 30 PM" src="https://github.com/bitcoin/bit
...

📝 hebasto opened a pull request: "refactor, test: Always initialize pointer"
(https://github.com/bitcoin/bitcoin/pull/30026)
This change fixes MSVC warning [C4703](https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4703).

All `DisableSpecificWarnings` dropped from `test_bitcoin.vcxproj` as all remained are inherited from `common.init.vcxproj`.

Required to simplify warning suppression porting to the CMake-based build system.

💬 hebasto commented on pull request "refactor, test: Always initialize pointer":
(https://github.com/bitcoin/bitcoin/pull/30026#issuecomment-2091832414)
cc @jamesob as a test author

💬 fjahr commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2091915882)
I had to rebase to fix a silent merge conflict and I think I managed to make the CI happy with the tests I added. In general, I think this can be moved out of draft status now since we are discussing the details of the changes in parameters but overall there seems to be no opposition to the change. I initially had removed Testnet3 as part of this already and keeping it in place makes this much less controversial/dangerous.

I have removed the adjustments on min difficulty and changed the excep
...

👋 fjahr's pull request is ready for review: "Testnet4 including PoW difficulty adjustment fix"
(https://github.com/bitcoin/bitcoin/pull/29775)

⚠️ 09352333528 opened an issue: "Some import path needed then."
(https://github.com/bitcoin/bitcoin/issues/30027)
Some import path needed then.

_Originally posted by @paveljanik in https://github.com/bitcoin/bitcoin/issues/8035#issuecomment-218124694_

✅ 09352333528 closed an issue: "Some import path needed then."
(https://github.com/bitcoin/bitcoin/issues/30027)

:lock: fanquake locked an issue: "Some import path needed then."
(https://github.com/bitcoin/bitcoin/issues/30027)