💬 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

💬 maflcko commented on pull request "test: enabling wallet migration functional test on windows":
(https://github.com/bitcoin/bitcoin/pull/32219#issuecomment-3009004461)
only small style changes

re-ACK 941b8f54c0d35d3243bb6083f3b52681d1b9a555 🍪

<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:
...

💬 maflcko commented on issue "`dumptxoutset` rollback feature does not take forks into account":
(https://github.com/bitcoin/bitcoin/issues/32817#issuecomment-3009013387)
> you'll just be unable to dump at that height.

Could do a manual `invalidateblock` of the low-work forks to work around the error, but yea, could be annoying if the forks are more than 1, or longer than 1 block.

💬 darosior commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2169429571)
Yeah.

💬 maflcko commented on pull request "util: explicitly close all AutoFiles that have been written":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r2169431059)
If a `Commit` is not possible without closing the file, then `Commit` should close the file. Requiring all call sites of `Commit` to manually close the file seems verbose and less safe to me.

💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3009050890)
> Whiteboarded in person. Hopefully this explanation is clear and explains it in a more permanent location. If this ends up being correct we can adapt the benchmark to be more realistic.

💬 musaHaruna commented on pull request "rpc: Distinguish between vsize and sigop adjusted mempool vsize":
(https://github.com/bitcoin/bitcoin/pull/32800#discussion_r2169450138)
Done! Thanks for the suggestion it was realy insightful.