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

💬 achow101 commented on pull request "rpc: use CScheduler for HTTPRPCTimer":
(https://github.com/bitcoin/bitcoin/pull/32796#issuecomment-3029409482)
> which is that timers are mapped by name and can be removed by name if, for example, the user calls the RPC again with a different timeout value.

This behavior would already be changing in this PR. Deleting the `HTTPRPCTimer` does not de-schedule the task and it will still execute later.

> Current behavior I believe is to only honor the most recent walletpassphrase argument.

This behavior would still be maintained even if the task runs multiple times because there is an explicit check
...

💬 l0rinc commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2181030545)
I doubt anyone will review it if we push it separately - but I think we could make these tests less awkward. Not a blocker from my side, just a preference

💬 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_r2181030565)
Changed, I think tracking bytes is better.

💬 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_r2181030928)
Did not implement as this gave me a compilation error?

💬 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_r2181031427)
Agree phrasing was confusing, have changed.

💬 l0rinc commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2181033230)
Can you show us the raw measurements?