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

💬 vasild commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-3497705768)
I re-assessed this carefully.

It seems ok to me to have v2only with the following properties:

* Can only be enabled if not listening, like suggested above. Otherwise we have to either:
* accept v1 incoming which defeats the purpose of this because then we will have cleartext connections or
* deny v1 incoming (only allow v2 incoming) which is a problem if this is widely adopted because then old nodes that do not support v2 will have a hard time finding peers to connect to and will b
...

👍 l0rinc approved a pull request: "test: Add test on skip heights in CBlockIndex"
(https://github.com/bitcoin/bitcoin/pull/33661#pullrequestreview-3428711246)
ACK ea998e18f6d64f485d4e2d85a1028474fb35120a

Thanks for taking my suggestion, the code is very clean and easy to understand now.
I left some nits I still saw, but they're not blocking.

💬 l0rinc commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2499361757)
nit: below we're using brace init + dividing a `double` by a `size_t` is also double-division (untested):
```suggestion
const auto avg{double(std::accumulate(skip_diffs.begin(), skip_diffs.end(), 0)) / skip_diffs.size()};
```

💬 l0rinc commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2499366014)
nit: redundant comment

💬 l0rinc commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2499367082)
nit: redudnant comment, the code already expresses this clearly

💬 l0rinc commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2499375298)
nit: should likely be a `BOOST_REQUIRE` since this is still the setup page, not yet the validation stage

💬 maflcko commented on pull request "refactor: optimize block index comparisons (1.4-6.8x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#issuecomment-3497856061)
> Profiling the performance regression in [#33618 (comment)](https://github.com/bitcoin/bitcoin/issues/33618#issuecomment-3399937858) revealed that `CBlockIndexWorkComparator` and its underlying `base_uint<256u>::CompareTo` are hot paths during block validation, consuming ~4% of CPU time.

I don't follow here. The issue is about the additional `CWallet::WriteBestBlock` overhead? `CBlockIndexWorkComparator` seems unrelated to the issue, at least I can't see how it changed the result between 29.
...