👍 stickies-v approved a pull request: "[23.x] rpc: Fix invalid bech32 handling"
(https://github.com/bitcoin/bitcoin/pull/27756#pullrequestreview-1446142241)
ACK d98770720885cdce1bbe2109a6d214c63fa1814a

Verified that the commit is identical to c2e9214effe9abecae6f81cb10158f9661065da3 (#27755). Change lgtm.

<details>
<summary><code>diff <(git show c2e9214effe9abecae6f81cb10158f9661065da3) <(git show d98770720885cdce1bbe2109a6d214c63fa1814a)</code></summary>

```
1c1
< commit c2e9214effe9abecae6f81cb10158f9661065da3
---
> commit d98770720885cdce1bbe2109a6d214c63fa1814a
336c336
< index b9233ef2c..9ade9e9ea 100755
---
> index b0f24e3b9..
...

💬 fanquake commented on pull request "doc: Add Python install notes to build-osx.md.":
(https://github.com/bitcoin/bitcoin/pull/27728#issuecomment-1564333068)
> Still Concept Nack?

Yes. Developers are free to use pyenv if they want, but it's not a requirement. I'd also say that most developers are likely using a version of Python which is actually much newer, than our oldest supported version.

Also, note that the `ds_store` and `mac_alias` packages will be dropped entirely after #27099, so I think there is even less need to make these changes at this point.

💬 MarcoFalke commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564348866)
Well, it isn't building the image, just printing `CACHED`. I'd need the output from when it was built of the step that ran base_install:


```
CACHED [4/4] RUN ["bash", "-c", "cd /ci_base_install/ && set -o errexit && source ./ci/test/00_setup_env.sh && ./ci/test/01_base_install.sh"]

💬 MarcoFalke commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564349776)
> the rest above prints my env, which I don't want to paste here

If you can reproduce on a fresh install of your OS, that'd help too

💬 hebasto commented on pull request "refactor: Replace `std::optional<bilingual_str>` with `util::Result`":
(https://github.com/bitcoin/bitcoin/pull/25977#issuecomment-1564350880)
> Looks like there is a clang bug. Could be fixed by either bumping it again, see #27682, but I am not sure how to do that for macOS. Or by replacing `return addrman;` with `return {std::move(addrman)};`, or something like that (temporarily).

Although GCC complains:
```
$ ./configure CONFIG_SITE=$PWD/depends/arm-linux-gnueabihf/share/config.site --enable-suppress-external-warnings CXXFLAGS="-Wextra -Wno-psabi"
$ make > /dev/null
addrdb.cpp: In function ‘util::Result<std::unique_ptr<AddrM
...

💬 dergoegge commented on pull request "fuzz: Change LIMIT_TO_MESSAGE_TYPE from a compile-time to a run-time setting":
(https://github.com/bitcoin/bitcoin/pull/27766#discussion_r1206750051)
I think we can just get rid of the `FUZZ_TARGET_MSG` macro including `MsgTypes()` and use `getAllNetMessageTypes` directly?

🤔 dergoegge reviewed a pull request: "fuzz: Change LIMIT_TO_MESSAGE_TYPE from a compile-time to a run-time setting"
(https://github.com/bitcoin/bitcoin/pull/27766#pullrequestreview-1446186895)
Concept ACK

💬 Crypt-iQ commented on issue "bitcoind hangs waiting for `g_requests.empty()`":
(https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1564380223)
@willcl-ark can you find what libevent version you're using?

💬 TheCharlatan commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564383177)
Re https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564349776

> If you can reproduce on a fresh install of your OS, that'd help too

Not a fresh install, but another OS where I have not run this before:
<p>
<details>
MAKEJOBS="-j12" FILE_ENV="./ci/test/00_setup_env_native_tidy.sh" ./ci/test_run_all.sh
Setting specific values in env
Fallback to default values in env (if not yet set)
...
CONTAINER_NAME=ci_native_tidy
DEPENDS_DIR=/home/drgrid/bitcoin/depends
TERM=xterm-256
...

💬 MarcoFalke commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#issuecomment-1564406303)
`void` can be removed from OP?

💬 hebasto commented on pull request "refactor: Make `CCheckQueue` RAII-styled":
(https://github.com/bitcoin/bitcoin/pull/26762#issuecomment-1564429422)
Rebased 52129e335cbc68ac5d863f283f7d1a328ce79581 -> 2076d846cc917cbafe61937a99b7867067011341 ([pr26762.08](https://github.com/hebasto/bitcoin/commits/pr26762.08) -> [pr26762.09](https://github.com/hebasto/bitcoin/commits/pr26762.09)) due to the conflict with #25977.

🚀 fanquake merged a pull request: "test: Move test_chain_listunspent wallet check from mempool_packages to wallet_basic"
(https://github.com/bitcoin/bitcoin/pull/27735)

💬 stickies-v commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#issuecomment-1564441571)
@brunoerg are you going to address nits or leave as is? I'd really like to get this merged asap to prevent further rebase conflicts. Happy to quickly re-ack nits too, though.

💬 1ma commented on pull request "Allow accepting non-standard transactions on mainnet":
(https://github.com/bitcoin/bitcoin/pull/27578#issuecomment-1564474817)
Even then they won't go further than that, unless these peers are miners. This option would be only somewhat relevant if you run a solo mining node or a pool coordinator.

💬 furszy commented on pull request "Return EXIT_FAILURE on post-init fatal errors":
(https://github.com/bitcoin/bitcoin/pull/27708#discussion_r1206869341)
The reason behind that is https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1199624849.
But.. could not expose it and move the caller to use `AbortNode()` instead. Same as we do with the external block import failures.

💬 joostjager commented on pull request "Allow accepting non-standard transactions on mainnet":
(https://github.com/bitcoin/bitcoin/pull/27578#issuecomment-1564482472)
Agreed that the option isn't the most relevant in typical scenarios. But if a restriction can be lifted safely and benefit a specific group of users, why not?

💬 furszy commented on pull request "Return EXIT_FAILURE on post-init fatal errors":
(https://github.com/bitcoin/bitcoin/pull/27708#discussion_r1206881682)
Still, in order to do that in the cleanest possible way, will need to refactor `shutdown.cpp`, so `AbortNode` is placed after the static `StartErrorShutdown` function. Which I'm not so sure that worth it due the conflicts that could cause with your PRs. Thoughts?

💬 pinheadmz commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#issuecomment-1564497329)
@furszy awwwwwesome thank you for catching that. Ok, passing test is now at https://github.com/pinheadmz/bitcoin/commit/f1238700a1f3d3e88b7f20bc51f0088783d53595

💬 pinheadmz commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1205652539)
I'm curious if `.load()` is necessary for the atomic value? Is this the correct reason? https://stackoverflow.com/a/44288045/1653320

👍 pinheadmz approved a pull request: "wallet: improve IBD sync time by skipping block scanning prior birth time"
(https://github.com/bitcoin/bitcoin/pull/27469#pullrequestreview-1444104729)
ACK 82bb7831fa6052620998c7eef47e48ed594248a8

Reviewed code and tested locally, wrote a test. I am running this branch side-by-side with master on a VPS and will report who wins the race in a day or so ;-)

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK 82bb7831fa6052620998c7eef47e48ed594248a8
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmRwxNIACgkQ5+KYS2KJ
yTq5nhAA3dJDzRO4VmDUA8n9YO4RtpcQBHT/t9NOs
...