✅ fanquake closed a pull request: "depends: add zeromq patch to fix mingw CMake file install location"
(https://github.com/bitcoin/bitcoin/pull/33778)

💬 fanquake commented on pull request "depends: add zeromq patch to fix mingw CMake file install location":
(https://github.com/bitcoin/bitcoin/pull/33778#issuecomment-3502369453)
I wont bother changing this for now (also don't want to add to /share given that bundles content we are currently pruning). Maybe we can also convince upstream to rechange their approach.

✅ fanquake closed a pull request: "guix: Pointer Authentication and Branch Target Identification for aarch64 Linux"
(https://github.com/bitcoin/bitcoin/pull/24123)

💬 fanquake commented on pull request "guix: Pointer Authentication and Branch Target Identification for aarch64 Linux":
(https://github.com/bitcoin/bitcoin/pull/24123#issuecomment-3502370479)
Ideally #25573 will happen soon, which essentially includes this. So closing for now.

💬 l0rinc commented on issue "malloc: Failed to allocate segment from range group - out of space":
(https://github.com/bitcoin/bitcoin/issues/33806#issuecomment-3502430755)
I can reproduce it reliably on a MacBook M4 Pro with 64 GB memory.
The OOM happens at block ~500k already (at ~8.6GiB dbcache): https://gist.github.com/l0rinc/8ccb47e298d6176f8af28397eef7b4df

👍 rkrux approved a pull request: "log: show reindex progress in `ImportBlocks`"
(https://github.com/bitcoin/bitcoin/pull/33353#pullrequestreview-3433884021)
lgtm ACK d7de5b109f69293b375729363b4e5038f3fb6b15

I don't see a harm in logging the approximate completeness percentage, would be helpful instead for the user imo.

💬 maflcko commented on issue "malloc: Failed to allocate segment from range group - out of space":
(https://github.com/bitcoin/bitcoin/issues/33806#issuecomment-3502734842)
> I can reproduce it reliably on a MacBook M4 Pro with 64 GB memory.

Does it happen with da6f041e39efaf64a84b748556e321021ec1f756 reverted?

💬 maflcko commented on pull request "ci: Extend tidy job to cover kernel code":
(https://github.com/bitcoin/bitcoin/pull/33818#discussion_r2503744180)
i wonder if this can use the dev-mode preset, so that only a single list has to maintained for all targets

💬 frankomosh commented on pull request "test: add case where `TOTAL_TRIES` is exceeded yet solution remains":
(https://github.com/bitcoin/bitcoin/pull/33701#discussion_r2503752402)
> It's tricky that the api doesn't return a different result when the TOTAL_TRIES is exceeded.
>
> What if you add

`BOOST_CHECK(result_b->GetAlgoCompleted() == false);` right after the `BOOST_CHECK(!result_b);`.

I guess that might prove TOTAL_TRIES has been hit, instead of failing for any other reason. It obviously doesn’t give a typed error enum(like the rust version), but I guess it is the best that can be done with the current C++ API without a larger refactor. ?

💬 TheCharlatan commented on pull request "ci: Extend tidy job to cover kernel code":
(https://github.com/bitcoin/bitcoin/pull/33818#discussion_r2503782285)
sgtm

💬 hebasto commented on pull request "ci: Extend tidy job to cover kernel code":
(https://github.com/bitcoin/bitcoin/pull/33818#discussion_r2503836132)
Thanks! Adjusted.

📝 Sjors opened a pull request: "mining: add getCoinbase()"
(https://github.com/bitcoin/bitcoin/pull/33819)
Deprecate `getCoinbaseTx()` in favor of a new method that provides a struct with everything clients need to construct a coinbase. This is safer than providing a raw dummy coinbase that clients then have to manipulate.

Also deprecate `getCoinbaseCommitment()` and `getWitnessCommitmentIndex()`.

A new helper method `ExtractCoinbaseTemplate()` processes the dummy coinbase transaction generated by `BlockAssembler::CreateNewBlock` and produces a `CoinbaseTemplate`.

Expand the `interface_ipc.p
...

💬 Sjors commented on pull request "mining: add getCoinbase()":
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3502899702)
Working on a matching [sv2-tp](https://github.com/stratum-mining/sv2-tp) pull request.

🤔 rkrux reviewed a pull request: "rpc: combinerawtransaction now rejects unmergeable transactions"
(https://github.com/bitcoin/bitcoin/pull/31298#pullrequestreview-3434253430)
Concept ACK a26e9fd5b5908853d37b81915f5e12a5ad822f86

Thanks for adding the fix.

💬 rkrux commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2503845007)
```diff
--- a/test/functional/rpc_createmultisig.py
+++ b/test/functional/rpc_createmultisig.py
@@ -149,17 +149,16 @@ class RpcCreateMultiSigTest(BitcoinTestFramework):
assert_raises_rpc_error(-22, "TX decode failed", node2.combinerawtransaction, [rawtx2['hex'], rawtx3['hex'] + "00"])
assert_raises_rpc_error(-22, "Missing transactions", node2.combinerawtransaction, [])
assert_raises_rpc_error(-22, "Missing transactions", node2.combinerawtransaction, [rawtx2['hex']])
-
...

💬 rkrux commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2503860692)
Nit: Reading `Transaction 1` can be confusing for the user if they are not aware that 0-based indexing is being used.

💬 rkrux commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2503818549)
Nit: Can consider to be more verbose in the error message.

```diff
- throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Missing transactions");
+ throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Missing transactions. At least two transactions required.");
```

💬 rkrux commented on pull request "rpc: combinerawtransaction now rejects unmergeable transactions":
(https://github.com/bitcoin/bitcoin/pull/31298#discussion_r2503802001)
> This is done to align the method with PSBT. It could be removed without breaking the change. But it also is just clearing data which should take minimal time. I think it's worth keeping to align with PSBT format.

I don't fully understand how it is aligning with PSBT, can you please explain? `GetHash()` used both here later and in `combinePSBT` RPC doesn't take the witness into account, making the removal of witness unnecessary.

https://github.com/bitcoin/bitcoin/blob/93e79181da8c952f270c4fa1
...

💬 kevkevinpal commented on pull request "test: Ignore error message give from python because of PYTHON_GIL":
(https://github.com/bitcoin/bitcoin/pull/33795#discussion_r2503882272)
Thanks, I updated in this commit [d848b8d](https://github.com/bitcoin/bitcoin/pull/33795/commits/d848b8dc7ecdc63927fab7706383e238e809fcba)

💬 kevkevinpal commented on pull request "test: Ignore error message give from python because of PYTHON_GIL":
(https://github.com/bitcoin/bitcoin/pull/33795#discussion_r2503896536)
Thanks, I updated in this commit [d848b8d](https://github.com/bitcoin/bitcoin/pull/33795/commits/d848b8dc7ecdc63927fab7706383e238e809fcba)

I would mention that if any other test imports `capnp` in the future without adding the specific logic `interface_ipc.py`, then it will run into the same issue. But I think this makes sense for now