👍 dergoegge approved a pull request: "test: tx orphan handling"
(https://github.com/bitcoin/bitcoin/pull/28199#pullrequestreview-1571484607)
Code review ACK abe8536192c9f2cd6ba9d0e083f23dec4d20841f

💬 fanquake commented on pull request "BIP324 ciphersuite":
(https://github.com/bitcoin/bitcoin/pull/28008#issuecomment-1672921810)
We can track and deal with all remaining comments in one of the followup PRs.

🚀 fanquake merged a pull request: "BIP324 ciphersuite"
(https://github.com/bitcoin/bitcoin/pull/28008)

💬 dergoegge commented on pull request "p2p: outbound network diversity improvements":
(https://github.com/bitcoin/bitcoin/pull/28248#issuecomment-1672930044)
The title "p2p: outbound network diversity improvements" makes it sound like you are improving on the logic for making outbound connections but really you are refactoring and adding additional logging. Would you mind making the title more accurate?

> Please see the commit messages for details.

The PR description is added to the merge commit, so in my opinion it makes sense to have at least a summary of what the PR does in the description.

From our [docs](https://github.com/bitcoin/bitco
...

🤔 dergoegge reviewed a pull request: "fuzz: fix a couple incorrect assertions in the `coins_view` target"
(https://github.com/bitcoin/bitcoin/pull/28215#pullrequestreview-1571557702)
lgtm (only one small suggestion inline)

> See commits messages for details.

I'd prefer if you summarize what the PR does in the description. Mostly the fact that these assertions are only correct if we use the default `CCoinsView` but incorrect if we were using an actual backend impl.

💬 dergoegge commented on pull request "fuzz: fix a couple incorrect assertions in the `coins_view` target":
(https://github.com/bitcoin/bitcoin/pull/28215#discussion_r1289900910)
```suggestion
// Note we can't assert that `coin_using_get_coin == coin_using_backend_get_coin` because the coin in
// the cache may have been modified but not yet flushed.
```

💬 glozow commented on pull request "policy: Ephemeral anchors":
(https://github.com/bitcoin/bitcoin/pull/26403#issuecomment-1672959244)
> @instagibbs pushed 1 commit.
> [faeed68](https://github.com/bitcoin/bitcoin/commit/faeed687e5cde5e32750d93818dd1d4add837f24) nice crash bro

hacked a quick-ish fix on top of this branch [here](https://github.com/glozow/bitcoin/tree/2023-08-10-fix-crash-bro). As discussed offline, the general problem is with calling `TrimToSize` in the middle of package validation (meaning something can go from already-in-mempool to evicted, or just-submitted to evicted) so a coin may be cached in `m_view
...

👍 stickies-v approved a pull request: "blockstorage: Return on fatal flush errors"
(https://github.com/bitcoin/bitcoin/pull/27866#pullrequestreview-1570159243)
ACK 7721ab9139013d70ef0f058754fabc5aaeda2246

The changes in `blockstorage.cpp` and `blockstorage.h` are strict improvements, increasing visibility in flushing failures. I found the change in `validation.cpp` much more difficult to review, given the wide variety of ways in which `FlushStateToDisk()` is used. I've found that:
- it improves behaviour in some ways. For example, if `FlushBlockFile()` fails and the `WriteBlockIndexDB()` call still succeeds, we can end up in a situation where we ex
...

💬 stickies-v commented on pull request "blockstorage: Return on fatal flush errors":
(https://github.com/bitcoin/bitcoin/pull/27866#discussion_r1288920603)
nit: can be declared closer to where it's used

💬 stickies-v commented on pull request "blockstorage: Return on fatal flush errors":
(https://github.com/bitcoin/bitcoin/pull/27866#discussion_r1288925171)
nit: long line

💬 hebasto commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#issuecomment-1672973129)
> Anything left to do here, for a required CI-only pull request with review?

I believe this PR is ready to merge unless @fanquake has more comment regarding its description.

However, it requires adjusting "Actions permissions" and "Workflow permissions" in the repo in a similar way as it was done in https://github.com/bitcoin-core/secp256k1/pull/1389.

@achow101 Would you mind taking care of that, please?

💬 fanquake commented on pull request "doc: use llvm-config for bitcoin-tidy example":
(https://github.com/bitcoin/bitcoin/pull/28245#issuecomment-1672982779)
Simplifed this further, given we can actually just ask for `llvm-config --cmakedir`.

⚠️ Sjors opened an issue: "Raise maximum -dbcache setting"
(https://github.com/bitcoin/bitcoin/issues/28249)
### Please describe the feature you'd like to see added.

`nMaxDbCache` was set to 16 GB back in 2015: https://github.com/bitcoin/bitcoin/commit/b3ed4236beb7f68e1720ceb3da15e0c3682ef629#diff-d102b6032635ce90158c1e6e614f03b50e4449aa46ce23370da5387a658342fd

The UTXO set has been growing significantly over the past few months so we should probably raise it.

### Is your feature related to a problem, if so please describe it.

Doing IBD without flushing dbcache is faster. That said, I'm not sure
...

💬 Sjors commented on issue "Raise maximum -dbcache setting":
(https://github.com/bitcoin/bitcoin/issues/28249#issuecomment-1672986348)
cc @sipa

💬 theStack commented on pull request "test: locked_wallet, skip default fee estimation":
(https://github.com/bitcoin/bitcoin/pull/28232#discussion_r1289938015)
nit: could use the `get_fee` helper (from test_framework.util) for not having to bother with manual fee-rate unit coversion and fee calculation:
```suggestion
value = inputs[0]["amount"] - get_fee(expected_tx_size, self.min_relay_tx_fee)
```

👍 pinheadmz approved a pull request: "doc: Improve documentation of rpcallowip"
(https://github.com/bitcoin/bitcoin/pull/27480#pullrequestreview-1571626541)
ACK c8e066461b54d745b85411035fcc00a1a4044d76

reviewed code changes, built locally. tested the option with various (in)valid parameters to confirm the docs match the behavior

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

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

ACK c8e066461b54d745b85411035fcc00a1a4044d76
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmTUwzsACgkQ5+KYS2KJ
yTpDbhAAzRSmfx29YKWZDQ4gY4bCIKF4/PDAwX4DTo3hh7mOpKai7eKx3TPl2RAd
oy42t0
...

⚠️ Vasu-08 opened an issue: "RPC `createmultisig` outputs a `sh(addr(...))` descriptor when address_type field is "p2sh-segwit""
(https://github.com/bitcoin/bitcoin/issues/28250)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

While executing rpc createmultisig it outputs a addr descriptor wrapped around sh. Addr descriptor is supposed to be a top level descriptor. Please refer to this gist [https://gist.github.com/Vasu-08/57c1ff479baf2e70e7e2195d31575651](url) for further details.

### Expected behaviour

Not sure.

### Steps to reproduce

![image](https://github.com/bitcoin/bitcoin/assets/107955853/a5c3cb23-
...

💬 TheCharlatan commented on pull request "blockstorage: Drop legacy -txindex check":
(https://github.com/bitcoin/bitcoin/pull/28195#issuecomment-1673110008)
ACK fae405556d56f6f13ce57f69a06b9ec1e825422b, though I lack historical context to really judge the second commit fa8685597e7302fc136f21b6dd3a4b187fa8e251.

I was curious if it might be possible to easily make one of the `CBlockIndex*` in `BlockTreeDB` a `CBlockIndex&`, but the change became a bit sprawling.

👍 TheCharlatan approved a pull request: "doc: use llvm-config for bitcoin-tidy example"
(https://github.com/bitcoin/bitcoin/pull/28245#pullrequestreview-1571755564)
Nice, Re-ACK d82bb90a5b9dc1fd48b10514bdcd8f425aced256

💬 furszy commented on pull request "test: locked_wallet, skip default fee estimation":
(https://github.com/bitcoin/bitcoin/pull/28232#discussion_r1290085370)
sure, pushed. Thanks.
would be nice to have a document somewhere mentioning the existence of these utility functions.