💬 fjahr commented on pull request "init: Require explicit -asmap filename":
(https://github.com/bitcoin/bitcoin/pull/33770#issuecomment-3497066592)
Thanks @ryanofsky for the ping and for moving the conversation forward.

I am -0 on this change for now because in the recollection of conversations about ASMap usage whenever the conversation touched this (which was rarely) then either people were indifferent about the default path or found it nice to have. I don’t recollect anyone expressing negativity about it, as in it doesn’t make sense or it won’t be used. I can not point at specific people or conversations that were documented for this,
...

💬 dergoegge commented on pull request "refactor: optimize block index comparisons (1.4-6.8x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#discussion_r2498817972)
> I look at fuzz tests as exploratory tools that are hard to write and run and debug and do code coverage ... not trivial to maintain ... not my go-to way to test something

There is a learning curve to writing and using fuzz testing but it is 100% worth getting in to. The maintenance aspect only relates to running them using libFuzzer on macOS.

On average, coverage guided fuzzing will be magnitudes better at finding bugs than this style of unit test. For something as small as the comparato
...

💬 yancyribbens commented on pull request "docs: add doc comment for SRD selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/33622#discussion_r2498830734)
> Are you using it as a synonym for “reduce”?

Yes, that is correct. I was using it in terms of a minimization function, that is, it is a step in the direction of a _more_ minimal solution. Agree to remove that wording though if your don't like it; I can see why it's confusing.

> Removing the OutputGroup with the lowest effective_value is a simple algorithm that increases the average effective_value per weight of the selection.

Said another way, the selection density is biased towards
...

💬 roconnor-blockstream commented on pull request "Relax standardness rules regarding CHECKMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/33755#issuecomment-3497161573)
Doe this mean that, due to `AreInputsStandard`, even more UTXOs were soft-confiscated when #12460 was added?

💬 fjahr commented on issue "Default ASmap file path is not used unless -asmap is set":
(https://github.com/bitcoin/bitcoin/issues/33386#issuecomment-3497169826)
> Remove the default value to prevent any confusion.

@vostrnad since you reported this issue and [#33770](https://github.com/bitcoin/bitcoin/pull/33770) implements this suggestion of your, could you say if the removal would be your preferred resolution or if you would like to keep the option around? I think you haven't said if you wanted to actually use the default path or if you just happened to stumble upon this. Thanks!

💬 ryanofsky commented on pull request "init: Require explicit -asmap filename":
(https://github.com/bitcoin/bitcoin/pull/33770#issuecomment-3497216753)
> [P]eople were indifferent about the default path or found it nice to have. I don’t recollect anyone expressing negativity about it, as in it doesn’t make sense or it won’t be used

To be sure, I think it could make sense to have a default path that is loaded by default. I think it just to confusing to have a default path that is not loaded by default, and I think https://github.com/bitcoin/bitcoin/issues/33386 shows I'm not the only one who finds the current situation confusing.

Also if #
...

💬 maflcko commented on pull request "util: Allow Assert() in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2498944152)
I think a temporary workaround, local to two CI tasks, is fine. I don't think there is value in doing more here. See https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2498265195. Leaving as-is for now.

💬 maflcko commented on pull request "util: Allow Assert() in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2498945053)
Thx, done for all in a new, unrelated, commit.

💬 maflcko commented on pull request "util: Allow Assert() in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2498945189)
My preference is to just use `%s` for all args, unless there is a reason not to. So I'll leave this as-is for now.

Now that the libc++ minimum version is 17, which includes `std::format`, in about 12 months, we can probably bump GCC to 13 as well. Then `{}` can be used for all args, unless there is a reason not to.

💬 maflcko commented on pull request "util: Allow Assert() in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2498947280)
thx, ran clang-format while changing the `{}` initialization.

💬 maflcko commented on pull request "util: Allow Assert() in contexts without __func__":
(https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2498948494)
thx, removed redundant and fixed NOLINT

📝 waketraindev opened a pull request: "qt: Comment out sensitive commands in history to prevent re-execution"
(https://github.com/bitcoin/bitcoin/pull/33807)
Prefix filtered commands with "#" before adding them to the RPC console history.

Console lines starting with "#" are ignored by the executor, preventing accidental re-execution of filtered sensitive commands when recalled from history.

Also adds a noop signal to handle ignored commands cleanly and documents comment behavior in the console help text (`help-console`).

✅ waketraindev closed a pull request: "qt: Comment out sensitive commands in history to prevent re-execution"
(https://github.com/bitcoin/bitcoin/pull/33807)

📝 waketraindev opened a pull request: "qt: Comment out sensitive commands in history to prevent re-execution"
(https://github.com/bitcoin-core/gui/pull/909)
Prefix filtered commands with "#" before adding them to the RPC console history.

Console lines starting with "#" are ignored by the executor, preventing accidental re-execution of filtered sensitive commands when recalled from history.

Also adds a noop signal to handle ignored commands cleanly and documents comment behavior in the console help text (`help-console`).

💬 waketraindev commented on pull request "qt: Comment out sensitive commands in history to prevent re-execution":
(https://github.com/bitcoin/bitcoin/pull/33807#issuecomment-3497269907)
closed, pr was meant for gui repository. sorry.

💬 fjahr commented on pull request "init: Require explicit -asmap filename":
(https://github.com/bitcoin/bitcoin/pull/33770#issuecomment-3497304815)
> To be sure, I think it could make sense to have a default path that is loaded by default. I think it just to confusing to have a default path that is not loaded by default, and I think #33386 shows I'm not the only one who finds the current situation confusing.

Sure I don't question that there is a potential confusion, we just disagree on whether the confusion is bad enough to take away the feature. I would like to go with whatever the users want which might be in favor of keeping the optio
...

👍 rkrux approved a pull request: "wallet/rpc: fix listdescriptors RPC fails to return descriptors with private key information when wallet contains descriptors missing any key"
(https://github.com/bitcoin/bitcoin/pull/32471#pullrequestreview-3428265433)
reACK 2b16014

```
git range-diff 8b4d4b3...2b16014
```

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#issuecomment-3497405231)
Pushed minor fixes after comments:
- simplify test assertions
- simplify computing of total/avg and max (no for loop)
- formatting

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2499080199)
"0--10" meant "from 0 to 10 inclusive". Changed to "0-10" to avoid parsing issues.

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2499081771)
The checks have been simplified, usage of "magic numbers" reduced.