Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 maflcko commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2194330070)
nit in b5d0cc29ac378f360424472f637f6d6af74660a5: Can remove the trailing `\n` (it is added by logging).

Also, could use the Warning log level instead, because rate-limiting should never happen in practise and if it does, it seems like something that should be fixed by adjusting the logging at that location?
💬 maflcko commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2194249282)
nit: Probably wanted to say "... *to be able* to catch ..." or "to throw any exceptions." Otherwise this reads a bit as if `noexcept(false)` were to catch any thrown exceptions. Could also just remove the comment, but just a nit.
💬 maflcko commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2194621936)
nit in 6e47dad25c5a167cd171e5c11d51a7bee7c8d3c6: Could use the `HasReason` helper, but up to you.
💬 maflcko commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2194367325)
nit in https://github.com/bitcoin/bitcoin/commit/b5d0cc29ac378f360424472f637f6d6af74660a5: I don't think the `ll` is needed to silence the w-sign-compare warning. I'd go with just `0u`, or if you want to fully specify the type `uint64_t{0}`.
💬 Sjors commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-3052127988)
It's still marked as draft though.
💬 fanquake commented on issue "qt: translation related warnings":
(https://github.com/bitcoin/bitcoin/issues/32710#issuecomment-3052248717)
@hebasto If this is an actual issue can it be fixed up? Otherwise, it'd be good if there was a way to avoid this much build log spam. ie:
```bash
CMake Warning at depends/arm64-apple-darwin/native/lib/cmake/Qt6LinguistTools/Qt6LinguistToolsMacros.cmake:418 (message):
Failed to resolve language code for
/root/ci_scratch/src/qt/locale/bitcoin_am.ts. Please update
CFBundleLocalizations in your Info.plist manually.
Call Stack (most recent call first):
depends/arm64-apple-darwin/native/lib/c
...
💬 TheCharlatan commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2194840399)
The way I read it you are currently just checking if the field is set, not if it is set to true. At least providing "return_spending_tx":false in the cli still provides the full transaction.
💬 TheCharlatan commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2194916715)
Yes, I just wasn't sure if this is really useful. There are many other ways something could be misconfigured. We'd also be warning here now on what continues to be just normal working behaviour. Clients should be querying `getindexinfo` first anway in my opinion, but then again, I am also not sure what exactly the precedent is here. Maybe add a note to pass `"mempool_only":true` to silence the warning?
📝 maflcko opened a pull request: "fuzz: Add missing calls to `SetMockTime` for determinism"
(https://github.com/bitcoin/bitcoin/pull/32927)
(Tracking issue https://github.com/bitcoin/bitcoin/issues/29018)

During fuzzing, `AppInitParameterInteraction` may actually disable a previously set mocktime. This is confusing and can also cause non-determinism.

Fix this issue, by

* fixing the erroneous `-mocktime` parsing in `AppInitParameterInteraction`.
* adding the missing `SetMockTime` calls to the affected fuzz init functions.
* adding a `CheckGlobals` to the fuzz init, to prevent this issue in the future.

This can be tested
...
👋 ryanofsky's pull request is ready for review: "ipc: Handle unclean shutdowns better"
(https://github.com/bitcoin/bitcoin/pull/32345)
💬 ryanofsky commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-3052619311)
> It's still marked as draft though.

Thanks, fixed!
💬 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_r2195027425)
Added in 4c772cbd83e502a1339e8993d192ea6416ecd45c
💬 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_r2195027732)
Changed in 4c772cbd83e502a1339e8993d192ea6416ecd45c
💬 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#issuecomment-3052667445)
4c772cbd83e502a1339e8993d192ea6416ecd45c changes the release notes for clarity.
💬 TheCharlatan commented on pull request "kernel: Flush in ChainstateManager destructor":
(https://github.com/bitcoin/bitcoin/pull/31382#issuecomment-3052670668)
Rebased eec7c49cf777cdcc53aeb20e31fbd5881c2b008b -> 8512349ccd73c7acd8ccd6d4c2708459bdce3301 ([chainman_flush_destructor_5](https://github.com/TheCharlatan/bitcoin/tree/chainman_flush_destructor_5) -> [chainman_flush_destructor_6](https://github.com/TheCharlatan/bitcoin/tree/chainman_flush_destructor_6), [compare](https://github.com/TheCharlatan/bitcoin/compare/chainman_flush_destructor_5..chainman_flush_destructor_6))
💬 TheCharlatan commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-3052676701)
Rebased 98a34dd55ac32f323b297bb6d77eefe096f27074 -> 254d0a75b50b0eaf91003ea8a0534981ec740090 ([blocktreestore_1](https://github.com/TheCharlatan/bitcoin/tree/blocktreestore_1) -> [blocktreestore_2](https://github.com/TheCharlatan/bitcoin/tree/blocktreestore_2), [compare](https://github.com/TheCharlatan/bitcoin/compare/blocktreestore_1..blocktreestore_2))

* Fixed conflict with #32835
💬 maflcko commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#issuecomment-3052678805)
Looks like the release notes were changed a bit. No other changes.

re-ACK 4c772cbd83e502a1339e8993d192ea6416ecd45c 🕚

<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+krxU
...
💬 andrewtoth commented on pull request "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries":
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r2195046660)
Done.
💬 andrewtoth commented on pull request "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries":
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r2195046932)
Done.
💬 andrewtoth commented on pull request "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries":
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r2195047148)
Done.