💬 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

💬 Sjors commented on issue "Mining interface tracking issue":
(https://github.com/bitcoin/bitcoin/issues/33777#issuecomment-3502947184)
@ismaelsadeeq I'm think about deprecating `getBlock()` in favour of a new `getTransactions()`. That would allow us to avoid shoehorning block templates in `CBlock`, which e.g. means we no longer need a dummy coinbase transaction.

💬 kevkevinpal commented on pull request "test: Ignore error message give from python because of PYTHON_GIL":
(https://github.com/bitcoin/bitcoin/pull/33795#issuecomment-3502952335)
Would it make sense to add a TODO to remove the warning suppression once GIL is supported in the capnp library?

💬 ismaelsadeeq commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2503927416)
> Is a1a1bfa necessary?
Won't old versions be able to read the file even if estimations are sub 1sat/vb?

Yes it is necessary, old clients won't be able to read the file saved with new client, and new client won't also be able to read the file saved by old client see https://github.com/bitcoin/bitcoin/pull/33199#issuecomment-3242381132 comment with a linked PR that allows for that approach.

> just curiosity, how is this number chosen? Related to release number?

Yes but not necessary can
...

💬 ismaelsadeeq commented on pull request "fees: enable `CBlockPolicyEstimator` return sub 1 sat/vb fee rate estimates":
(https://github.com/bitcoin/bitcoin/pull/33199#discussion_r2503896071)
What you mentioned is exactly what the comment is now.