📝 amitiuttarwar opened a pull request: "p2p: diversity network outbounds follow-ups"
(https://github.com/bitcoin/bitcoin/pull/28189)
builds on #27213 and addresses outstanding review comments

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1657295575)
thank you for the reviews everyone 🙌

In terms of next steps, we could either keep this PR as is in aims of merging soon & address follow-ups separately, or incorporate the review comments here. I've recapped the current status of the PR and am curious to hear feedback as to what route seems preferable.

* the current tip on this PR has 3 review ACKs ([vasild](https://github.com/bitcoin/bitcoin/pull/27213#pullrequestreview-1516160146), [mzumsande](https://github.com/bitcoin/bitcoin/pull/
...

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1278638362)
incorporated brace initialization in [`4428c6d` (#28189)](https://github.com/bitcoin/bitcoin/pull/28189/commits/4428c6d25b9a9a0cbcb6ff01525fe2d751b7a5aa)

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1278638425)
updated in [`4428c6d` (#28189)](https://github.com/bitcoin/bitcoin/pull/28189/commits/4428c6d25b9a9a0cbcb6ff01525fe2d751b7a5aa)

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1278639092)
for anyone following along in review, some historical context of why it was initially changed from `std::array` to `std::unordered_map` here: https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1242196931

@ajtowns I agree with your reasoning in https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1253827052 that an array doesn't tangibly make the code more brittle. since an array has a significantly smaller memory footprint, I think it's slightly preferable. incorporated in [`8449
...

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1278639126)
updated in [`84495cd` (#28189)](https://github.com/bitcoin/bitcoin/pull/28189/commits/84495cd2a8d12121b3164d20bd6b7c87a9ef43e2) by turning `m_network_counts` to an array

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1278639150)
done in [`006b8dd` (#28189)](https://github.com/bitcoin/bitcoin/pull/28189/commits/006b8dd3e6a161932a376a3988b5531410fa88a1)

💬 amitiuttarwar commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1657297571)
@mzumsande - release note added in [`a6d270d` (#28189)](https://github.com/bitcoin/bitcoin/pull/28189/commits/a6d270db4238c48a7a1a70e4398f2b33a97ed037)

💬 BitcoinMechanic commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1657332489)
> Replace by fee makes double spending easier and harm's Bitcoin's ability to be used as a currency in my opinion

RBF also allows people to be more frugal with their fee estimates as they can bump more easily if necessary - this certainly helps bitcoin's ability to be used a currency.

Moreover, it has never been appropriate to rely on unconfirmed transactions. Maintaining an illusion (that transactions that aren't in the blockchain can be relied upon) is not something worth attempting by f
...

⚠️ Jagaban2 opened an issue: "I tried on 25.0, where it will just fill the storage at full speed in a never ending loop (aborted after 30 minutes)"
(https://github.com/bitcoin/bitcoin/issues/28190)
I tried on 25.0, where it will just fill the storage at full speed in a never ending loop (aborted after 30 minutes)


_Originally posted by @MarcoFalke in https://github.com/bitcoin/bitcoin/issues/28057#issuecomment-1628461546_

💬 pokkst commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1657356702)
This entire discussion is pointless since Core won't listen to any feedback and will proceed with their centralized development and decision making process.

✅ achow101 closed an issue: "."
(https://github.com/bitcoin/bitcoin/issues/28190)

:lock: achow101 locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/28190)

💬 ChrisCho-H commented on pull request "script: check op_verif and op_vernotif":
(https://github.com/bitcoin/bitcoin/pull/28169#issuecomment-1657393698)
Thx! I was also considering it but not 100% sure whether `OP_VERIF` and `OP_VERNOTIF` can be treated as disabled. I will use the existing one as your suggestion.

💬 ajtowns commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1657669845)
> [Binance Pool, 8.2%](https://mempool.space/tx/64a26f750c7c58812bd475d054a9aa05248b6a8a2d53c0db38c0624197a4c68a)

This replaced 87f20bf4a8b71a255e47876ca7563cdcab1a830490a1b130bbe71c28d8da3884 which had a 3.14sat/vb feerate. statoshi reported minfees up to 3.09sat/vb in the period between the original propagating and the replacement being mined.

> [KuCoinPool, 1.4%](https://mempool.space/tx/8df183b087d433eac4d8e44eca5c7162a2540ee078a28ca04316fd82e12ec435)

This replaced e17630b903e9fb5bd
...

💬 ChrisCho-H commented on pull request "script: throw disabled err for op_ver and its variants":
(https://github.com/bitcoin/bitcoin/pull/28169#issuecomment-1657781606)
updated and passed the tests

💬 MarcoFalke commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#issuecomment-1657805615)
Can you link to 10 passing tests in your fork? Otherwise we may run into intermittent issues.

💬 MarcoFalke commented on pull request "fuzz: Test headers pre-sync through p2p interface":
(https://github.com/bitcoin/bitcoin/pull/28043#discussion_r1278891247)
I don't think you can use annotations one way or the other. As soon as you assign to `std::function<void()>`, the annotations are dropped, so you might as well not have them in the first place.

If you want them, and use `CallOneOf`, you can use the same trick to wrap each annotated lambda into a `std::function<void()>{lambda_bla}` temporary to achieve the same result of dropping any annotations.

💬 dergoegge commented on pull request "kernel: Prune leveldb headers":
(https://github.com/bitcoin/bitcoin/pull/28186#issuecomment-1657827011)
Concept ACK

💬 petertodd commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1657857733)
@ajtowns

> > [Binance Pool, 8.2%](https://mempool.space/tx/64a26f750c7c58812bd475d054a9aa05248b6a8a2d53c0db38c0624197a4c68a)
>
> This replaced 87f20bf4a8b71a255e47876ca7563cdcab1a830490a1b130bbe71c28d8da3884 which had a 3.14sat/vb feerate. statoshi reported minfees up to 3.09sat/vb in the period between the original propagating and the replacement being mined.
>
> > [KuCoinPool, 1.4%](https://mempool.space/tx/8df183b087d433eac4d8e44eca5c7162a2540ee078a28ca04316fd82e12ec435)
>
> This
...