💬 maflcko commented on pull request "refactor: Remove unused HaveKey and HaveWatchOnly":
(https://github.com/bitcoin/bitcoin/pull/32476#issuecomment-2875412918)
(follow-up to https://github.com/bitcoin/bitcoin/pull/32438)

💬 maflcko commented on pull request "bench: replace benchmark block with more representative one (413567 → 784588)":
(https://github.com/bitcoin/bitcoin/pull/32457#discussion_r2086208814)
> Yes, this one definitely, but in the other cases I'm worried about introducing a strong bias.

Again, it would be good to list the benchmark that needs this. Also, serialization itself shouldn't care if the data is synthetic (random) or if it exactly matches a real past block. If you worry about a bias, it should actually be easier to provide synthetic data, than to try to find a fitting past block. In any case, there will always be a bias, even if the data is fully synthetic, as the real ch
...

🤔 maflcko reviewed a pull request: "contrib: add xor-blocks tool to obfuscate blocks directory"
(https://github.com/bitcoin/bitcoin/pull/32451#pullrequestreview-2835753705)
I wonder if `-reindex` could be taught to do this instead. This may be simpler for users to set. Also, it will be atomic, safe and interruptible. However, it will eat more CPU and thus take a longer time (I'd say this is probably fine).

💬 maflcko commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2086226509)
too bad simply getting a few bytes of os rand is still experimental and only in rust nightly. Maybe using the `getrandom` crate directly can cut down the dependencies?

💬 fanquake commented on pull request "wallet: Use `util::Error` throughout `AddWalletDescriptor` instead of returning `nullptr` for some errors":
(https://github.com/bitcoin/bitcoin/pull/32475#issuecomment-2875540449)
```bash
/Users/runner/work/bitcoin/bitcoin/src/wallet/test/wallet_tests.cpp:69:10: error: unused variable 'spk_manager' [-Werror,-Wunused-variable]
auto spk_manager = *Assert(wallet.AddWalletDescriptor(w_desc, provider, "", false));
^
1 error generated.
```

💬 Sjors commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2875574267)
Rebased and moved `SIGNET_HEADER` into `blocktools.py`.

💬 Sjors commented on pull request "signet: omit commitment for some trivial challenges":
(https://github.com/bitcoin/bitcoin/pull/29032#issuecomment-2875626028)
Encoded it as bytes: `SIGNET_HEADER = b"\xec\xc7\xda\xa2"` and using the constant in `contrib/signet/miner`

💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2875639340)
@ryanofsky I added your libmultiprocess patch (and rebased).

👍 vasild approved a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2835944933)
ACK 4e1aae19512df82af584a064640c2143c5c5fa4f

Good to see the manual quoting go away and `bitcoin daemon -daemon` be renamed to `bitcoin node -daemon`.

💬 Sjors commented on pull request "refactor: Remove unused HaveKey and HaveWatchOnly":
(https://github.com/bitcoin/bitcoin/pull/32476#issuecomment-2875734524)
utACK fabdc5ad06bce8cd3b0e525071d10f616a30bb04

💬 ajtowns commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2086377216)
This is only a limit on the arbitrary data you're encoding in OP_RETURN scriptPubKeys, not a limit on the overall transaction size.

If you want to encode additional data, there are three approaches: extend an existing PUSH with extra data, add an additional PUSH, or add an additional output. These have an overhead of roughly 0, 1 and 2 with respect to this limit, and an overhead of 0, 1 and 10 vbytes wrt transaction weight with a corresponding impact on fees needed. If you did indeed have 83
...

📝 maflcko opened a pull request: "lint: Check for missing trailing newline"
(https://github.com/bitcoin/bitcoin/pull/32477)
A missing trailing newline is harmless, but a bit problematic:

* `git` shows a warning by default
* After another line is appended, the diff will be verbose and `git blame` will be wrong for the "untouched" line.

Fix the problems by just requiring what is already the default, see also https://github.com/bitcoin/bitcoin/blob/663a9cabf811e2fc53102bc6da00d09fc99d1d81/.editorconfig#L9 and https://github.com/bitcoin/bitcoin/blob/663a9cabf811e2fc53102bc6da00d09fc99d1d81/test/lint/test_runner/sr
...

💬 vasild commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2086457036)
Yes, that is the same in `master` so no change in behavior by this PR. In master it is using a magic number `9050` whereas here it uses the constant `DEFAULT_TOR_SOCKS_PORT`.

`-proxy=127.0.0.1=cjdns` would be interpreted as `-proxy=127.0.0.1:9050=cjdns` which might be the correct config, or if not then the user could use `-proxy=127.0.0.1:1234=cjdns` to specify another port.

Personally, I would favor a more dumb implementation that does not do such automations and guessings on behalf of th
...

💬 vasild commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2086457264)
Done.

💬 vasild commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#discussion_r2086458031)
Done.

💬 vasild commented on pull request "config: allow setting -proxy per network":
(https://github.com/bitcoin/bitcoin/pull/32425#issuecomment-2875900750)
`2059eaa692...e98c51fcce`: address suggestions

💬 hebasto commented on pull request "cmake: Add application manifests when cross-compiling for Windows":
(https://github.com/bitcoin/bitcoin/pull/32396#issuecomment-2875926603)
Rebased due to a conflict with the merged bitcoin/bitcoin#32458.

💬 hebasto commented on pull request "Update `secp256k1` subtree to latest master":
(https://github.com/bitcoin/bitcoin/pull/32028#issuecomment-2875949757)
Rebased and updated.

💬 maflcko commented on pull request "lint: Check for missing trailing newline":
(https://github.com/bitcoin/bitcoin/pull/32477#issuecomment-2875950885)
This should also catch issues where existing newlines are removed (and make the diff even more verbose): https://github.com/bitcoin/bitcoin/pull/32404#discussion_r2072592239 (cc @l0rinc)

⚠️ 1440000bytes opened an issue: "Allow multiple data outputs in `createrawtransaction` and `createpsbt`"
(https://github.com/bitcoin/bitcoin/issues/32478)
### Motivation

> Each key may only appear once, i.e. there can only be one 'data' output, and no address may be duplicated.

https://bitcoincore.org/en/doc/29.0.0/rpc/rawtransactions/createrawtransaction/

If and when https://github.com/bitcoin/bitcoin/pull/32406 gets merged, it will be helpful to use multiple data outputs while creating a transaction.

Related comment: https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2875133307

### Possible solution

Remove the check for duplicate d
...