👍 theStack approved a pull request: "wallet: coin selection, don't return results that exceed the max allowed weight"
(https://github.com/bitcoin/bitcoin/pull/26720#pullrequestreview-1375685000)
Code-review ACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635

💬 ajtowns commented on pull request "Remove -mempoolfullrbf option":
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1499931992)
> Right, so I'd be correct in saying that based on blocks from all mining pools generally containing about a dozen Extra Transactions on your miningpool.observer site, blocks generally contain about a dozen transactions not previously broadcast that compact blocks can't accellerate.

No, you wouldn't. Of the last 460 blocks, 375 (81%) required no additional txs, and an additional 67 (14%) required only one or two additional transactions. All of the 8 Luxor pool blocks in that set required betw
...

💬 1440000bytes commented on pull request "Deprecate and remove BIP35 mempool p2p message":
(https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1499987244)
> The [BIP35](https://github.com/bitcoin/bips/blob/master/bip-0035.mediawiki) mempool P2P message can be used to share the txids in a node's mempool (motivation in BIP35). However it is no longer used (?), is bad for privacy, and we can simplify our P2P code by removing it.

It is still possible to guess mempool transactions using `getdata` by maintaining mempool with no restrictions. I dont think sharing mempool affects privacy particularly in this case as its behind `NetPermissionFlags::Memp
...

🚀 fanquake merged a pull request: "ci: Run base install at most once"
(https://github.com/bitcoin/bitcoin/pull/27429)

💬 fanquake commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#issuecomment-1500021194)
We can have a followup, to address any further issues/improvements.

🚀 fanquake merged a pull request: "contrib: allow multi-sig binary verification v2"
(https://github.com/bitcoin/bitcoin/pull/27358)

💬 josibake commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1160562186)
gotcha, thanks for the explanation!

💬 vasild commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1500100974)
> > If we overshoot the 8 connections (like described above), then for some time we will have more, until eviction kicks in. How much time is that exactly? If that is short then it would be sub-optimal to open a IPv4 connection and drop it shortly after in cases where Tor connectivity is working smoothly. If it is long then we have practically increased the limit of 8 which will cause more traffic.
>
> Since we already have short-lived connections through feelers, rotating extra block relay o
...

🤔 josibake reviewed a pull request: "MiniTapscript: port Miniscript to Tapscript"
(https://github.com/bitcoin/bitcoin/pull/27255#pullrequestreview-1376054310)
Left some style nits, nothing important. Might want to run `clang-tidy`, as it ended up reformatting a lot of the new code. Also got a few warnings, not quite sure about the `FromPKBytes` warning

Still trying to wrap my head around the logic; I'll follow up with a (hopefully) more helpful review

💬 josibake commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1160599451)
```suggestion
KeyParser parser(/*out =*/&out, /*in=*/nullptr, /*ctx=*/miniscript::MiniscriptContext::P2WSH);
```

💬 josibake commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1160606743)
same comment as above re: `assert`

💬 josibake commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1160600034)
```suggestion
KeyParser parser(/*out=*/nullptr, /*in=*/&provider, /*ctx=*/miniscript::MiniscriptContext::P2WSH);

💬 josibake commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1160606636)
iirc, `assert` crashes the node. wouldn't it be better to use the `Assume` macro?

💬 josibake commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1160606878)
same comment as above re: `assert`

💬 josibake commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1160609483)
not a big deal, but I think it's more correct to say "CHECKMULTISIG is disabled in Tapscript" in the commit message

💬 josibake commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1160608237)
probably out of scope for this PR, but it would be nice to have the parser tell you why it wasn't able to parse the script, rather than just return empty

💬 josibake commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1160615879)
"warning: suggest parentheses around arithmetic in operand of ‘|’ [-Wparentheses]"

```suggestion
next_sats_diff.push_back((sats_diff[j] + subs[i]->ss.dsat.diff) | ((sats_diff[j - 1] + subs[i]->ss.sat.diff) + add_diff));

💬 josibake commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#discussion_r1160612094)
```suggestion
if (!parse_multi_exp(in, /*is_multi_a=*/true)) return {};
```

💬 Sjors commented on pull request "contrib: allow multi-sig binary verification v2":
(https://github.com/bitcoin/bitcoin/pull/27358#issuecomment-1500209136)
Followup suggestions:

1. allow user to specify the domain, e.g. `--domain=bitcoincore.org` so you don't get warnings about bitcoin.org, which only has up to version 22.0 at the moment. Conversely, it will make a future domain evacuation safer.

2. "osx" in the documentation example should be "macos" (but it doesn't work either way as @theuni found)

3. "The os/arch filters like verify.py 22.0-osx no longer work for me and I'm not quite sure why." - would be nice to restore this, or just f
...

👍 vasild approved a pull request: "Add "warnings", deprecate "warning" in {create,load,unload,restore}wallet"
(https://github.com/bitcoin/bitcoin/pull/27279#pullrequestreview-1376126094)
ACK e67da5fedd526c38e030d6d18f3678313a683dc9

Some non-blockers below.