💬 polespinasa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2169312647)
Actually I think nSize is not needed at all and the check can be deleted. If `EvaluateFeeUp` is taking an uint32 there's no need to convert `num_bytes` to other types!!

💬 polespinasa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2169317441)
You mean the commented Assume can be deleted?

💬 sipa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2169326321)
No, the `if` condition is wrong now. The result can overflow.

💬 fanquake commented on pull request "Add read-only mode to sqlite db and use in `bitcoin-wallet`":
(https://github.com/bitcoin/bitcoin/pull/32818#issuecomment-3008874065)
https://github.com/bitcoin/bitcoin/actions/runs/15904154552/job/44856855999?pr=32818#step:12:437:
```bash
test 2025-06-26T14:50:15.271166Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "D:\a\bitcoin\bitcoin\test\functional\test_framework\test_framework.py", line 190, in main
self.run_test()
~~~~~~~~~~~~~^^
...

💬 HowHsu commented on pull request "checkqueue: implement a new scriptcheck worker pool with atomic variables":
(https://github.com/bitcoin/bitcoin/pull/32791#issuecomment-3008895117)
Hi folks, any hints on how to see node logs of failed CI tests: [https://github.com/bitcoin/bitcoin/pull/32791/checks?check_run_id=44855035637#:~:text=%5B11%3A09%3A03.884%5D%20%EF%BF%BD%5B0%3B36m%20test%20%202025%2D06%2D26T15%3A09%3A03.312746Z%20TestFramework%20(ERROR)%3A%20Hint%3A%20Call%20/ci_container_base/test/functional/combine_logs.py%20%27/ci_container_base/ci/scratch/test_runner/test_runner_%E2%82%BF_%F0%9F%8F%83_20250626_150635/wallet_fundrawtransaction_252%27%20to%20consolidate%20all%2
...

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#issuecomment-3008910090)
Coins are already accessed multiple times in checking an unconfirmed transaction (which should cache them anyways), doing once more shouldn't introduce any noticeable overhead and i like the upside of facilitating review of a future consensus-touching PR. But if many feel strongly i'm happy to revert the previous approach, and i'm sure Greg would be fine too as he Concept ACK'd the previous approach. Let me know!

💬 maflcko commented on pull request "[DO NOT MERGE] Erlay: bandwidth-efficient transaction relay protocol (Full implementation)":
(https://github.com/bitcoin/bitcoin/pull/30277#issuecomment-3008913285)
> I'm surprised our existing fuzz tests do not catch this (`process_messages` might but I haven't tried), but it looks like we actually never call `Minisketch::Deserialize` with bytes straight from the fuzzer but rather only with result from `Minisketch::Serialize` (maybe to avoid the assertion? not sure):

I tried this by starting 8 fuzz processes 5 days ago. 7 still run and one of them crashed after two days. I minimized the input:

```
$ git log -1
commit bd242fd6e3f42e03a8c8abf23f9e92
...

💬 polespinasa commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2169364191)
Oh you're right! I think adding an `Assume(at_size <= INT32_MAX)` should be enough?

👍 hebasto approved a pull request: "[29.x] More backports"
(https://github.com/bitcoin/bitcoin/pull/32810#pullrequestreview-2962594362)
ACK fe8034b09c53f49d02b54f4e55cfe11bbd113fed. I've backported all mentioned PRs locally and got zero diff with this branch.

💬 maflcko commented on pull request "checkqueue: implement a new scriptcheck worker pool with atomic variables":
(https://github.com/bitcoin/bitcoin/pull/32791#issuecomment-3008935991)
> Hi folks, any hints on how to see node logs of failed CI tests, It's not easy to create all the environements including OS and libs and to run it locally:

You can follow the " View more details on Cirrus CI " link: https://cirrus-ci.com/task/6753181050339328

It says system error, so you are likely launching enough threads to run into a system limit.

💬 maflcko commented on pull request "checkqueue: implement a new scriptcheck worker pool with atomic variables":
(https://github.com/bitcoin/bitcoin/pull/32791#issuecomment-3008939307)
For the CI docs, see https://github.com/bitcoin/bitcoin/tree/master/ci#ci-scripts, but you'll likely have to run that on a 64 core system to recreate the system error

💬 hebasto commented on pull request "build: Find Boost in config mode":
(https://github.com/bitcoin/bitcoin/pull/32667#issuecomment-3008949412)
> Needs a rebase for #32814.

Thanks! Rebased.

💬 hebasto commented on pull request "build: Find Boost in config mode":
(https://github.com/bitcoin/bitcoin/pull/32667#issuecomment-3008959403)
Added `EXACT` keyword to `find_package(boost_included_unit_test_framework ...)` command when using vcpkg (on Windows).

💬 maflcko commented on pull request "correct variable name in p2p_monitor.py":
(https://github.com/bitcoin/bitcoin/pull/32816#issuecomment-3008967559)
lgtm ACK 6bb38bf37fd81a9ce2b66a3f078375dce9274754

💬 instagibbs commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2169392812)
TIL. We could use this one in the other standardness check, right?

💬 fjahr commented on issue "`dumptxoutset` rollback feature does not take forks into account":
(https://github.com/bitcoin/bitcoin/issues/32817#issuecomment-3008973146)
Sigh, yeah, that's probably right. When taken to an extreme (aka Testnet4) that's a really annoying problem as I have experienced in #31117. Since I have to deal with this anyway so I will address this soon.

💬 maflcko commented on issue "`dumptxoutset` rollback feature does not take forks into account":
(https://github.com/bitcoin/bitcoin/issues/32817#issuecomment-3008985262)
Isn't this addressed by the `throw` here?

https://github.com/bitcoin/bitcoin/blob/67ea4b9994e668dcea5e5d0f62f886d92e3737dc/src/rpc/blockchain.cpp#L3111


I think there is still a race when calling `submitblock`, but this doesn't seem related to forks per se.

💬 achow101 commented on pull request "Add read-only mode to sqlite db and use in `bitcoin-wallet`":
(https://github.com/bitcoin/bitcoin/pull/32818#issuecomment-3008989919)
How does this compare with #32685?

💬 sipa commented on issue "`dumptxoutset` rollback feature does not take forks into account":
(https://github.com/bitcoin/bitcoin/issues/32817#issuecomment-3008997075)
@maflcko Right! So indeed, it won't (ignoring race conditions) dump the wrong thing - but if you have a fork at a certain height, you'll just be unable to dump at that height.

💬 instagibbs commented on pull request "node: cap `-maxmempool` and `-dbcache` values for 32-bit":
(https://github.com/bitcoin/bitcoin/pull/32530#issuecomment-3009000500)
utACK 9f8e7b0b3b787b873045a4a8194e77d0b0a2b3b6