💬 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.

💬 fanquake commented on pull request "guix: build for Linux HOSTS with `-static-libgcc`":
(https://github.com/bitcoin/bitcoin/pull/33181#issuecomment-3497508754)
Guix Build (aarch64):
```bash
05fc5cae41b19bbfdec5942e334e7b7d78ee0406ba69c221381b0b07b0a9e886 guix-build-f06c6e189831/output/aarch64-linux-gnu/SHA256SUMS.part
961613408c4d0b3b169488b43edc838135be0b712740b4015457f4f2808e103b guix-build-f06c6e189831/output/aarch64-linux-gnu/bitcoin-f06c6e189831-aarch64-linux-gnu-debug.tar.gz
906437c316d50e1f60fdfe2098fe72f917be109a68a2103e915183ed4fe01947 guix-build-f06c6e189831/output/aarch64-linux-gnu/bitcoin-f06c6e189831-aarch64-linux-gnu.tar.gz
73a257
...

👍 l0rinc approved a pull request: "util: Allow `Assert` (et al.) in contexts without __func__"
(https://github.com/bitcoin/bitcoin/pull/33785#pullrequestreview-3428691572)
Code review ACK fad6efd3bef1d123f806d492f019e29530b03a5e

The only nit I still have is the global suppression, but we can also just tackle that in a follow-up if needed.

🤔 rkrux reviewed a pull request: "descriptor: fix comments in descriptor.cpp::DescriptorImpl"
(https://github.com/bitcoin/bitcoin/pull/33384#pullrequestreview-3428691088)
CR 768eeafbda3b36428d90239c92ddfb41402cfd0c

💬 rkrux commented on pull request "descriptor: fix comments in descriptor.cpp::DescriptorImpl":
(https://github.com/bitcoin/bitcoin/pull/33384#discussion_r2499345608)
This doesn't seem to be correct. I debugged the unit tests & the `listdescriptors` RPC, and found that `m_pubkey_args.size()` remains 1 for `tr()` descriptors as well. It's the `m_subdescriptor_args.size()` that increases if script path(s) is(are) present. Following points support this:
1. The `TRDescriptor` constructor accepts a single `PubkeyProvider` that's treated as the Taproot internal key: https://github.com/bitcoin/bitcoin/blob/5c5704e730796c6f31e2d7891bf6334674a04219/src/script/descript
...