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

💬 brandonblack commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1863182118)
ACK
(Re-upping)

💬 ryanofsky commented on issue "Unnecessary symbols being exported in `libbitcoinconsensus.so`":
(https://github.com/bitcoin/bitcoin/issues/26698#issuecomment-1863192161)
From the linked issue https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36022#c4:

> For C++, types that are to be used across shared libraries have to be visible

Seems to suggest that it is a good thing for these symbols to be exported so the library and its callers use the same symbols, particularly `type_info` symbols, and I guess generally so symbols in shared libraries have the same visibility as symbols in static libraries. So linking dynamically doesn't turn shared symbols into hidden sym
...

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 2)":
(https://github.com/bitcoin/bitcoin/pull/28983#issuecomment-1863197392)
@Fi3 found the issue, fixed in a7f08e74428c4e5da62365d686e5c51c5afc959d

```patch
--- a/src/common/sv2_noise.cpp
+++ b/src/common/sv2_noise.cpp
@@ -56,11 +56,11 @@ void Sv2CipherState::EncryptMessage(Span<std::byte> input, Span<std::byte> outpu
for (size_t i = 0; i < num_chunks; ++i) {
size_t chunk_start = i * max_chunk_size;
size_t chunk_end = std::min(chunk_start + max_chunk_size, input.size());
size_t chunk_size = chunk_end - chunk_start;
con
...

💬 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-1863241394)
Oh, now that you told me where to look for there are indeed two strange looking key paths.

All but two are in the format `m/a'/b'/c'`, e.g. `m/0'/1'/289'` or `m/0'/1'/4'`. Most are distinct but eleven key paths are found twice in the dump. And then there are two with a very odd path: `m/0'/1'/299'/0'/1'/300'` and `m/0'/0'/299'/0'/0'/300'`.

🤔 mzumsande reviewed a pull request: "logging: Simplify API for level based logging"
(https://github.com/bitcoin/bitcoin/pull/28318#pullrequestreview-1784982210)
Concept ACK

I like the simplified interface, having to write something like `LogPrintLevel(BCLog::NET, BCLog::Level::Error, ...)` for each log would be annoying, plus I like that there wouldn't be a widely-used function/macro that could log both unconditionally and conditionally depending on parameters, which is a bit of a footgun.

💬 mzumsande commented on pull request "logging: Simplify API for level based logging":
(https://github.com/bitcoin/bitcoin/pull/28318#discussion_r1430739019)
As long as they are used all over the place, I think that `LogPrintf()` and `LogPrint()` should be mentioned to avoid confusion (could explain that they are a legacy options no longer recommended for use). Especially since the current state could last a while, who knows how fast future PRs updating existing log entries with dozen of conflicts would get merged.

💬 mzumsande commented on pull request "logging: Simplify API for level based logging":
(https://github.com/bitcoin/bitcoin/pull/28318#discussion_r1428526172)
Just noting that this doesn't change the the one existing use of `BCLog::Level::Info` in `httpserver` from non-default to default because `InitHTTPServer()` just disables `libevent` logging callbacks entirely unless `BCLog::LIBEVENT` isn't enabled.

⚠️ linkified opened an issue: "bitcoind ubuntu generating fake addresses ?"
(https://github.com/bitcoin/bitcoin/issues/29116)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo

- [X] I still think this issue should be opened here

### Report

installed bitcoind on ubuntu im using rpc and when i generate a wallet either legacy and or regular i get fake wallets they dont exist
when i tried to sent crypto to one it said its not a real btc wallet

tb1qr2eurhm32w5na9q42k9qwsga5793rxty2rsw78

tb1qmm4w350w0xw0h3tnamsrk0ujzvzwf5q6ve03e8


root@rdp:~# bitcoin-cli getnew
...

💬 maflcko commented on issue "bitcoind ubuntu generating fake addresses ?":
(https://github.com/bitcoin/bitcoin/issues/29116#issuecomment-1863303086)
Those look like testnet addresses. If you want to start with a different test chain or the main chain, you'll have to select the correct chain.

🤔 Zero-1729 reviewed a pull request: "[doc]: add doxygen comment describing what `CheckPackageLimits` returns"
(https://github.com/bitcoin/bitcoin/pull/29115#pullrequestreview-1789573880)
utACK 19bb65bf255df0f876e37de90fb8c4c6229cdf52

💬 mzumsande commented on pull request "net: Attempts to connect to all resolved addresses on `addnode`":
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1431825997)
There should be a different name than `addrConnect` (either here or above), shadowing should be avoided.