🚀 fanquake merged a pull request: "cmake: Drop no longer necessary "cmakeMinimumRequired" object"
(https://github.com/bitcoin/bitcoin/pull/32954)

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3068782737)
Thank you for the review @stringintech!

Rebased 690a5dac223ed18a65c9d9e6c535466cc3ad4511 -> 52bab146a5045899ea6800305fa6d9b4efdcc6bd ([kernelApi_42](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_42) -> [kernelApi_43](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_43), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_42..kernelApi_43))

Updated 52bab146a5045899ea6800305fa6d9b4efdcc6bd -> 267a7b3f321304f75e8c47e380da49ba9c64bc84 ([kernelApi_43](https://gith
...

💬 maflcko commented on pull request "test: headers sync timeout":
(https://github.com/bitcoin/bitcoin/pull/32677#issuecomment-3068819299)
re-ACK 11546183c70def6c0aa539642fd1c9ada3d46840 🗻

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: re-ACK 11546183c70def6c0aa5
...

💬 maflcko commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2204495759)

exists in disk -> exists on disk [“in disk” is nonstandard; use “on disk”]


An alternative wording could be "if ... was persisted, clear it"

💬 hebasto commented on pull request "ci: Avoid cd into build dir":
(https://github.com/bitcoin/bitcoin/pull/32880#issuecomment-3068845648)
> > ACK [fa0eca8](https://github.com/bitcoin/bitcoin/commit/fa0eca82ec1222ec1c68835ce7acdf9c8c4740ad), I have reviewed the code and it looks OK.
>
> I think you reviewed a commit that still had the pre-existing $GOAL bug?

I did review the correct branch but accidentally copied the wrong hash from my Sublime Merge instance. Now fixed.

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3068922992)
Re https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3068613759

Thanks for writing all of that up and your detailed tour @purpleKarrot. I think you raise some excellent points on your blog, but I am not sure how I am to interpret your NACK here.

You mention that fundamental changes are required, but after reading some of your proposed changes in `/btck` I am not sure how materially different those are from what is proposed here. I think naming conventions is probably the easiest w
...

🚀 fanquake merged a pull request: "ci: Avoid cd into build dir"
(https://github.com/bitcoin/bitcoin/pull/32880)

💬 purpleKarrot commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3069008389)
> am not sure how I am to interpret your NACK here.

Just as "I think this PR should not be merged in its current form." I definitely do agree with the approach of adding a C API.

Regarding the other points, maybe we should have a private discussion.

💬 maflcko commented on pull request "[POC] ci: Skip compilation when running static code analysis":
(https://github.com/bitcoin/bitcoin/pull/32953#discussion_r2204607453)
It seems fine to wait a few months until the 26.04 tag is available and then just use that. Also, it seems fine to just require cmake 3.31 (or higher) if anyone wants to use the codegen target, but no strong opinion.

💬 fanquake commented on issue "Intermittent failure in rpc_invalidateblock.py assert_equal(self.nodes[0].getblockchaininfo()['headers'], 7) [ AssertionError: not(24 == 7)]":
(https://github.com/bitcoin/bitcoin/issues/32965#issuecomment-3069047625)
https://github.com/bitcoin/bitcoin/actions/runs/16263971992/job/45915380111#step:13:114

💬 Sjors commented on pull request "descriptor: don't underestimate the size of a Taproot spend (instead, overestimate it)":
(https://github.com/bitcoin/bitcoin/pull/32964#issuecomment-3069141228)
I don't think we should be overpaying by default, for two reasons:
1. it's expensive
2. it reveals the presence of script paths

An intermediate solution could be that we never sign script paths by default, i.e. make `keypath_only` the default after #32857. Only when the user opts in to script path spending do we apply this solution of using the worst case fee. Since at least that's better than the current "solution" of manually picking a higher fee rate.

💬 Sjors commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3069171378)
> Over that period the USD price of BTC has risen by roughly 2-3 orders of magnitude, making the economic cost of relaying small transactions disproportionately high.

The question is whether DoS protection back then was actually sufficient, or if the increase in fiat terms was the luck we needed. I don't know the answer to that.

Another difference between now and back then is the presence of Lightning and other systems that are sensitive to mempool shenanigans in general. Again I don't if
...

💬 Sjors commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2204657999)
In 89831dace9d21456eb4a019d86164304e22f458e _wallet/rpc: add create silent-payments wallet option_: while testing I found that it doesn't disable block filters.

🤔 fanquake requested changes to a pull request: "ci, iwyu: Treat warnings as errors for specific directories"
(https://github.com/bitcoin/bitcoin/pull/31308#pullrequestreview-3016058319)
This makes the CI dependant on being run on x86_64; the tidy job will no-longer pass when run on aarch64.

💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3069275601)
may be good to fix the aarch64 ci. Otherwise:

re-ACK bfbf7de389649f848dffe23be6727df715648b01 🏖

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wL
...

💬 fanquake commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3069279294)
> may be good to fix the aarch64 ci.

How would you fix it, given IWYU is giving different recommendations, based on the architecture?

💬 flack commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3069313602)
small nit: The PR description should rather point to this commit: 9e93640be6c49fa1505ba5c5df8c89210da5a6e4

💬 fanquake commented on pull request "depends: Force `CMAKE_EXPORT_NO_PACKAGE_REGISTRY=TRUE`":
(https://github.com/bitcoin/bitcoin/pull/32943#issuecomment-3069317099)
ACK 44f3bae300dcafbe53f9b07e6cc22a112833e579

> Yikes. Concept ACK.

Yea. It's an interesting example of how bumping/changing a CMake minimum version, can opt you into, or out of, unexpected behaviour (I imagine this is why this stopped being the default).

✅ fanquake closed an issue: "cmake: searching across directories for config files"
(https://github.com/bitcoin/bitcoin/issues/32938)

🚀 fanquake merged a pull request: "depends: Force `CMAKE_EXPORT_NO_PACKAGE_REGISTRY=TRUE`"
(https://github.com/bitcoin/bitcoin/pull/32943)