💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-2966042888)
Rebased df0eb65bd98f300618cccd667720f1ccc6145865 -> 969799291d34da706b43209ba1209038ec349424 ([spendblock_5](https://github.com/TheCharlatan/bitcoin/tree/spendblock_5) -> [spendblock_6](https://github.com/TheCharlatan/bitcoin/tree/spendblock_6), [compare](https://github.com/TheCharlatan/bitcoin/compare/spendblock_5..spendblock_6))

* Fixed conflict with #32673
* Fixed conflict with #32421

💬 PeterWrighten commented on pull request "wallet: Allow read-only database access for info and dump commands":
(https://github.com/bitcoin/bitcoin/pull/32685#discussion_r2142290203)
You mean we should throw an exception before get a return from `sqlite3_exec` ?

💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2142292007)


I don't think users should configure this. It should be determined by the forecaster, based on the potential increment of the top block in the mempool.

Previously, I researched how long it takes for a transaction to finish propagating through the network. @sr-gi responded with an estimate of approximately 7 seconds:
https://bitcoin.stackexchange.com/questions/125776/how-long-does-it-take-for-a-transaction-to-propagate-through-the-network.

I thought I had updated this accordingly, but
...

💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2142292170)
Done

💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#discussion_r2142297543)
Not sure whether that a good idea for ASAP-mempool Forecaster.
But can good if you want something like https://github.com/FelixWeis/WhatTheFee--legacy
So far I've not experimented with whatTheFee yet to determine its correctness.

💬 ismaelsadeeq commented on pull request "Fees: add Fee rate Forecaster Manager":
(https://github.com/bitcoin/bitcoin/pull/31664#issuecomment-2966076465)
> This looks pretty complete now, nice work.

Thanks 👍🏾

> Slightly sad it's not wired up to the rpc for so I could do some live testing, but the unit tests confirm the expected behaviour.

Please do it is implemented in the complete PR https://github.com/bitcoin/bitcoin/pull/30157, the RPC changes is next to review If we manage to get this in.

> Left a few comments about possibly allowing some of the mempool-based fee estimator constants to be altered at startup, as I suspect those
...

💬 josibake commented on pull request "depends: fix cmake compatibility error for freetype":
(https://github.com/bitcoin/bitcoin/pull/32693#issuecomment-2966079989)
Updated to add a patch per @hebasto 's [suggestion](https://github.com/bitcoin/bitcoin/pull/32693#discussion_r2140453692).

💬 fanquake commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2142312348)
```suggestion
- `NO_IPC`: Don't build Cap’n Proto and libmultiprocess packages
```

I don't think we need to list binaries, it's another list to sync, and we don't do it for other options (i.e bitcoin-wallet) and `NO_WALLET`.

💬 fanquake commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#discussion_r2142312539)
Unrelated CI formatting changes, in a `cmake:` commit?

💬 Sjors commented on pull request "deps: Bump lief to 0.16.6":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2966089437)
Unfortunately this introduced a dependency on `/gnu/store/hmd1s3jd77jxnc084pdb4i6i1qrxacx9-go-1.17.13.drv` which fails to build on guix for those (some??) who don't use substitutes (not just aarch64) due to an invalid certificate in its test: https://lists.gnu.org/archive/html/bug-guix/2025-01/msg00288.html

💬 pinheadmz commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2966094663)
> Completely misses the fact that this PR lowers the barrier to entry for such garbage infinitely, but again, that doesn't seem to matter to anyone here along with any of the other adverse issues causes by this PR.

gmaxwell made a great point earlier in this thread, which has been repeated in [Antoine's post](https://delvingbitcoin.org/t/addressing-community-concerns-and-objections-regarding-my-recent-proposal-to-relax-bitcoin-cores-standardness-limits-on-op-return-outputs/1697) that the entr
...

💬 josibake commented on pull request "depends: fix cmake compatibility error for freetype":
(https://github.com/bitcoin/bitcoin/pull/32693#discussion_r2142329694)
Thanks for the link! I read the chapter on policy and it was really helpful.

For posterity (and to reason out my own understanding), I don't think the recommendation against `CMAKE_POLICY_VERSION_MINIMUM` (as opposed to using a patch) applies here. I took that paragraph as saying: "here is an easy way to override a projects minimum policy version; only use this as a temporary measure until the project itself updates to a newer cmake minimum."

This advice, IMO, also applies to patching the
...

💬 rkrux commented on pull request "Musig2 tests":
(https://github.com/bitcoin/bitcoin/pull/32724#issuecomment-2966114656)
I feel this PR should be in draft until #31244 is merged as all the commits here are from that PR except the last one.

👍 rkrux approved a pull request: "rpc: Use type-safe exception to pass RPC help"
(https://github.com/bitcoin/bitcoin/pull/32660#pullrequestreview-2920584042)
re-ACK fa946520d229ae45b30519bccc9eaa2c47b4a093

```
git range-diff fa0cf42...fa94652
```

Thanks for accepting the naming suggestion.

💬 janb84 commented on pull request "docs: add guidance on initialism capitalisation in PascalCase identifiers.":
(https://github.com/bitcoin/bitcoin/pull/32720#discussion_r2142415579)
```suggestion
- With `PascalCase`, Acronyms must be treated as words. To minimize code churn, legacy identifiers that do not comply with this rule will remain unchanged and will only be renamed when the surrounding code is modified. Hence:
```
NIT: Do not tell what NOT to do but explain what to do. This suggestion explains (imho) what the intention is, treating acronyms as words, and therefor the PascalCase can easily be applied.

💬 sipa commented on pull request "docs: add guidance on initialism capitalisation in PascalCase identifiers.":
(https://github.com/bitcoin/bitcoin/pull/32720#issuecomment-2966248213)
-0, I don't see why this can't be left to the author, given how rare it is.

💬 l0rinc commented on pull request "blocks: force hash validations on disk read":
(https://github.com/bitcoin/bitcoin/pull/32638#discussion_r2142452856)
Done, thanks

💬 rkrux commented on pull request "wallet: Remove ISMINE_WATCHONLY and watchonly from RPCs":
(https://github.com/bitcoin/bitcoin/pull/32618#discussion_r2142472335)
Does this argument `include_watchonly` not require the `DEPRECATED` flag now in the RPCResult description?

💬 fanquake commented on pull request "deps: Bump lief to 0.16.6":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2966291005)
Looks like this is getting pulled in by `python-scikit-build-core` -> `"/gnu/store/8a28mn0n9bd0nwq5vd6d1lfhr1j27cbk-rust-ring-0.17.8.tar.gz.drv"` -> `"/gnu/store/wbfmclxgnmlqbin55ipb0x05i1yapiy5-go-1.21.13.drv"` -> `"/gnu/store/wb36d09msn72aamvj8hj4y55551rqilz-go-1.17.13.drv"` -> `"/gnu/store/mjvb787fwd5bb6m2z02hhy30n5jzd9gw-go-1.4-bootstrap-20171003.drv"`

💬 l0rinc commented on pull request "blocks: force hash validations on disk read":
(https://github.com/bitcoin/bitcoin/pull/32638#discussion_r2142496127)
I'm not sure I fully understand the comment here. Are you saying that it's obvious that this is always true? if so, isn't that kinda' the role of an assert though :)?

If you think it's ridiculously trivial (though you had to look inside the methods to understand the relation, this assertion was meant to avoid doing that, since I'm replacing one with the other in the code, this should help reviewers understand why that's safe).

So here it's meant to document why subsequent code now relies o
...

💬 m3dwards commented on pull request "test: enabling wallet migration functional test on windows":
(https://github.com/bitcoin/bitcoin/pull/32219#discussion_r2142521186)
It was only a problem on Windows. Made a note of that.