💬 darosior commented on pull request "BIP-119 (OP_CHECKTEMPLATEVERIFY) (regtest only)":
(https://github.com/bitcoin/bitcoin/pull/31989#issuecomment-3045062002)
I [believe](https://gnusha.org/pi/bitcoindev/F5vsDVNGXP_hmCvp4kFnptFLBCXOoRxWk9d05kSInq_kXj0ePqVAJGADkBFJxYIGkjk8Pw1gzBonTivH6WUUb4f6mwNCmJIwdXBMrjjQ0lI=@protonmail.com/) the ability to commit to the transaction to spend an output, combined with BIP340 signature verification of arbitrary messages, is a reasonable bundle of capabilities to consider for a Bitcoin soft fork. However there is clearly no consensus about this among the Bitcoin development community (see for instance [here](https://gnu
...

💬 maflcko commented on issue "fuzz: Speed up mini_miner fuzz target?":
(https://github.com/bitcoin/bitcoin/issues/32870#issuecomment-3045070337)
I am not. If you want to work on something, you can just do it without asking. In case there are two pull requests for the same thing, the authors should be glad about it, because they should be qualified to review each others pull request and pick the one that is more suitable to be merged.

💬 enirox001 commented on issue "fuzz: Speed up mini_miner fuzz target?":
(https://github.com/bitcoin/bitcoin/issues/32870#issuecomment-3045099798)
Got it.

💬 1BitcoinBoWP1FZ4xwTNkq6XksKidmgYYw commented on pull request "New SVG, Icons, PNGs and X PixMaps":
(https://github.com/bitcoin/bitcoin/pull/32891#issuecomment-3045152747)
@maflcko Thanks, I will use github.com/bitcoin-core/gui to send a new PR. Also I will squash the commits. There is a not just a few KB saved with the PR but a lot and not because removed shadows but because the files are optimized. The current files in the bitcoin/bitcoin:master are very old and not optimized.

💬 Eunovo commented on pull request "refactor: CFeeRate encapsulates FeeFrac internally":
(https://github.com/bitcoin/bitcoin/pull/32750#discussion_r2190134390)
+1 on the use of 'Assume' here.

🤔 marcofleon reviewed a pull request: "allocators: Apply manual ASan poisoning to `PoolResource`"
(https://github.com/bitcoin/bitcoin/pull/32581#pullrequestreview-2993932032)
code review ACK ad132761fc49c38769c09653a265fdbc3b93eda5

Built with ASan and ran the unit tests to be sure. Also added a small use-after-free test in `pool_tests` that passes on master but not on this PR, as expected. Nice.

💬 furszy commented on pull request "wallet: remove dead code in legacy wallet migration":
(https://github.com/bitcoin/bitcoin/pull/32758#discussion_r2190143690)
would keep the assertion. It would be bad if this is ever not the case and we end up resetting the pointer.

💬 l0rinc commented on pull request "mempool: Avoid needless vtx iteration during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#issuecomment-3045279193)
That's my expectation, yes. Would you like me to measure it to sure?

🤔 furszy reviewed a pull request: "index: fix wrong assert of current_tip == m_best_block_index"
(https://github.com/bitcoin/bitcoin/pull/32878#pullrequestreview-2994003229)
I don't think the test is useful. The change is being tested against a `Sync()` mechanism introduced within the test itself and not the real one, which can change over time (see for example #26966 that changes it completely).
It would be better, and simpler for you, to share a patch that reproduces the issue (as @mzumsande mentioned, just adding a sleep() call and triggering the reorg is enough to reproduce it).

Other than that, concept ACK.

💬 maflcko commented on pull request "New SVG, Icons, PNGs and X PixMaps":
(https://github.com/bitcoin/bitcoin/pull/32891#issuecomment-3045287022)
The are optimized, see https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/optimize-pngs.py

💬 fanquake commented on pull request "depends: fix libevent `_WIN32_WINNT` usage":
(https://github.com/bitcoin/bitcoin/pull/32837#issuecomment-3045291987)
The cross-arch non-determinism here is in `bitcoin-qt.exe`.
x86_64
```bash
55c49e31615341a1f8e72cd4c7193a3052843a2a21cecaa42d1c374faf4e1600 bitcoin-5565d7d610dd/bin/bitcoin-cli.exe
c91ca56fd76dfddf120d1f92f692eff99f4f4e7ff64f2c486b44ce08c4ef930c bitcoin-5565d7d610dd/bin/bitcoin-qt.exe
7ddc376534a0811e5c6dd3a49cfffafef1c23be9e76f1e5586bfdbba25a2dbe5 bitcoin-5565d7d610dd/bin/bitcoin-tx.exe
38cd1c4bd75862ff5a58052027d006140761cb4162ec59eb90b0ed4e9e0ddc0e bitcoin-5565d7d610dd/bin/bitcoin-u
...

💬 maflcko commented on pull request "test: fix feature_init.py intermittencies":
(https://github.com/bitcoin/bitcoin/pull/32835#issuecomment-3045335544)
lgtm ACK 4207d9bf823bea9f94b484ebf3c9274ca781b245 🍄

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: lgtm ACK 4207d9bf823bea9f
...

💬 maflcko commented on pull request "test, subprocess: Improve coverage report correctness":
(https://github.com/bitcoin/bitcoin/pull/30075#issuecomment-3045397135)
@hebasto are you still working on this?

💬 rkrux commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2189541657)
In 0d24f0af7a3430dd8861d99390d902cde21f4275 "wallet: Add addhdkey RPC"

I believe this requires a release note?

💬 rkrux commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2190005890)
In 10b677530e5fd9c502290acbcb9c706a2a368b5b "descriptor: Add unused(KEY) descriptor"

Perhaps we can update the doc of this function?
https://github.com/bitcoin/bitcoin/blob/e3f416dbf7633b2fb19c933e5508bd231cc7e9cf/src/script/descriptor.h#L111-L112

```diff
- /** Whether this descriptor will return one scriptPubKey or multiple (aka is or is not combo) */
+ /** Whether this descriptor will return at most one scriptPubKey or multiple (aka is or is not combo) */
```

Because in this c
...

💬 rkrux commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2190133772)
In 10b677530e5fd9c502290acbcb9c706a2a368b5b "descriptor: Add unused(KEY) descriptor"

Nit because there already is a note below.

```diff
- // Apply the label if necessary, for descriptors that produce output scripts (i.e. not unused())
- // Note: we disable labels for ranged descriptors
+ // Apply the label if necessary
+ // Note: we disable labels for descriptors that are ranged or that don't produce output scripts (i.e. unused())
```

💬 rkrux commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2190031968)
In 10b677530e5fd9c502290acbcb9c706a2a368b5b "descriptor: Add unused(KEY) descriptor"

For explicitness, loop can be replaced with an `Assume` on length being one.

```diff
- for (auto& pubkey : keys) {
- if (pubkey->IsRange()) {
- error = "unused(): key cannot be ranged";
- return {};
- }
- ret.emplace_back(std::make_unique<UnusedDescriptor>(std::move(pubkey)));
+ Assume(keys.size() == 1);
+ auto& pub
...

💬 rkrux commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2190191964)
In 0d24f0af7a3430dd8861d99390d902cde21f4275 "wallet: Add addhdkey RPC"

Only reason being to restrict the call sites of `MakeNewKey` function that at the moment is being used only in one place barring the unit tests - straightaway lets us know the places of key generation in the wallet.

```diff
- CKey seed_key;
- seed_key.MakeNewKey(true);
+ CKey seed_key = GenerateRandomKey();
```

💬 rkrux commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2190198179)
In https://github.com/bitcoin/bitcoin/commit/0d24f0af7a3430dd8861d99390d902cde21f4275 "wallet: Add addhdkey RPC"

Should we add a check for external signer wallet flag as well here?

💬 rkrux commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2190215458)
In https://github.com/bitcoin/bitcoin/commit/0d24f0af7a3430dd8861d99390d902cde21f4275 "wallet: Add addhdkey RPC"

```diff
- expected_void_desc = descsum_create(f"unused({imp_xpub})")
+ expected_unused_desc = descsum_create(f"unused({imp_xpub})")
```