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

💬 Sjors commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1142032854)
The only other call site for `AddToBlockIndex()` is `LoadGenesisBlock()`, so I guess that's the only reason for the (generic looking) check of `hashPrevBlock`.

💬 MarcoFalke commented on issue "test: `wallet_importdescriptors.py --descriptors` failure under `--valgrind` ":
(https://github.com/bitcoin/bitcoin/issues/27282#issuecomment-1476131126)
What is the combined log?

💬 MarcoFalke commented on pull request "RPC: Fix fund transaction crash when at 0-value, 0-fee":
(https://github.com/bitcoin/bitcoin/pull/27271#issuecomment-1476131979)
Does this need backport?

💬 MarcoFalke commented on issue "test: `interface_bitcoin_cli.py --descriptors` failure under `--valgrind`":
(https://github.com/bitcoin/bitcoin/issues/27281#issuecomment-1476133389)
What is the combined log?

💬 fanquake commented on pull request "RPC: Fix fund transaction crash when at 0-value, 0-fee":
(https://github.com/bitcoin/bitcoin/pull/27271#issuecomment-1476134405)
> Does this need backport?

I think it's not-super-required but could be nice if we do.

💬 jamesob commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1142036850)
In most logging systems I've used elsewhere, (i) INFO is the default loglevel, and (ii) the level is configurable via flag/envvar. I guess what I'll do here is use `Warning` and leave a TODO.

💬 jamesob commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1142037129)
Good point! Thanks.

💬 Sjors commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1142037513)
Indeed this code is pretty ancient (and could use a comment): https://github.com/bitcoin/bitcoin/blame/f3ae51dcced8a16175426051ce888130cc2493af/src/main.cpp#L2056-L2063

💬 Sjors commented on issue "Memory leak in wallet_tests with BDB using address sanitizer":
(https://github.com/bitcoin/bitcoin/issues/27283#issuecomment-1476147317)
@fanquake that was 65840 bytes, this one is 320 bytes, so I assume it's a different issue?

I was going to add more details if anyone couldn't reproduce it, but here we go:

Found it while testing a PR, but reproduced on master @ 40e1c4d4024b8ad35f2511b2e10bf80c5531dfde.

```
Ubuntu clang version 15.0.6
Target: x86_64-pc-linux-gnu

Ubuntu 22.10 (GNU/Linux 6.0.6-060006-generic x86_64)
```

@MarcoFalke I'll see if I can get that too work.