💬 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
...

💬 MarcoFalke commented on pull request "kernel: Prune leveldb headers":
(https://github.com/bitcoin/bitcoin/pull/28186#issuecomment-1657883999)
> The dbwrapper is a key component of the libbitcoinkernel library. Future users of this library would not want to contend with having the leveldb headers exposed and potentially polluting their project's namespace.

Not sure about the motivation. I don't think any user should include the dbwrapper either? No user should be directly writing or reading from the db. The only reason why they'd have to include the header is to get access to `DBOptions`, which would alternatively and trivially be a
...

💬 MarcoFalke commented on pull request "ci: Use documented `CCACHE_MAXSIZE` instead of `CCACHE_SIZE`":
(https://github.com/bitcoin/bitcoin/pull/28188#issuecomment-1657898481)
lgtm ACK 79ceb161dbf7e033ce557d98e297bc3333665f26

💬 dergoegge commented on pull request "fuzz: Test headers pre-sync through p2p interface":
(https://github.com/bitcoin/bitcoin/pull/28043#discussion_r1278954263)
switched to `CallOneOf` and used `NO_THREAD_SAFETY_ANALYSIS`.