💬 maflcko commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-2813376299)
> While this wasn't the main motivation for the change, IBD on Ubuntu/GCC on SSD with i9 indicates a 2% speedup as well:

If IBD speedup is not the main motivation, what else is the motivation, given that the title mentions `[IBD]`?

Also, what code part is hitting this during IBD, given that reads and writes are now buffered?

📝 l0rinc converted_to_draft a pull request: "[IBD] specialize block serialization"
(https://github.com/bitcoin/bitcoin/pull/31868)
This change is part of [[IBD] - Tracking PR for speeding up Initial Block Download](https://github.com/bitcoin/bitcoin/pull/32043)

### Summary

This PR contain a few different optimization I found by IBD profiling, and via the newly added block seralization benchmarks. It also takes advantage of the recently merged [`std::span` changes](https://github.com/bitcoin/bitcoin/pull/31519) enabling propagating static extents.

The commits merge similar (de)serialization methods, and separates th
...

💬 l0rinc commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#discussion_r2049233469)
Yes:
> typedef unsigned char uint8_t;

💬 l0rinc commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-2813392140)
> what code part is hitting this during IBD

During serialization we have very many 1 byte writes, this change avoids the heavy allocations.
But drafting until I have time to properly remeasure everything after the recent merge.

💬 maflcko commented on pull request "[IBD] specialize block serialization":
(https://github.com/bitcoin/bitcoin/pull/31868#issuecomment-2813398761)
I understand that there are many 1-byte writes, but they go into the BufferedReader/Writer, which is not modified here. The Buffered* then passes them to AutoFile, which is modified here, but never receives 1-byte writes anymore

💬 marcofleon commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#discussion_r2049250853)
Good call, done. Made me realize I should update `vMatch` in the parent PR, so thanks.

💬 marcofleon commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#discussion_r2049251124)
Done.

💬 marcofleon commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#discussion_r2049251391)
Done.

💬 marcofleon commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#discussion_r2049251829)
Done.

💬 marcofleon commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#discussion_r2049252754)
Done.

💬 ryanofsky commented on pull request "doc: Add deps install notes for multiprocess":
(https://github.com/bitcoin/bitcoin/pull/32293#issuecomment-2813429363)
Code review ff4ea070aa4b1ed1ebb6dcf62e01c9dd57319f69

Thanks for adding this. Somehow I thought the capnproto dependencies were already listed, but that change is sitting in a commit in #10102 (bbcbe85cef21049b5f321265264e2c4fe394d209).

Few suggestions:

- Would suggest changing "Multiprocess Dependencies" to "IPC Dependencies" since the only thing they are use for on master is -ipcbind feature, and multiprocess functionality is a superset of IPC functionality.
- Might be good to state e
...

💬 marcofleon commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#discussion_r2049254166)
I'll leave this as is for now. A bit out of scope for this PR.

💬 marcofleon commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#discussion_r2049254712)
Agreed, done.

💬 marcofleon commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#discussion_r2049256701)
Done. I love structured bindings now.

💬 marcofleon commented on pull request "qt, wallet: Convert uint256 to Txid":
(https://github.com/bitcoin/bitcoin/pull/32238#discussion_r2049256933)
Done.

💬 furszy commented on pull request "wallet: Automatically repair corrupted metadata with doubled derivation path":
(https://github.com/bitcoin/bitcoin/pull/29124#discussion_r2049089569)
it would be better to use `find()` instead of `at()` in case the key metadata is not in the wallet. E.g. an edge case.. could probably happen when `hdKeypath` path matches the 0.21 bug pattern but fails the hardened bits checks (this requires user's manual intervention to occur).

💬 l0rinc commented on pull request "ci: switch to LLVM 20 in tidy job":
(https://github.com/bitcoin/bitcoin/pull/32226#issuecomment-2813461635)
Was `modernize-type-traits` removed from `src/.clang-tidy` in latest push on purpose?

💬 fanquake commented on pull request "ci: switch to LLVM 20 in tidy job":
(https://github.com/bitcoin/bitcoin/pull/32226#issuecomment-2813463406)
Yes.

💬 hebasto commented on issue "ci: failure in Windows cross-test":
(https://github.com/bitcoin/bitcoin/issues/32291#issuecomment-2813495750)
Reminds https://github.com/bitcoin/bitcoin/issues/27352.

💬 l0rinc commented on pull request "ci: drop -priority-level from bench in win cross CI":
(https://github.com/bitcoin/bitcoin/pull/32288#issuecomment-2813509164)
Seems related: https://github.com/l0rinc/bitcoin/actions/runs/14519683955/job/40737543737?pr=8
> Running with -sanity-check option, output is being suppressed as benchmark results will be useless.
terminate called after throwing an instance of 'std::filesystem::__cxx11::filesystem_error'
what(): filesystem error: cannot remove all: The process cannot access the file because it is being used by another process [C:\Users\RUNNER~1\AppData\Local\Temp\test_common bitcoin\WalletMigration\af1d7c4
...