✅ maflcko closed a pull request: "New SVG, Icons, PNGs and X PixMaps"
(https://github.com/bitcoin/bitcoin/pull/32891)

💬 maflcko commented on pull request "New SVG, Icons, PNGs and X PixMaps":
(https://github.com/bitcoin/bitcoin/pull/32891#issuecomment-3045041266)
Closing for now:

* Not sure if removing shadows is the right approach to save a few KB. This needs a stronger motivation and screenshots on all supported Systems.
* Gui pull request should be opened against the gui repo: github.com/bitcoin-core/gui
* Please don't include the source code in the pull description. If anyone wanted to look at the source code, they could just look at it directly.
* Please don't ping for reviewers in the pull description. Also, pinging people that have not been
...

🤔 ryanofsky reviewed a pull request: "cmake: Move internal binaries from bin/ to libexec/"
(https://github.com/bitcoin/bitcoin/pull/31679#pullrequestreview-2993763846)
Updated 6a6b6f9093c1f82b8ea05ad87f421eabc8849738 -> f49840dd902cd9b14b6aadb431b16a4aeb719c3f ([`pr/libexec.10`](https://github.com/ryanofsky/bitcoin/commits/pr/libexec.10) -> [`pr/libexec.11`](https://github.com/ryanofsky/bitcoin/commits/pr/libexec.11), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/libexec.10..pr/libexec.11)) addressing remaining comments and making `bitcoin` help output more consistent

💬 ryanofsky commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2190024343)
re: https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2185072353

> The table below shows the install location and availability **of** each binary after this change

Makes sense, fixed this too

💬 ryanofsky commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2190019152)
re: https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2189175462

Oops, you're right. I forgot the `--help` help and only updated the `help` help. Sound be fixed now, thanks!

💬 ryanofsky commented on pull request "cmake: Move internal binaries from bin/ to libexec/":
(https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2190022159)
re: https://github.com/bitcoin/bitcoin/pull/31679#discussion_r2189175462

> nit: we might want to realign the comments after the change

Thanks! Updated now

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