💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2189573061)
Which doc? This is in the output of `bitcoin-cli help sendrawtransaction`.

✅ maflcko closed a pull request: "ci: Catch tests corrupting the source directory"
(https://github.com/bitcoin/bitcoin/pull/32874)

💬 maflcko commented on pull request "ci: Catch tests corrupting the source directory":
(https://github.com/bitcoin/bitcoin/pull/32874#issuecomment-3044435815)
Closing for now. Probably best to move this to a dedicated nightly ci, than to try to force it into the existing ci infra.

📝 maflcko opened a pull request: "bench: Avoid tmp files in pwd"
(https://github.com/bitcoin/bitcoin/pull/32890)
It is a bit confusing that one bench run, when aborted, could leave behind temp files in the current working directory. It is similarly confusing to delete those files in the next run of bench.

Fix all issues by using `BasicTestingSetup`, which provides a proper temp folder to use and also cleans up after itself.

Can be tested via:

```
( echo 'my file content' > streams_tmp ) && ls streams_tmp && ./bld-cmake/bin/bench_bitcoin --filter=FindByte && ls streams_tmp
```

Previously the f
...

💬 maflcko commented on pull request "threading: use correct mutex name in reverse_lock fatal error messages":
(https://github.com/bitcoin/bitcoin/pull/32829#issuecomment-3044551062)
Done in https://github.com/bitcoin/bitcoin/pull/32888

💬 instagibbs commented on pull request "p2p: Relax BlockRequestAllowed to respond to advertised blocks":
(https://github.com/bitcoin/bitcoin/pull/32869#issuecomment-3044614613)
@davidgumberg might be talking past each other, but what's happening here is that locally as soon as the block is reconstructed (including merkle checks), prior to script checks, the block is advertised forward via compact blocks to its peers. This is inline with the protocol description, and helps speed up block propagation.

💬 instagibbs commented on pull request "p2p: Relax BlockRequestAllowed to respond to advertised blocks":
(https://github.com/bitcoin/bitcoin/pull/32869#discussion_r2189796353)
will do on touchup

🤔 Prabhat1308 reviewed a pull request: "ci: Use optimized Debug build type in test-each-commit"
(https://github.com/bitcoin/bitcoin/pull/32888#pullrequestreview-2993523631)
ACK [`fa21d3c`](https://github.com/bitcoin/bitcoin/pull/32888/commits/fa21d3c7781f36d96aa13fb32cad40aad5b5131f)

changes seem good as discussed in #32829 . Can we use `mold` as a linker in other Linux based system workflows ? dependencies we have seem to satisfy the deps here https://github.com/rui314/mold?tab=readme-ov-file#how-to-build

💬 maflcko commented on pull request "ci: Use optimized Debug build type in test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/32888#issuecomment-3044773485)
> Can we use `mold` as a linker in other Linux based system workflows ?

Sure, happy to review a follow-up. Only place to avoid it would probably the ci tasks that mirror the guix build (win-cross, mac-cross)

💬 instagibbs commented on pull request "p2p: Relax BlockRequestAllowed to respond to advertised blocks":
(https://github.com/bitcoin/bitcoin/pull/32869#discussion_r2189897570)
fixed

💬 instagibbs commented on pull request "p2p: Relax BlockRequestAllowed to respond to advertised blocks":
(https://github.com/bitcoin/bitcoin/pull/32869#discussion_r2189897816)
done

👍 hodlinator approved a pull request: "test: fix feature_init.py intermittencies"
(https://github.com/bitcoin/bitcoin/pull/32835#pullrequestreview-2993615431)
ACK 4207d9bf823bea9f94b484ebf3c9274ca781b245

Changes since my concept review (https://github.com/bitcoin/bitcoin/pull/32835#issuecomment-3029241989):
* Addressed my feedback (https://github.com/bitcoin/bitcoin/pull/32835#pullrequestreview-2979101334)

(Would still prefer less stringly typed code, but have a feeling my suggestion in https://github.com/bitcoin/bitcoin/pull/32835#discussion_r2180861847 could probably be improved).

📝 1BitcoinBoWP1FZ4xwTNkq6XksKidmgYYw opened a pull request: "New SVG, Icons, PNGs and X PixMaps"
(https://github.com/bitcoin/bitcoin/pull/32891)
This PR clean up the old SVG, Icons, PNGs and X PixMaps by providing optimized much smaller file size versions while at the same time keeping resolutions untouched.
Shadows are removed from the files.

Ping: @jonasschnelli because he was the author of the previous files. Please check the PR.

New SVG source code:
`<?xml version="1.0" encoding="UTF-8"?>
<svg width="1024px" height="1024px" version="1.1" viewBox="0 0 1024 1024" xmlns="http://www.w3.org/2000/svg">
<g id="bitcoin">
<path
...

💬 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_r2189866340)
> Can you explain these performance reasons?

Not having to acquire a lock and iterate over an entire map.

> But shouldn't we only prefix the dropped lines by `*` anyway

No, the point is to continue making it visible to users that log lines are being suppressed, because they might have missed the initial announcement.

💬 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_r2189951621)
We're trying to get this in for 30.0. We shouldn't rush it in if we have concerns about the stability or effectiveness of this code, but blocking it for stylistic or test improvement purposes seems counter-productive when they can be done in a follow-up, ensuring progress on the important bits.

🤔 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-2993531256)
> My main concerns are that the code doesn't seem finished yet, it still contains commented out code, typos, redundant comments, dangerous recursion, prefixes for non-dropped log lines, duplicate state management - and I'm not even entirely sure we should be restricting logs per line in the first place.

I've responded to most of your concerns in-line. I think you've left some excellent suggestions for a follow-up, but I've not seen anything that imo warrants not merging this PR in this curren
...

💬 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_r2189897739)
> contains multiple styles

I don't think that's a good reason to hold up a useful and important PR that already has gone several iterations (in this and a previous PR) when nitting about documentation styles can block progress entirely as eventually reviewers lose interest and dilligence in re-reviewing. Follow-ups are entirely appropriate places for this to happen.

💬 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_r2189948409)
> when the logging is called via some dedicated method which hijacks the location

I don't think we can reasonably protect against malicious modifications to the binary?

> Or if macros are messing up the lines in any way.

That seems both unlikely and just something that should be, and is, tested.

> you just have to make sure you exhaust the target logger

"just" seems inappropriate, unconditional logging _should_ always take care that it can never be used to exhaust resources, but w
...

💬 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_r2189884875)
> I'm not sure I fully understand yet why we need these states

It's because the callsites behave differently whether a log line is un, newly or still suppressed. Previously, we encapsulated all that logic inside the `Consume()` function (then called `NeedsRateLimiting()`, but that gave the function too many responsibilities (as I believe you also pointed out), so this cleans that up. It think perhaps working out the code you'd prefer would be helpful.

💬 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_r2189872916)
This is a public class, explaining the parameters and interface seems very desirable to me. `scheduler_func` could mean many things, when I review code I like to be able to quickly see what something's intent is. This seems like a pretty obvious best practice?