💬 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

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

> nit: typo

Thanks, fixed

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

Thanks, fixed

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

> Maybe mention that his is based on code that is not merged yet, for example `chain.capnp` is linked below but it doesn't exist on master yet.

Oops, this is not good. I changed the link to point to the branch version for now since I think seeing the interface definition can be helpful.

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

> nit suggestions:
>
>* 'central to the bitcoin network' isn't really relevant to this doc
>
>* I hear people complain that big changes are always a security risk in the short term all the time

Yes, definitely agree and took the suggestion.

💬 maflcko commented on pull request "util: Faster std::byte (pre)vector (un)serialize":
(https://github.com/bitcoin/bitcoin/pull/29114#issuecomment-1863110548)
Benchmarks are also done by https://corecheck.dev/bitcoin/bitcoin/pulls/29114 :)

Though, to test this, one would have to manually apply the diff either way, so local running is probably the easiest.

(I don't think `std::vector<std::byte>` serialization is used anywhere, where performance matters or is measured)

👍 ismaelsadeeq approved a pull request: "Add multiplication operator to CFeeRate"
(https://github.com/bitcoin/bitcoin/pull/29037#pullrequestreview-1789316545)
ACK 1757452cc55a6dacc62e4258043ee4d711fd281a

💬 ismaelsadeeq commented on pull request "Add multiplication operator to CFeeRate":
(https://github.com/bitcoin/bitcoin/pull/29037#discussion_r1431686247)
nit: will be nice to squashed this to the first commit

💬 achow101 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-1863129994)
Do you see any `hdkeypath` that doesn't follow the pattern `m/a'/b'/c'`?

💬 ryanofsky commented on pull request "build: Build `libbitcoinconsensus` from its own convenience library":
(https://github.com/bitcoin/bitcoin/pull/24994#issuecomment-1863153024)
> cc @ryanofsky

I'd like to help with this, but I just don't understand most of the discussion so far. I see that this fixes some kind of a bug on windows and is supposed to have other benefits mentioned in the PR description, but I don't see a clear description of the bug anywhere. Maybe someone can open an issue with a description of the bug and why it happens? The issue could links to whatever PRs have been proposed to fix it.