🤔 amitiuttarwar reviewed a pull request: "p2p: Drop m_recently_announced_invs bloom filter"
(https://github.com/bitcoin/bitcoin/pull/27675#pullrequestreview-1555948320)
review ACK fb02ba3c5f5

it's been a while since I've hung out with mempool transaction code, so in addition to reading through the code I spent a good chunk of energy refreshing contextual behaviors. some context behind the ack:
- thinking through if there is a meaningful shift in privacy model: I find these changes acceptable
- understanding the memory tradeoff: this approach makes sense to me, esp with #27630 in the pipeline
- re-familiarizing myself with how transactions enter the mempo
...

💬 amitiuttarwar commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1279974340)
is this used anywhere?

💬 amitiuttarwar commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1280016445)
out of curiosity, why did you opt to make a separate function instead of having `last_sequence` be an optional field on `info`? is it to make it more apparent to future callers (aka devs implementing new call sites) that `last_sequence` must be passed in when retrieving a `TxMempoolInfo` with intent to relay?

💬 furszy commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1289385441)
nit: could verify the exact weight by summing up all the utxos weight.

💬 furszy commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1289386461)
can't be negative. It's a ranged amount.

💬 furszy commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1289382284)
`continue`?

💬 furszy commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1289385038)
`continue`?

💬 furszy commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1289387061)
The manual selection is always the same. Could place it outside of the loop.

💬 furszy commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1289387991)
```c++
assert(result->GetInputSet().size() == input_set.size() + manual_utxos.size());
```

💬 ajtowns commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1289400462)
No, it's not

💬 jonatack commented on pull request "doc: use llvm-config for bitcoin-tidy example":
(https://github.com/bitcoin/bitcoin/pull/28245#discussion_r1289401710)
Concept ACK on improving the example usage. Any tips on how to make it work with macOS 13.5 arm64? Seeing `Undefined symbols for architecture arm64` errors at this step in both versions of this README (thanks!)

<details><summary>terminal output</summary></p>

```zsh
jon|master =:~/bitcoin/bitcoin/contrib/devtools/bitcoin-tidy$ cmake --version
cmake version 3.27.1
jon|master =:~/bitcoin/bitcoin/contrib/devtools/bitcoin-tidy$ cmake -S . -B build -DLLVM_DIR=$(llvm-config --libdir)/cmake/llv
...

👋 jonatack's pull request is ready for review: "p2p: outbound network diversity improvements"
(https://github.com/bitcoin/bitcoin/pull/28248)

🤔 ariard reviewed a pull request: "Break up script/standard.{h/cpp}"
(https://github.com/bitcoin/bitcoin/pull/28244#pullrequestreview-1570874742)
Concept ACK

💬 ariard commented on pull request "Break up script/standard.{h/cpp}":
(https://github.com/bitcoin/bitcoin/pull/28244#discussion_r1289439494)
It could be noted what represents a “failed solve” is up to the caller. Actually `TxoutType::WITNESS_UNKNOWN` is considered as a failure in `AreInputsStandard`, as witness unknown is a special case of non-standardness for undefined segwit output.

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#issuecomment-1672442082)
Force-pushed addressing:
- https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1289387991
- https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1289387061
- https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1289386461
- https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1289382284

Thanks, @furszy!

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1289454868)
Done

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1289454926)
Done

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1289454987)
Done

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1289455057)
yea, done!

💬 fanquake commented on pull request "doc: use llvm-config for bitcoin-tidy example":
(https://github.com/bitcoin/bitcoin/pull/28245#discussion_r1289566686)
Use vanilla LLVM/Clang, Not Apple LLVM/Clang.