✅ fanquake closed a pull request: "log: expand BCLog::LogFlags (categories) to 64 bits"
(https://github.com/bitcoin/bitcoin/pull/26619)

💬 Sjors commented on pull request "Switch hardened derivation marker to h (in normalized descriptors and new wallets)":
(https://github.com/bitcoin/bitcoin/pull/26076#discussion_r1141994246)
I suppose it would be better if legacy wallet RPC calls don't change behavior. I'll look into this (and the other comments above).

💬 ismaelsadeeq commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1141997488)
functions in the codebase have one blank line in between.

💬 josibake commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1141998972)
The goal is to have the code base consistent with our style guide, not consistent with itself. This means any new code should adhere to the style guide and, if possible, old code should be formatted consistent with the style guide if it's already being touched as part of a PR.

💬 ismaelsadeeq commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1142001889)
I understand.

💬 josibake commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1142002452)
This is because a lot of code was written before we introduced a style guide, or because it was missed during review. If something is not covered by the style guide, then we should try to be consistent with the surrounding code. If something _is_ covered by the style guide, we should be consistent with the style guide, even if that is not consistent with the surrounding code. This helps nudge the codebase towards the style guide over time.

💬 ajtowns commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1142002479)
How do you have a valid header without a known height? If you don't know all the ancestors, you can't tell if the `nBits` value is valid, which we check in `ContextualCheckBlockHeader` earlier in this function... (A missing prev block should result in `return Invalid(BLOCK_MISSING_PREV)` prior to that) The only case where we don't do all that is if we're looking at the genesis block header, but we know its height is 0... What am I missing here?

💬 josibake commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1142005645)
style nit: these should be lexicographically sorted

💬 jonatack commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1142007013)
- Good idea @Sjors, will do.

- "None" is internal-only (like Network::NET_MAX for iterating) and IIRC a commit in the severity -level logs parent PR ends up removing it

- "Info" is planned to be logged unconditionally as well, in addition to Warning and Error.

💬 ismaelsadeeq commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1142008651)
Thanks

💬 ismaelsadeeq commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1142009388)
Okay :+1:

⚠️ Sjors opened an issue: "Memory leak in wallet_tests with BDB using address sanitizer"
(https://github.com/bitcoin/bitcoin/issues/27283)
```
src/test/test_bitcoin --run_test=wallet_tests
```

Passed when configured without BDB, fails when configured with BDB:

```sh
./configure 'BDB_LIBS=-L/…' BDB_CFLAGS=-I/…/include CC=clang CXX=clang++ --enable-suppress-external-warnings --disable-asm --with-sanitizers=address --disable-fuzz-binary --without-gui
make -j33 -C src/test
src/test/test_bitcoin --run_test=wallet_tests

…

*** No errors detected

=================================================================
==63742
...

💬 Sjors commented on issue "Memory leak in wallet_tests with BDB using address sanitizer":
(https://github.com/bitcoin/bitcoin/issues/27283#issuecomment-1476101437)
cc @achow101

💬 fanquake commented on issue "Memory leak in wallet_tests with BDB using address sanitizer":
(https://github.com/bitcoin/bitcoin/issues/27283#issuecomment-1476110273)
Doesn't look like you've provided anything more actionable or useful than the last comment in #19034?

💬 josibake commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#issuecomment-1476110683)
ACK https://github.com/bitcoin/bitcoin/commit/9aac917c6e7fae48ceae5fe786c412a583e46d3e

Thanks for incorporating the feedback and addressing the style comments! Looks good.

💬 Sjors commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1142021516)
`AddToBlockIndex` checks if `hashPrevBlock` is already in the index, and if not it won't set `pprev` and `nHeight` on the `CBlockIndex` entry it returns. But I didn't check if that condition can ever be reached from the call in `AcceptBlockHeader`.

💬 fanquake commented on issue "Memory leak in wallet_tests with BDB using address sanitizer":
(https://github.com/bitcoin/bitcoin/issues/27283#issuecomment-1476116230)
Also if you are going to open issues like this, can you provide more information. i.e you don't even tell us what version of the code this happens with. I guess we are meant to assume it's the latest commit in master? What compiler versions did you use? What system are you running on?

💬 MarcoFalke commented on issue "Memory leak in wallet_tests with BDB using address sanitizer":
(https://github.com/bitcoin/bitcoin/issues/27283#issuecomment-1476120851)
>There's currently no suppression file for the address sanitizer

See https://github.com/bitcoin/bitcoin/blob/master/test/sanitizer_suppressions/lsan

💬 fanquake commented on pull request "test: Use consistent assert functions":
(https://github.com/bitcoin/bitcoin/pull/26356#issuecomment-1476122672)
Closing for now. Looks like the user has also disspeared off GitHub.

✅ fanquake closed a pull request: "test: Use consistent assert functions"
(https://github.com/bitcoin/bitcoin/pull/26356)