💬 hodlinator commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2180836207)
This comment for the perturbed files is lost, unlike the one for the deleted files above.

💬 hodlinator commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2180033937)
nit: Could add quotes and make output as verbose as variables:
```suggestion
self.log.info(f"Expecting error fragment: {err_fragment!r}")
```

💬 hodlinator commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2180861847)
nit: Could use `@dataclass` for slightly less stringly typed code, and `tuple` for immutability:
<details><summary>Expand for diff</summary>

```diff
diff --git a/test/functional/feature_init.py b/test/functional/feature_init.py
index 6ee075f906..1913002962 100755
--- a/test/functional/feature_init.py
+++ b/test/functional/feature_init.py
@@ -9,6 +9,7 @@ import platform
import shutil
import signal
import subprocess
+from dataclasses import dataclass, field

from test_framework
...

💬 achow101 commented on pull request "wallet, test: best block locator matches scan state follow-ups":
(https://github.com/bitcoin/bitcoin/pull/32580#issuecomment-3029243288)
ACK 1b5c545e82fe3cf5027f16b43e2306aeb8d4ef9b

🚀 achow101 merged a pull request: "test: Fix wait_for_getheaders() call in test_outbound_eviction_blocks_relay_only()"
(https://github.com/bitcoin/bitcoin/pull/32823)

💬 achow101 commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-3029320429)
ACK 0ab0e1be5461084b8e945f14406868f044c90718

💬 glozow commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2180971720)
Yes, those shouldn't be included!

💬 tnndbtc commented on issue "bitcoind shouldn't fail to progress with synchronization: endless [leveldb] Generated table ... logs":
(https://github.com/bitcoin/bitcoin/issues/31882#issuecomment-3029345008)
@GregTonoski The tests I have done is to prove that
1) making debug=1 have turned on too much logs and it clearly slows down the IBD process even in a fast device (SSD).
2) levelDB can be tuned to have less disk operations while each time bitcoind can flush more data from memory to disk, if disk I/O is not sufficiently leveraged.

I agree that for a fair performed hardware, bitcoind shouldn't take too long to sync. So, I googled average write speed of HDD, the first entry (generated by their A
...

💬 glozow commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2180989848)
True, there isn't actually anything to group in this test. Just copied from the other tests.

💬 glozow commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#issuecomment-3029351762)
reACK 1632fc104be via range-diff

🤔 glozow reviewed a pull request: "cluster mempool: add TxGraph reorg functionality"
(https://github.com/bitcoin/bitcoin/pull/31553#pullrequestreview-2980663908)
reACK 1632fc104be via range-diff

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2181013847)
> Lastly, do we really need the RemovePrefix here, given that we know the exact file structiure?

Yup, the file name may differ across platforms and the test will fail on some platforms otherwise. Necessary because it's done here: https://github.com/bitcoin/bitcoin/blob/68ca13e1f96a9dae997558a1d7a873b10056091b/src/logging.cpp#L377

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2181018846)
I have kept the comment -- I made a note in the commit message about the change.

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2181022825)
Now uses a parameter, but I have kept the comment. I think it is clear as is? I think a follow-up could address this?

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2181023549)
I haven't implemented this, could this be left for a follow-up?

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2181024709)
I've modified the comment so it doesn't repeat the phrase "rate limit" twice.

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2181026104)
Modified the commit message and have checked that `MemUsage` returns an accurate accounting of the memory used. Thanks for this comment, it encouraged me to verify this.

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2181027949)
I have not run benchmarks, though this `insert` is pre-existing so will resolve. It would be nicer if there was a better way to format the string other than the insert-at-beginning logic in `FormatLogStrInPlace`.

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2181028316)
Modified the commit message 👍

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2181029394)
I agree this is confusing and threw me. The different base part is pre-existing so will resolve. I think another PR could eliminate the different bases unless there's a compelling "historical" reason to keep both?

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2181029843)
Latest change address this.