📝 jonatack opened a pull request: "p2p: outbound network diversity improvements"
(https://github.com/bitcoin/bitcoin/pull/28248)
Please see the commit messages for details.

🤔 donsheddy4 reviewed a pull request: "bumpfee: Allow the user to choose which output is change"
(https://github.com/bitcoin/bitcoin/pull/26467#pullrequestreview-1570563013)
Very good

💬 jonatack commented on pull request "doc: Use GitHub's "Alert" markdown syntax":
(https://github.com/bitcoin/bitcoin/pull/28243#issuecomment-1672248656)
Just one opinion, but I find these alerts/admonitions distract from the text itself both when read on GitHub and also when read in markdown format. They also make reading the source files more difficult, which is how I consume the docs.

It's also preferable to avoid GitHub-specific features and workflows. Greater developer lock-in has been a clear goal for GitHub for years now.

💬 jonatack commented on pull request "doc: Improve documentation of rpcallowip":
(https://github.com/bitcoin/bitcoin/pull/27480#discussion_r1289302225)
While retouching, mind `s/Valid for/Valid values for/` or `s/source. Valid for <ip> are/source, which can be`

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1289335082)
Just changed it to not use two srd solutions and I created a function to "simulate" a manual selection. Also, added more asserts, including checking the input set.

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1289335245)
Sounds good. Just did it.

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#discussion_r1289336536)
just changed to `1000`. but the reason is to make it more realistic. e.g. do we really use it for 1 sat?

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#issuecomment-1672291591)
Force-pushed:

- Improved `Merge`: now it checks the input set and it tries to merge every solution with a "manual" one.
- Improved `AddInputs` to check the total weight.

💬 jonatack commented on pull request "Use shared_ptr for CNode inside CConnman":
(https://github.com/bitcoin/bitcoin/pull/28222#issuecomment-1672297168)
Seeing these failures locally:

```
$ ./src/test/test_bitcoin -t denialofservice_tests
Running 5 test cases...
Assertion failed: lock m_nodes_mutex not held in net.cpp:1598; locks held:
'cs_main' in net_processing.cpp:5203 (in thread 'test')
unknown location:0: fatal error: in "denialofservice_tests/stale_tip_peer_management": signal: SIGABRT (application abort requested)
test/denialofservice_tests.cpp:182: last checkpoint
Assertion failed: (ret.second), function AddArg, file args.cpp,
...

💬 ariard commented on pull request "doc: Use GitHub's "Alert" markdown syntax":
(https://github.com/bitcoin/bitcoin/pull/28243#issuecomment-1672297394)
+1 Sooner we’ll move out of Github better we’ll be and we would be rather to prepare for greater disruptions of GH availability (e.g regularly backing up the decade of comments on the repo) than rely more on its features.

💬 brunoerg commented on pull request "fuzz: fix a couple incorrect assertions in the `coins_view` target":
(https://github.com/bitcoin/bitcoin/pull/28215#issuecomment-1672302859)
Concept ACK

🤔 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());
```