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

💬 TheCharlatan commented on pull request "doc: Add deps install notes for multiprocess":
(https://github.com/bitcoin/bitcoin/pull/32293#issuecomment-2813519306)
> Thanks for adding this. Somehow I thought the capnproto dependencies were already listed, but that change is sitting in a commit in https://github.com/bitcoin/bitcoin/pull/10102 (https://github.com/bitcoin/bitcoin/commit/bbcbe85cef21049b5f321265264e2c4fe394d209).

Heh, I was looking around if either Sjors or you already did this in one of the open PRs after forgetting to install capnproto on a new machine, but forgot the check the original PR...

Updated ff4ea070aa4b1ed1ebb6dcf62e01c9dd573
...

💬 Sjors commented on pull request "doc: Add deps install notes for multiprocess":
(https://github.com/bitcoin/bitcoin/pull/32293#issuecomment-2813533741)
re-ACK 5ae18914805f0a78dde85cfe2a7876c4e52c0fe7

👍 ryanofsky approved a pull request: "doc: Add deps install notes for multiprocess"
(https://github.com/bitcoin/bitcoin/pull/32293#pullrequestreview-2776404590)
Code review ACK 5ae18914805f0a78dde85cfe2a7876c4e52c0fe7. Thanks for the change and updates!