💬 fanquake commented on pull request "test: update BIP340 test vectors and implementation (variable-length messages)":
(https://github.com/bitcoin/bitcoin/pull/32642#issuecomment-2921839411)
cc @sipa @real-or-random @jonasnick

💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2921871074)
https://github.com/bitcoin-core/libmultiprocess/pull/172 landed and the subtree was updated in #32641.

Marking ready to review again.

Although #31679 is orthogonal, it's also worth reviewing. There should be no impact in user experience, because thanks to https://github.com/bitcoin/bitcoin/pull/31375 `bitcoin -m` will find `bitcoin-node` whether it's in `/usr/local/bin` or `/usr/local/libexec`. But an upgrade would leave one binary in both paths, so it's nicer to get that PR in the same re
...

👋 Sjors's pull request is ready for review: "Add bitcoin-{node,gui} to release binaries for IPC"
(https://github.com/bitcoin/bitcoin/pull/31802)

📝 fanquake opened a pull request: "doc: miscellaneous changes"
(https://github.com/bitcoin/bitcoin/pull/32644)
A round up of #32629 + some other changes that had previously been PR'd.

🚀 fanquake merged a pull request: "[28.x] Backport guix: accomodate migration to codeberg"
(https://github.com/bitcoin/bitcoin/pull/32639)

💬 TheCharlatan commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#issuecomment-2921899092)
Concept ACK

🚀 fanquake merged a pull request: "wallet, rpc, gui: List legacy wallets with a message about migration"
(https://github.com/bitcoin/bitcoin/pull/32619)

🤔 stickies-v reviewed a pull request: "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError"
(https://github.com/bitcoin/bitcoin/pull/32604#pullrequestreview-2880741415)
Concept ACK on locking down this attack vector, and the code changes required seem acceptable. Also nice to start using `std::source_location` here.

💬 stickies-v commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2115514944)
nit: I think a brief docstring on their suggested usage here would be helpful here

💬 stickies-v commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2115526293)
Is there a good use case for disabling rate limiting? I can't think of one. Letting the user define the amount of rate limiting is an alternative, but even then, I'm not sure. If an attacker or bug is able to reach the sane default maximum, then is the user really going to get any benefit from being to inspect the additional volume of un-ratelimited logs? I'm not sure. If so, I think it would make sense to not have this as a configurable option.

💬 stickies-v commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2115570067)
Hmm, I'm feeling uneasy about using `m_categories`, when rate limiting flags fundamentally don't represent a category. I can see how it reduces the diff, but I'm not sure that's worth it. I'll think about an alternative approach - have you considered any already?

Since the user doesn't have to be able to configure any of this, and we only have a single `UNCONDITIONAL_ALWAYS` use case (and likely won't have (many) more in the future), I think adding an `is_conditional=True` arg to `LogPrintLev
...

👍 Sjors approved a pull request: "walletdb: Log additional exception error messages for corrupted wallets"
(https://github.com/bitcoin/bitcoin/pull/32598#pullrequestreview-2880884812)
utACK ad9a13fc424e9deb262e2b1d54bcdc7370263ea0

🚀 fanquake merged a pull request: "walletdb: Log additional exception error messages for corrupted wallets"
(https://github.com/bitcoin/bitcoin/pull/32598)

💬 fanquake commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#issuecomment-2922025698)
> The windows cross-built job is failing on the rate_limiting test at every file-size comparison. I don't have a windows machine to debug this, but I think maybe fs::file_size is failing or some other quirk is showing up?

@hebasto can you take a look here? I ran `test_bitcoin.exe` from this branch (rebased) under wine and didn't see any failures.

💬 fanquake commented on pull request "wallet: Translate [default wallet] string in progress messages":
(https://github.com/bitcoin/bitcoin/pull/31296#issuecomment-2922075703)
@ryanofsky want to rebase here?

🤔 ismaelsadeeq reviewed a pull request: "cmake: Move internal binaries from bin/ to libexec/"
(https://github.com/bitcoin/bitcoin/pull/31679#pullrequestreview-2881033332)
Concept ACK

💬 furszy commented on pull request "wallet: migration, avoid creating spendable wallet from a watch-only legacy wallet":
(https://github.com/bitcoin/bitcoin/pull/31423#discussion_r2115721183)
> I think empty_local_wallet is unnecessary and the condition can check local_wallet->GetAllScriptPubKeyMans().empty() directly.

That would also delete blank legacy wallets.

💬 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_r2115756875)
That's a good point, I can't think of any concrete use case to disable rate limiting here.

💬 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_r2115778728)
I haven't considered alternative approaches, but I think adding a boolean is better than adding special-cased categories.

💬 furszy commented on pull request "wallet: re-activate "AmountWithFeeExceedsBalance" error":
(https://github.com/bitcoin/bitcoin/pull/25269#discussion_r2115800834)
ha.. true. It seems I'm not putting much brain power here.. updated per feedback. Thanks.