💬 stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2090961175)
note: this logic is different to `RecalculateBestHeader`: https://github.com/bitcoin/bitcoin/blob/c779ee3a4044df3a263394bbb8b104aeeaa7c727/src/validation.cpp#L6404

From our [earlier conversation](https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2059128406) it seems like `CBlockIndexWorkComparator` is the correct choice here, so probably this can be fixed (if necessary) in a separate pull, but it seems worth raising here? Relatedly, I tried modifying `CheckBlockIndex` to use `CBlockIn
...

📝 Sjors opened a pull request: "rfc: only put replaced txs in extra pool"
(https://github.com/bitcoin/bitcoin/pull/32510)
We currently store rejected mempool transactions in the extra pool. This includes (some) policy and consensus rejected transactions.

It seems trivial to attack this pool for free with consensus invalid transactions, or almost free with policy violating transactions that are extremely unlikely to ever get mined.

At the same time the orphanage already handles some cases that were initially dealt with here.

This commit simplifies things by only putting replaced transactions in the extra po
...

💬 Sjors commented on pull request "rfc: only put replaced txs in extra pool":
(https://github.com/bitcoin/bitcoin/pull/32510#discussion_r2091044581)
Not sure what to replace this check with, but now it doesn't check anything.

💬 brunoerg commented on pull request "test: descriptor: cover invalid multi/multi_a cases":
(https://github.com/bitcoin/bitcoin/pull/32504#discussion_r2091049875)
Yes, this is weird. I don't know why.

💬 maflcko commented on pull request "test: descriptor: cover invalid multi/multi_a cases":
(https://github.com/bitcoin/bitcoin/pull/32504#discussion_r2091058887)
The other lines in this test are excessively long. It would be better to use c-style multi-line string:

```c
CheckUnparsable("multi("
"bla,"
"foo,"
);
```

However, this seems unrelated to the changes here and would require reformatting the whole file.

🤔 ryanofsky reviewed a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2843363337)
Updated 4e1aae19512df82af584a064640c2143c5c5fa4f -> 7af6e1089ea264e870b26ac83e81e7aa374acbe1 ([`pr/wrap.32`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.32) -> [`pr/wrap.33`](https://github.com/ryanofsky/bitcoin/commits/pr/wrap.33), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/wrap.32..pr/wrap.33)) with suggestions adding windows uninstall line, removing no longer used cmake build prefixes, and improving many comments.

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091049127)
re: https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2087629089

Thanks, added a code comment with this information

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091019509)
re: https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2087583779

Just added a note to the `GetExePath` doc, because I'm not really sure `std::nullopt` is better representation of a path that could not be determined than `path.empty()`. Returning {empty path, nonempty path} seems safer than returning {null path, empty path, nonempty path} if the goal is to avoid bugs.

Theoretically if we had a `not_empty<T>` wrapper which asserted the wrapped value is non-empty on construction maybe
...

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2090970340)
re: https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2086894554

Good catch, fixed!

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091026948)
re: https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2087585166

Thanks, added a similar comment

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2090999369)
re: https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2087562615

I think they are somewhat distinct. The subprocess header is trying to mimic python's subprocess API and provides functions for starting and interacting with child processes. This header provides functions for getting information about and replacing the current process that don't have any equivalent in python's subprocess API. So not sure I'd want to merge them, though it could be a possibility.

For copyright probably
...

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091025097)
re: https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2087585166

Good catch, added.

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2090976743)
re: https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2086920895

> are these prepended paths (here and also for the `gui`, `bench` and `test` commands above) still relevant? They seem to reflect a folder structure that doesn't exist anymore since #31161

Good find, removed these. The prefixes were harmless but no longer do anything after the update in https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2819431273 following the #31161 merge.

💬 ryanofsky commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091059112)
re: https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2087636740

Thanks to both for the suggestion and testing. Added this line

💬 Sjors commented on pull request "rfc: only put replaced txs in extra pool":
(https://github.com/bitcoin/bitcoin/pull/32510#issuecomment-2883701666)
cc @0xB10C do you have any stats on what rejected stuff typically ends up in the extra pool?

💬 Sjors commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2883726902)
re-utACK 7af6e1089ea264e870b26ac83e81e7aa374acbe1

I didn't retest.

📝 maflcko opened a pull request: "refactor: bdb removals"
(https://github.com/bitcoin/bitcoin/pull/32511)
remove dead code

💬 glozow commented on pull request "rfc: only put replaced txs in extra pool":
(https://github.com/bitcoin/bitcoin/pull/32510#issuecomment-2883734377)
Agree that a FIFO ring buffer isn't a robust data structure; it's opportunistic as it says on the can. However, I think deleting `vExtraTxnForCompact` because it's trivially DoSable is a bit like giving up on orphan resolution because the orphanage is trivially DoSable... it seems more productive to try to come up with ways to make it less vulnerable to churn.

I have to imagine that especially with policy changes happening, this vector is useful (yes, in the absence of attackers). If you don'
...

💬 hebasto commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2883738240)
Updated c0f41523973e33a8cd104a8ebf4906a4d99f3eed -> 8f4fed7ec70093e2535423d63e9f9dd400c378ac ([pr32396.08](https://github.com/hebasto/bitcoin/commits/pr32396.08) -> [pr32396.09](https://github.com/hebasto/bitcoin/commits/pr32396.09), [diff](https://github.com/hebasto/bitcoin/compare/pr32396.08..pr32396.09)):

The [feedback](https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2883491267) from @fanquake has been addressed. Thank you!

⚠️ fanquake opened an issue: "build: Alpine portability issues"
(https://github.com/bitcoin/bitcoin/issues/32512)
Currently, when building on Alpine Linux, the build might fail in the following ways:

x86_64 && aarch64

```bash
/bitcoin/depends/work/build/x86_64-pc-linux-musl/native_capnp/1.1.0-725f3670538/src/kj/mutex.c++:37:10: fatal error: linux/futex.h: No such file or directory
37 | #include <linux/futex.h>
| ^~~~~~~~~~~~~~~
compilation terminated.
```
```bash
/bitcoin/depends/work/build/x86_64-pc-linux-musl/qt/6.7.3-be399392c6d/qtbase/src/corelib/io/qfilesystemengine_unix.cpp:64:12:
...