👍 willcl-ark approved a pull request: "test: Support decoding segwit address in address_to_scriptpubkey()"
(https://github.com/bitcoin/bitcoin/pull/27269)
ACK d17808299

I also tested two of the BIP173 [Test Vectors](https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki#test-vectors):

```python
check_bech32_decode(bytes.fromhex('0020000000c4a5cad46221b2a187905e5266362b99d5e91c6ce24d165dab93e86433'), 1)
check_bech32_decode(bytes.fromhex('00201863143c14c5166804bd19203356da136c985678cd4d27a1b8c6329604903262'), 1)
```

...which also passed. In doing so I noticed the hardcoding of human readable part `hrp` in _address.py_#L196 was li
...

💬 willcl-ark commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1147454419)
I think this is totally fine as is here for now, but in the future we may want to put these all in a list/enum

💬 willcl-ark commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1147486821)
Instead of hardcoding the hrp here, why not have `check_bech32_decode()` take `hrp` as a parameter, then we can add `bc1` test vectors instead of being limited to `tb1`?

💬 willcl-ark commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#discussion_r1147460292)
I may be mistaken but I believe that actually this [_should_](https://peps.python.org/pep-0008/#blank-lines) have 1 line gap, as it's a member function? But its OK, I wouldn't change and invalidate the ACKS you've got already! (I configured my editor to show PEP8 diagnostics so I can view them directly as I'm editing).

🚀 fanquake merged a pull request: "test: Support decoding segwit address in address_to_scriptpubkey()"
(https://github.com/bitcoin/bitcoin/pull/27269)

🚀 fanquake merged a pull request: "ci: Cache more stuff in the ci images: msan, iwyu, pip, sdks"
(https://github.com/bitcoin/bitcoin/pull/27028)

⚠️ MarcoFalke opened an issue: "ci: Cleanup ci/test/01_base_install.sh"
(https://github.com/bitcoin/bitcoin/issues/27321)
### Motivation

Currently `ci/test/01_base_install.sh` has the following issues:

* Two names to alias `bash -c`: `CI_EXEC_ROOT`, and `CI_EXEC`
* The aliases are not needed, as explained in the comment in the file

Thus, they should be removed, or at least de-duplicated. The whole file is already run in bash, so the only place where `bash -c "..."` is needed is in the `apt install` step to properly pass the packages string with spaces.

### Possible solution

* Remove or de-duplicate `CI_EX
...

💬 vasild commented on pull request "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg":
(https://github.com/bitcoin/bitcoin/pull/27257#discussion_r1147543379)
Alas that does not work if the condition includes function calls.

💬 willcl-ark commented on issue "core stops to run with `Failed to read block` error":
(https://github.com/bitcoin/bitcoin/issues/27142#issuecomment-1482781323)
Do you think it's something you will try again in the future? It would be good to get reproduction steps if possible...

📝 jonatack opened a pull request: "move-only: IsDeprecatedRPCEnabled to rpc/util to fix CI builds when called from wallet"
(https://github.com/bitcoin/bitcoin/pull/27322)
While deprecating the remaining wallet RPC "warning" fields in #27279, the CI reported `undefined reference to IsDeprecatedRPCEnabled in libbitcoin_wallet` compiler errors when this function was called from wallet RPCs.

Fix this by moving the function from `rpc/server` to `rpc/util`, which by looking at `doc/design/librairies.md` and `src/Makefile.am` looks like the right place for it. Happy to change approach per reviewer feedback.

Cherry-picked from #27279, where the relevant CI is now
...

💬 jonatack commented on issue "wallet_create_tx.py "Not solvable pre-selected input" exception":
(https://github.com/bitcoin/bitcoin/issues/27316#issuecomment-1482789635)
Same error reported by CI in https://cirrus-ci.com/task/5776249860128768?logs=functional_tests#L2682.

```
[default_wallet] AddToWallet 87b8816da11c3e05844802cf2ca4ac0fdcd605d0b53ae046f470264aec116878
test 2023-03-23T23:35:57.593000Z TestFramework (ERROR): JSONRPC error
Traceback (most recent call last):
File "C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\test\functional\test_framework\test_fra
...

💬 virtu commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1147575925)
I can imagine some use cases where having consistent netgroup identifiers would be advantageous. Unless `NetGroupManager::GetGroup()` adds too much overhead, they seem preferable.

💬 furszy commented on issue "wallet_create_tx.py "Not solvable pre-selected input" exception":
(https://github.com/bitcoin/bitcoin/issues/27316#issuecomment-1482795409)
@jonatack https://github.com/bitcoin/bitcoin/pull/27318

💬 MarcoFalke commented on pull request "move-only: IsDeprecatedRPCEnabled to rpc/util to fix CI builds when called from wallet":
(https://github.com/bitcoin/bitcoin/pull/27322#issuecomment-1482807752)
lgtm

Maybe explain that this moves it from `node` to `common`?

📝 MarcoFalke opened a pull request: "ci: Add missing libclang-rt-dev package"
(https://github.com/bitcoin/bitcoin/pull/27323)
This is required, due to `--no-install-recommends` in `ci/test/01_base_install.sh`, I presume.

If not added, this will cause issues if the CI image is bumped, for example, to Ubuntu Lunar.

Asan Lunar:

```
configure:22272: clang++ -std=c++20 -o conftest -g -O2 -DARENA_DEBUG -DDEBUG_LOCKORDER -fsanitize=address,integer,undefined conftest.cpp >&5
/usr/bin/ld: cannot find /usr/lib/llvm-15/lib/clang/15.0.7/lib/linux/libclang_rt.asan_static-x86_64.a: No such file or directory
/usr/bin/l
...

💬 virtu commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#issuecomment-1482823973)
ACK [6de4fc7](https://github.com/bitcoin/bitcoin/commit/6de4fc73c935baa2cc6f54dd57c8e57a5df3c7ae)

Code reviewed tracepoints, tests, and demo script. Tracepoint placement looks good, so do tests. Successfully ran functional tests and the demo script.

Some points:
- Any reason why only `inbound_connection` and `outbound_connection` explicitly cast all arguments in the demo script?
- I think some of the format specifiers in the demo script are lightly off, leading to outputs with negative n
...

💬 jonatack commented on pull request "move-only: IsDeprecatedRPCEnabled to rpc/util to fix CI builds when called from wallet":
(https://github.com/bitcoin/bitcoin/pull/27322#issuecomment-1482827329)
Thanks @MarcoFalke, good idea, updated the PR description and will update the commit message shortly.

💬 MarcoFalke commented on pull request "ci: Add missing libclang-rt-dev package":
(https://github.com/bitcoin/bitcoin/pull/27323#issuecomment-1482831967)
Ah sorry. This requires bumping the image at the same time. Closing.

✅ MarcoFalke closed a pull request: "ci: Add missing libclang-rt-dev package"
(https://github.com/bitcoin/bitcoin/pull/27323)

💬 jnewbery commented on pull request "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg":
(https://github.com/bitcoin/bitcoin/pull/27257#discussion_r1147619018)
> Alas that does not work if the condition includes function calls.

Why not? From the GDB docs:

> Break conditions can have side effects, and may even call functions in your program.