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

💬 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-1863413296)
> Most are distinct but eleven key paths are found twice in the dump

Since there is an `inactivehdseed`, that means that the seed has been rotated, probably by encrypting the wallet. So these duplicated paths are expected. One of the keys is derived from the inactive seed, and the other from the current active seed.

> 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'`.

This lines up with the errors in the debug.log.

These derivation p
...

💬 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-1863432854)
> Are there any keys with any of the derivation paths `m/0'/1'/299'`, `m/0'/1'/300'`, `m/0'/0'/299'`, or `m/0'/0'/300'`?

Only these two:

```
reserve=1 [..] hdkeypath=m/0'/0'/299'
reserve=1 [..] hdkeypath=m/0'/0'/300'
```

> If you are able to, try loading the wallet in 0.17 or earlier and then doing `dumpwallet`. If you lookup these two addresses (with the weird derivation paths) in the dump, what derivation paths do they have?

I did as you described and `dumpwallet` with 0.17.0 gi
...

💬 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-1863449058)
`bitcoin-wallet` doesn't create any file:

```
% bitcoin-wallet -datadir=. -dumpfile=026.dump -wallet=wallet.dat dump
Error reading wallet.dat! All keys read correctly, but transaction data or address book entries might be missing or incorrect.
```

🤔 furszy reviewed a pull request: "p2p: adaptive connections services flags"
(https://github.com/bitcoin/bitcoin/pull/28170#pullrequestreview-1789712050)
PR reworked based on the feedback. Thanks @vasild for the rounds of review.

The functionality is no longer tied to the last tip update time; is now linked to the actual tip time, which is now cached by ´PeerManager´ to prevent undesirable ´cs_main´ locks. This implies simpler code since we are no longer modifying ´TipMayBeStale()´ nor the ´m_initial_sync_finished´ field.

I still believe that we should implement some of the changes made earlier, such as the ´PeerManager´ IBD flag modificati
...

💬 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-1863491818)
> `bitcoin-wallet` doesn't create any file:

Oh, that's a bug, Should be simple to fix. In any case, I don't think that dump will be needed since the `dumpwallet` output should be enough to have an idea of what was stored.

***

The `dumpwallet` file has a xprv at the top. Can you try deriving the 4 keys at the derivation paths I mentioned above and see which match the lines with the weird derivation paths? I'm pretty sure the wallet just stored the wrong derivation path somehow.