🤔 glozow reviewed a pull request: "Remove mempoolfullrbf"
(https://github.com/bitcoin/bitcoin/pull/30592#pullrequestreview-2400041340)
> `doc/bips.md/` bip125 is still opt-in, we are drifting further and further away from that BIP and will finally basically kill it off with cluster mempool. Not sure what we want to do there.

I think usually if we stop implementing a BIP we can just remove it from bips.md. Or you can say "BIP 125 is not supported. See doc/policy/mempool-replacements.md for replacement policy" ?

💬 glozow commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#discussion_r1819648604)
Deprecated means still supported but discouraged
```suggestion
1. (Removed)
```

💬 glozow commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#discussion_r1819647758)
That's not what deprecated means, maybe just replace the whole thing with "(Removed)"

💬 glozow commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#discussion_r1819649058)
Move to bottom to retain chronological ordering?

💬 glozow commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#discussion_r1819652441)
There is a test for both cases v3 + BIP125 and for v3 + non-BIP125 iirc

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1819663339)
I squashed the change in here: ad95b2c0e21f1e865f967aa9463ef99bc252550a

💬 l0rinc commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#discussion_r1819663675)
> Does changing strprintf to tfm_format_raw here catch any potential issues that wouldn't be seen in manual testing?

By manual testing do you mean automated testing of print statements?

> Clang-format can be helpful for new code, but I don't think it's necessary to re-indent code for it that isn't otherwise touched

Please format affected code, don't leave it unformatted, that's why the current code is so confusing.
```
Coding Style (General)
----------------------

Various coding s
...

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#issuecomment-2442487595)
Squashed @naumenkogs fix on @marcofleon comment in ad95b2c0e21f1e865f967aa9463ef99bc252550a, plus added a commit replacing `IsFanoutTarget` with `GetFanoutTargets` so the cache for fanout can be dropped. After this, the fanout targets are computed just once per transaction

💬 l0rinc commented on pull request "ci: Use clang-19 from apt.llvm.org":
(https://github.com/bitcoin/bitcoin/pull/30634#issuecomment-2442503039)
Concept ACK

💬 instagibbs commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#discussion_r1819683400)
done

💬 instagibbs commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#discussion_r1819683484)
taken

💬 instagibbs commented on pull request "Remove mempoolfullrbf":
(https://github.com/bitcoin/bitcoin/pull/30592#issuecomment-2442518446)
@glozow tried a version of your suggestion which should be more future proof

💬 jonatack commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#discussion_r1819693843)
> Please format affected code, don't leave it unformatted, that's why the current code is so confusing.

The indentation wasn't wrong, and in the past, long-term contributors generally or often didn't wish to reformat code in their pulls that wasn't wrong in some dangerous way when newer contributors asked for reformatting in reviews. There has been extensive bikeshedding over what the clang-format setup here should even do. Having these arguments on the same line read left-to-right, which was
...

🤔 mzumsande reviewed a pull request: "Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior"
(https://github.com/bitcoin/bitcoin/pull/30529#pullrequestreview-2400135259)
Concept ACK

Does this PR have the aspiration to fix all instances where it makes sense?

If so, I think that the pattern from 40ecf91068545ba55a6af5e2536a61c459334925 could also be applied to `-port` and `-rpcport` in `CheckHostPortOptions()` or `-asmap`, which all fail init in master when negated and which could be used to negate earlier options if changed.

💬 l0rinc commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#discussion_r1819707701)
There was no bikeshedding about this before on https://github.com/bitcoin/bitcoin/pull/31149, it just started.

💬 jonatack commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#discussion_r1819737341)
> By manual testing do you mean automated testing of print statements?

No, manual testing by building and running the CLI commands.

There is no automated test coverage of `-netinfo` (the touched code above), nor, for instance, of `-addrinfo`.

👍 l0rinc approved a pull request: "tinyformat: enforce compile-time checks for format string literals"
(https://github.com/bitcoin/bitcoin/pull/31149#pullrequestreview-2400193814)
Compared to https://github.com/bitcoin/bitcoin/pull/30928 I see the following changes:
* clientversion.cpp: Merge conflict in clientversion at CopyrightHolders -> Seems to be resolved correctly
* logging.h: old LogPrintFormatInternal is retained
* run-lint-format-strings.py: FALSE_POSITIVES not removed
* util_string_tests.cpp: FALSE_POSITIVES not moved to tests
* strprintf.cpp: fuzz_fmt added to avoid renames, try-catch added around fuzz_fmt calls (any reason for not try-catching fuzz_fmt i
...

💬 l0rinc commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#discussion_r1819746061)
By doing some of the validation at compile time, there **is** now some automated 'test coverage' on it - i.e. it will bite back if it's very wrong without any additional manual check.

💬 jonatack commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#discussion_r1819751107)
Generally unneeded changes can be avoided, but that was just a suggestion. Further historical context to get started:

- https://github.com/bitcoin/bitcoin/issues/18651
- https://github.com/bitcoin/bitcoin/issues/15465

💬 l0rinc commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#discussion_r1819774471)
Useful context, thanks!
We must however keep our kitchen clean, we can't just be cooking all the time.
This change by @stickies-v was a small, low-risk change, making the code slightly more maintainable (or at least slightly less unmaintainable).
We should encourage these.