💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1850740240)
nit: since we're using all values later (here and in a few other examples below), we might as well leave this as `IsArgSet` at the top - it would simplify the PR diff, but make the scripted diff more complicated. Will leave it for you to decide, don't have strong feelings either way.

💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1850750700)
Do these same arguments apply to `blocksdir` & `walletdir`?

💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1850760249)
Since you edit these already, could you please check if the trailing newline is still needed?

💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1850768609)
You gotta' love these 3-state-booleans :)

nit: To reduce the noise, consider simplified to either
```suggestion
if (std::optional result{GenerateAuthCookie(&strRPCUserColonPass, cookie_perms)}) {
```
or
```suggestion
if (auto result{GenerateAuthCookie(&strRPCUserColonPass, cookie_perms)}) {
```

💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1850771750)
this seems like a slight behavior change: previously when `GenerateAuthCookie` returned `true`, this was logged, now it seems to me it may not be (haven't tried the code)

💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1850774218)
nit: we might as well use f-strings here:
```suggestion
assert_equal(BLOCKS, self.nodes[0].cli('-norpccookiefile', f'-rpcuser={user}', f'-rpcpassword={password}').getblockcount())
```

💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1850762988)
super-nit: formatting seems a bit off (whitespace and missing trailing full stop in first line)

🤔 furszy reviewed a pull request: "fuzz: Implement G_TEST_GET_FULL_NAME"
(https://github.com/bitcoin/bitcoin/pull/31333#pullrequestreview-2449405248)
utACK 92d3d691f097ead8e5ae571eb9bf691133a6aa49

💬 TheCharlatan commented on pull request "validation: fetch block inputs on parallel threads ~17% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r1848485522)
Should this rather say "optionally marking the emplaced coin as not dirty", since the default is always dirty?

🤔 l0rinc reviewed a pull request: "refactor: Check translatable format strings at compile-time"
(https://github.com/bitcoin/bitcoin/pull/31061#pullrequestreview-2449445449)
utACK f4df783f1ca22d96476d52ec5d1929547691ba13

💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850825553)
leaving as is

💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850825631)
removed

💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850825725)
done

💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850825828)
took modified version of file-local constant

💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850825989)
IIUC I'd have needed to at least make the RHS an optional to do that?

Either way, this goes away in a later commit, so I'm going to leave as-s.

💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850826058)
done

💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850826205)
Setting `-dustrelayfee=0` avoids the check much more directly and simply, with no recompilation needed.

Gating it based on standardness checks is just easier to understand when e.g., standardness isn't being enforced anyways, so dust should be treated as a no-op.

I'm going to mark this as resolved. If we want to reverse this logic, imo we should do it in another PR that removes both the logic in prioritisation as well as the modified fee checks.

💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850826295)
going to leave as is

💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1850826413)
sure, changed

💬 instagibbs commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#issuecomment-2489321625)
reworked https://github.com/bitcoin/bitcoin/commit/03ec3f6230f440b264abe3aa42a57fdd29bd3ba3 commit message

> Deduplicate assert_mempool_contents()
Deferring to future work since that's a different test, I'll review it if someone opens a clenaup PR.

> Sanity check for assert_mempool_contents()

Good idea, added minimal diff