💬 hebasto commented on pull request "doc: Improve `dependencies.md`":
(https://github.com/bitcoin/bitcoin/pull/31895#discussion_r2091435084)
nit:
```suggestion
| CMake | [link](https://cmake.org/download/) | [3.22](https://github.com/bitcoin/bitcoin/pull/30454) |
```

📝 instagibbs opened a pull request: "functional test: add MAX_DISCONNECTED_TX_POOL_BYTES coverage"
(https://github.com/bitcoin/bitcoin/pull/32516)
`DisconnectedBlockTransactions::LimitMemoryUsage()` has unit test coverage, but the default value end to end doesn't have coverage.

This test adds exercised coverage of memory limiting of the disconnect pool, and some basic behavior sanity checks.

I didn't assert the exact order of pruning, but can if desired.

💬 hebasto commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2884220162)
My Guix build:
```
aarch64
14f6c52455cfd4f871e97271938e906137d0a0eac8adb381cf22775e71e9662a guix-build-8f4fed7ec700/output/aarch64-linux-gnu/SHA256SUMS.part
793ebdb24d8956a0829c59074e3a5748b0914aa34f076956a6d0b4ef09274187 guix-build-8f4fed7ec700/output/aarch64-linux-gnu/bitcoin-8f4fed7ec700-aarch64-linux-gnu-debug.tar.gz
6bc3f991fce9e2f3dbd9e8aafd3f5a3957f92fdef478c1b95af2e5badfaddc30 guix-build-8f4fed7ec700/output/aarch64-linux-gnu/bitcoin-8f4fed7ec700-aarch64-linux-gnu.tar.gz
e475ede2
...

💬 instagibbs commented on pull request "functional test: add MAX_DISCONNECTED_TX_POOL_BYTES coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2091440961)
MiniWallet doesn't appear to support more than 49 utxos at a time? Unclear to me what's going on so I did this instead.

💬 furszy commented on issue "test: wallet_reorgsrestore.py failure under valgrind":
(https://github.com/bitcoin/bitcoin/issues/32493#issuecomment-2884223781)
We could try expanding the shutdown procedure to ensure the app's lock can be acquired before returning.
This would guarantee that the process can be re-initiated at any time afterward. Thinking of something like: https://github.com/furszy/bitcoin-core/commit/c6c9723f523c74615d784788b51e3365c5bee7c0

If this fails, it would mean the lock file is not being released at all after the process is killed (so far, we only can guess that during Valgrind runs, the lock release process might be taking lon
...

✅ fanquake closed an issue: "test: wallet_reorgsrestore.py failure under valgrind"
(https://github.com/bitcoin/bitcoin/issues/32493)

🚀 fanquake merged a pull request: "ci: Exclude failing wallet_reorgsrestore.py from valgrind task for now"
(https://github.com/bitcoin/bitcoin/pull/32507)

💬 fanquake commented on pull request "doc: Improve `dependencies.md`":
(https://github.com/bitcoin/bitcoin/pull/31895#discussion_r2091455024)
Anything here seems fine, we don't link to https://sqlite.org/download.html or https://freetype.org/download.html.

💬 szarka commented on pull request "rfc: only put replaced txs in extra pool":
(https://github.com/bitcoin/bitcoin/pull/32510#issuecomment-2884240017)
> I have to imagine that especially with policy changes happening, this vector is useful (yes, in the absence of attackers). If you don't support 1p1c yet, you'll currently be throwing the `TX_RECONSIDERABLE` parent and `TX_MISSING_INPUTS` child in there. Same if you don't support TRUC yet. I think I had a comment somewhere about returning false for `TX_CONSENSUS` errors.
>
> @0xB10C was collecting data on how often we use this for compact block relay reconstruction I think? Correct me if I'm
...

💬 maflcko commented on pull request "doc: Improve `dependencies.md`":
(https://github.com/bitcoin/bitcoin/pull/31895#discussion_r2091461356)
I don't any x11 packages are listed here, so it seems unrelated?

🤔 ismaelsadeeq reviewed a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2843509023)
Code review 7af6e1089ea264e870b26ac83e81e7aa374acbe1

I've also tested this on macOS.

It was running smoothly. I tried shutting it down using the CLI interface, and it worked as expected.

However, I encountered an issue when performing an unclean shutdown using CTRL+C.
The process hung, and I had to `pkill bitcoind` to terminate it.

I wasn’t running with -debug option, so I couldn’t figure out exactly what went wrong. I tried to reproduce the issue but couldn’t It worked smoothly in
...

💬 ismaelsadeeq commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091450477)
In "multiprocess: Add bitcoin wrapper executable" https://github.com/bitcoin/bitcoin/commit/ceabc8cd43fa17be795974a85083aa8e865c654d

help seems redundant here we can get same behavior with -h, maybe just add -a?

💬 ismaelsadeeq commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091062670)
In "multiprocess: Add bitcoin wrapper executable" https://github.com/bitcoin/bitcoin/commit/ceabc8cd43fa17be795974a85083aa8e865c654d

Should the list of executable mentioned that bitcoin-node and bitcoin-gui since are hidden unlike bitcoind and should be accessed via bitcoin process?

💬 ismaelsadeeq commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091131888)
Shouldn't we throw when that happen?

💬 ismaelsadeeq commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091060261)
In "multiprocess: Add bitcoin wrapper executable" https://github.com/bitcoin/bitcoin/commit/ceabc8cd43fa17be795974a85083aa8e865c654d
Copy right bump to 2025-present.

💬 ismaelsadeeq commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091447427)
In "multiprocess: Add bitcoin wrapper executable" https://github.com/bitcoin/bitcoin/commit/ceabc8cd43fa17be795974a85083aa8e865c654d

I think we should prevent mixing up options with commands using this suggestion or something like it.

If you pass -v you should not pass any other command same with help

```suggestion
if (cmd.show_version) {
if (argc > 2) {
tfm::format(std::cout, "Error: too many arguments");
return EXIT_FAILURE;

...

💬 ismaelsadeeq commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091152243)
In "multiprocess: Add bitcoin wrapper executable" ceabc8cd43fa17be795974a85083aa8e865c654d

Are these actually commands?

Maybe *target* would be a better name. The commands are what's being forwarded to the executable, right?

It gets confusing in the error message, like when you pass `cli`:

```
Error: Unrecognized command: 'cli'
```

Maybe it would be clearer as:

```
Error: Unrecognized target: 'cli'
```


It gets more ambigous when you omit the command in the RPC

the
...

👋 fanquake's pull request is ready for review: "[28.x] 28.2rc1"
(https://github.com/bitcoin/bitcoin/pull/32480)

📝 pinheadmz opened a pull request: "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response"
(https://github.com/bitcoin/bitcoin/pull/32517)
This change is motivated by external RBF clients like https://github.com/CardCoins/additive-rbf-batcher/. It saves the user a redundant re-looping of tx outputs, calling `getaddressinfo` on each one, looking for the change output in order to adjust the fee.

The field `"ischange"` only appears when `gettransaction` is called on a wallet, and is either `true` or not present at all. I chose not to include `ischange: false` because it is confusing to see that on *received* transactions.

Exampl
...

🤔 jonatack reviewed a pull request: "doc: Improve `dependencies.md`"
(https://github.com/bitcoin/bitcoin/pull/31895#pullrequestreview-2844215334)
ACK e62423d6f1514b022155edb5bc930cecc4236731