💬 maflcko commented on pull request "ci: disable cirrus cache in 32bit arm job":
(https://github.com/bitcoin/bitcoin/pull/33302#discussion_r2322672396)
hmm, still wrong https://github.com/bitcoin/bitcoin/actions/runs/17469260337/job/49613109539?pr=33302#step:5:7 ?

💬 fjahr commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-3254450772)
Dear reviewers, sorry for another slight change in approach here but I think this should be the last one and I think it makes reasoning about the review easier or at least not harder. Given the discussion about remaining (edge case) potential for overflows I have decided to switch the values we store to `arith_uint256`. This feels cleaner to me than dealing with overflows with additional code and a shared field etc. The nice side-effect of this is that this makes it feasible to keep the total va
...

💬 instagibbs commented on pull request "net: check for empty header before calling FillBlock":
(https://github.com/bitcoin/bitcoin/pull/33296#issuecomment-3254453984)
concept ACK, as per offline conversations. It would be really nice to make the state machine better, for now we can at least avoid the `Assume` in debug builds when a peer does this.

We could also disconnect the peer for this behavior, I think, but not a priority

💬 TheCharlatan commented on pull request "RFC: Riscv bare metal CI job":
(https://github.com/bitcoin/bitcoin/pull/31425#issuecomment-3254442149)
Rebased ec7c86c732c995d2c38e2a3f93ad55a451a8cf81 -> a577bbe5df766a4e0e5a36fc997b7880eec45c0e ([bare_metal_support_2](https://github.com/TheCharlatan/bitcoin/tree/bare_metal_support_2) -> [bare_metal_support_3](https://github.com/TheCharlatan/bitcoin/tree/bare_metal_support_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/bare_metal_support_2..bare_metal_support_3))

* Fixed conflict with #32989

🤔 maflcko reviewed a pull request: "ci: disable cirrus cache in 32bit arm job"
(https://github.com/bitcoin/bitcoin/pull/33302#pullrequestreview-3186038074)
also, looks like a cache outage in another task?

💬 instagibbs commented on issue "policy: allow RBF descendant carveout whenever conflicts exist, not just when number of conflicts == 1":
(https://github.com/bitcoin/bitcoin/issues/16819#issuecomment-3254477864)
I think we can just close this one; we're not going to improve carveouts at this point

✅ fanquake closed an issue: "policy: allow RBF descendant carveout whenever conflicts exist, not just when number of conflicts == 1"
(https://github.com/bitcoin/bitcoin/issues/16819)

📝 fanquake opened a pull request: "doc: fix `LIBRARY_PATH` comment"
(https://github.com/bitcoin/bitcoin/pull/33308)
Now that we build capnp, qt isn't the only native package.

💬 katesalazar commented on pull request "gui: Avoid pathological QT text/markdown behavior...":
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3254546250)
> [...] I am not 100% sure what is causing this issue in QT's conversion of our HTML to markdown [...]

You implying that an upstream bug must be searched in Qt?

💬 katesalazar commented on pull request "gui: Avoid pathological QT text/markdown behavior...":
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3254557610)
Thank you

Approach ACK

possible nit:
```
PlainCopyQTextEdit
^
```

💬 fjahr commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-3254558440)
Hm, not clear to me why clang-tidy CI complains here about lines I haven't touched in this PR. But that's what the last fixup commit I just pushed is for.

💬 katesalazar commented on pull request "gui: Avoid pathological QT text/markdown behavior...":
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3254577747)
nit: splash post in two paragraphs (none tiny) then the commit message is a one liner

💬 glozow commented on issue "policy: allow RBF descendant carveout whenever conflicts exist, not just when number of conflicts == 1":
(https://github.com/bitcoin/bitcoin/issues/16819#issuecomment-3254599880)
Yeah the new design stages the changes and then checks policy limits. Currently in #28676: https://github.com/bitcoin/bitcoin/blob/61f51546af33923c9e5dc9632ba5544aa853ae71/src/validation.cpp#L1359-L1363

So the carveout becomes unnecessary since we aren't estimating cluster size, but calculating it

💬 fjahr commented on pull request "index: Force database compaction in coinstatsindex":
(https://github.com/bitcoin/bitcoin/pull/33306#issuecomment-3254606614)
Good question and I am definitely not a LevelDB expert either. I think that at the size and usage pattern of the coinstatsindex we are not really facing any significant problems on a modern computer and system since the size of the data we are storing is very small relatively speaking for what LevelDB is capable of handling. But I think the impact is probably also non-zero and might increase over time as the blockchain grows. Very high-level rationale: there must be a reason LevelDB does these c
...

💬 fjahr commented on pull request "index: Force database compaction in coinstatsindex":
(https://github.com/bitcoin/bitcoin/pull/33306#issuecomment-3254623438)
FWIW, from the patterns I have observed so far it seems likely to me that the problem is only occurring during sync and afterwards the data of new blocks are actually compacted as it should, just those 55kb files are left over from the sync phase. So I am thinking about changing the approach to only compact once when the index is being set to `m_synced = true`, though this would make it a more complex change and I need to do some more testing if my anecdotal observation is correct.

💬 maflcko commented on pull request "ci: Checkout latest merged pulls":
(https://github.com/bitcoin/bitcoin/pull/33303#discussion_r2322828365)
thx, all done

🤔 janb84 reviewed a pull request: "ci: Checkout latest merged pulls"
(https://github.com/bitcoin/bitcoin/pull/33303#pullrequestreview-3186383766)
ACK fab00ebbc74fe55c41b2d531a964cdb083b02ab0

changes sinds last ACK:
- changes related to use YAML KEYS / templating.

Kinda in doubt about the use of YAML_KEY_01, in code it makes it clear that it is a YAML key but in the action execution it is visible as a `env` variable :

<details>

``` YAML
• Run actions/checkout@v5 with:
ref: refs/pull/33303/merge repository: bitcoin/bitcoin token: ***
ssh-strict: true ssh-user: git
persist-credentials: true clean: true
sparse-checkout-cone
...

💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#issuecomment-3254821483)
I had missed a few sources of memory usage in the numbers posted in the PR description before. I've redone the derivation, and explained it in more detail. Thanks @glozow for trying to make sense of my incorrect numbers.

👍 hebasto approved a pull request: "ci: Checkout latest merged pulls"
(https://github.com/bitcoin/bitcoin/pull/33303#pullrequestreview-3186457692)
ACK fab00ebbc74fe55c41b2d531a964cdb083b02ab0.

This time, it took GitHub ~20min to update the reference.

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3254910524)
@ryanofsky

You may be interested in these changes.