💬 c0deright commented on issue "Old wallet.dat: Error reading wallet database: keymeta found with unexpected path / All keys read correctly, but transaction data or address metadata may be missing or incorrect":
(https://github.com/bitcoin/bitcoin/issues/29109#issuecomment-1862813350)
I did `dumpwallet` and the file created looks like this:

```
# Wallet dump created by Bitcoin Core v26.0.0
# * Created on 2023-12-19T13:47:56Z
# * Best block at time of backup was 821931 (00000000000000000003241b21de4d2d4d3505c2941fba3cccec82f1dbdbce4b),
# mined on 2023-12-19T13:46:59Z

# extended private masterkey: xprv***********************************************************************************************************

L**************************************************n 197
...

💬 michaelfolkson commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1862817775)
Concept ACK. I always saw this as inevitable as it reduces mempool reasoning complexity and the supposed merits of zero conf discussion has been done to death.

💬 hebasto commented on issue "Unnecessary symbols being exported in `libbitcoinconsensus.so`":
(https://github.com/bitcoin/bitcoin/issues/26698#issuecomment-1862877305)
The underlying problem is described in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36022, and it was not considered as a bug.

I thing that moving code that uses C++ template functions/classes out from the `bitcoinconsensus.{h,cpp}` compilation unit, which were suggested in #24994, is a step in the right direction.

💬 hebasto commented on pull request "build: Build `libbitcoinconsensus` from its own convenience library":
(https://github.com/bitcoin/bitcoin/pull/24994#issuecomment-1862914332)
> Concept NACK. In general, I don't like this approach of refactoring code, and adding libraries, to workaround issues with build tooling, especially when its a problem that only exists for a single platform (let alone the DEBUG build, and ). I think this should just be closed in light of #27146 and the migration towards CMake.

I disagree with this NACK.

@fanquake's push back is mostly focused on the Windows-related issue mentioned in the PR description. However, none alternatives are sug
...

💬 fanquake commented on pull request "build: Build `libbitcoinconsensus` from its own convenience library":
(https://github.com/bitcoin/bitcoin/pull/24994#issuecomment-1862925103)
> his code is not fine.

How does the above indicate a problem with our code? Wouldn't this point to the linker being the issue? Does the same happen with `lld`? If not, then it's a problem with `ld`/`libstdc++`, not our code?

💬 hebasto commented on pull request "build: Build `libbitcoinconsensus` from its own convenience library":
(https://github.com/bitcoin/bitcoin/pull/24994#issuecomment-1862929878)
> Wouldn't this point to the linker being the issue?

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36022 -- `Status: RESOLVED INVALID`

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 2)":
(https://github.com/bitcoin/bitcoin/pull/28983#issuecomment-1862981292)
I moved chunking into `noise.h`, since the maximum message length of 65 KB is part of the protocol. I also dropped `ProcessMsg` handler in favor of handling handshake vs. regular transport more directly. Finally `SendBuf` is now called by `EncryptAndSendMessage` rather than by each message call site.

Main changes:
* 945838fca264c8469e24d629a4552741dbbd4dda -> f08a10a892fc467dbf6037e0c73227822e9f698a
* fc0aa335dd3f3beba383a42f19f73822b4ad2d34 -> 028c286d9a38343d8240c62a67207681d024a326
*
...

💬 EthnTuttle commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1862983648)
Ack.

📝 maflcko opened a pull request: "util: Faster std::byte (pre)vector (un)serialize"
(https://github.com/bitcoin/bitcoin/pull/29114)
Currently, large vectors of `std::byte` are (un)serialized byte-by-byte, which is slow. Fix this, by enabling the already existing optimization for them.

On my system this gives a 10x speedup for `./src/bench/bench_bitcoin --filter=PrevectorDeserializeTrivial`, when `std::byte` are used:

```diff
diff --git a/src/bench/prevector.cpp b/src/bench/prevector.cpp
index 2524e215e4..76b16bc34e 100644
--- a/src/bench/prevector.cpp
+++ b/src/bench/prevector.cpp
@@ -17,7 +17,7 @@ struct nontrivi
...

💬 maflcko commented on pull request "util: Faster std::byte (pre)vector (un)serialize":
(https://github.com/bitcoin/bitcoin/pull/29114#issuecomment-1862986532)
Draft while it is based on https://github.com/bitcoin/bitcoin/pull/29056

👍 TheCharlatan approved a pull request: "refactor: Print verbose serialize compiler error messages"
(https://github.com/bitcoin/bitcoin/pull/29056#pullrequestreview-1789197585)
Re-ACK fae526345de539ab8f9b80100f6dfbe8e1d3284b

📝 ismaelsadeeq opened a pull request: "[doc]: add doxygen comment describing what `CheckPackageLimits` returns"
(https://github.com/bitcoin/bitcoin/pull/29115)
This PR added a doxygen comment on `CheckPackageLimits` describing what the method returns.

Fixes https://github.com/bitcoin/bitcoin/pull/28863#discussion_r1429805433

💬 ismaelsadeeq commented on pull request "wallet, mempool: propagete `checkChainLimits` error message to wallet":
(https://github.com/bitcoin/bitcoin/pull/28863#discussion_r1431646003)
This is fixed in https://github.com/bitcoin/bitcoin/pull/29115

💬 jamesob commented on pull request "util: Faster std::byte (pre)vector (un)serialize":
(https://github.com/bitcoin/bitcoin/pull/29114#issuecomment-1863092993)
Concept ACK. I better blow the dust off of bitcoinperf...

🤔 ryanofsky reviewed a pull request: "doc: Add multiprocess design doc"
(https://github.com/bitcoin/bitcoin/pull/28978#pullrequestreview-1788797844)
re: https://github.com/bitcoin/bitcoin/pull/28978#pullrequestreview-1777912181

> Approach ACK. I'm still working on my understanding of multiprocess but this document was very helpful already in its current state, no major suggestions at this point.

Thank you, applied your suggestions, and improved the example flow section based on your feedback.

re: https://github.com/bitcoin/bitcoin/pull/28978#pullrequestreview-1782842554

> ACK [837c53d](https://github.com/bitcoin/bitcoin/commit/83
...

💬 ryanofsky commented on pull request "doc: Add multiprocess design doc":
(https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1431361667)
re: https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1424253642

> Steps 2 and 3 highlight what's handled by generated code, for consistency that might be helpful in steps 4&5 too?

Thanks, added more details to this section so it now mentions the generated client and server classes and references the generated code consistently.

💬 ryanofsky commented on pull request "doc: Add multiprocess design doc":
(https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1431361020)
re: https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1424248273

> typo nit

Thanks, fixed

💬 ryanofsky commented on pull request "doc: Add multiprocess design doc":
(https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1431360695)
re: https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1427363274

> nit

Useful clarification, added

💬 ryanofsky commented on pull request "doc: Add multiprocess design doc":
(https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1431361157)
re: https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1428164608

>

Thanks, fixed

💬 ryanofsky commented on pull request "doc: Add multiprocess design doc":
(https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1431361546)
re: https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1424249827

> nit: rogue backtick

Thanks, fixed