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

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

(This is replying to an old comment but I was going through these and resolving them).

Thanks for the diff and providing a concrete suggestion. I do not like this approach because it does the wrong thing and would execute multiple commands instead of a single command if execvp behaved the same way as dozens of other posix functions and returned 0 on success, -1 on failure. I understand execvp is special because it has
...

💬 willcl-ark commented on issue "Depends toolchain doesn't contain enough info to build from depends on a fresh NixOS install":
(https://github.com/bitcoin/bitcoin/issues/32428#issuecomment-2883781196)
OK I've taken a first stab at implementing this and the provider sounds like a tidy fix...

My current branch is here: https://github.com/bitcoin/bitcoin/compare/master...willcl-ark:bitcoin:cmake-depdency-provider

It appears to work quite nicely, for a single host build on NixOS, so long as QT is excluded, at least. (I can't seem to get the provider to pick up the various XCB components from Qt properly, and a bit lost on how to proceed with this now).

💬 nervana21 commented on pull request "doc: Add missing top-level description to pruneblockchain RPC":
(https://github.com/bitcoin/bitcoin/pull/32333#issuecomment-2883783441)
Thanks for the feedback :)

I’ve updated the description to clarify that pruning is conditional (not guaranteed), and that pruned data may be re-fetched in some cases (e.g., via getblockfrompeer). As such, the action is not strictly irreversible. Let me know if you'd like to see any further changes.

💬 darosior commented on pull request "qa: make feature_assumeutxo.py test more robust":
(https://github.com/bitcoin/bitcoin/pull/32117#issuecomment-2883790642)
My rationale for this PR was to make #32155 easier to review. Since it is now merged, i think my time would be better spent on other changes so i'm going to close this one.

✅ darosior closed a pull request: "qa: make feature_assumeutxo.py test more robust"
(https://github.com/bitcoin/bitcoin/pull/32117)

💬 glozow commented on pull request "rfc: only put replaced txs in extra pool":
(https://github.com/bitcoin/bitcoin/pull/32510#discussion_r2091160676)
Any thoughts on storing transactions from outbounds only? In this function we'd be able to query who the announcers were (that we still remember). net_processing also knows who sent the transaction.