Bitcoin Core Github
44 subscribers
121K links
Download Telegram
🚀 achow101 merged a pull request: "init: Correct coins db cache size setting"
(https://github.com/bitcoin/bitcoin/pull/31064)
💬 andrewtoth commented on pull request "RPC: Add reserve member function to `UniValue` and use it in `getblock` RPC":
(https://github.com/bitcoin/bitcoin/pull/31179#issuecomment-2445122123)
I think we can take this further and reserve for VOBJ types instead of just VARR.
The VOBJ uses both values and keys, so we can reserve both. We can count how many times we do `pushKV` and reserve that amount for both keys and values.
💬 achow101 commented on pull request "test: extend the SOCKS5 Python proxy to actually connect to a destination":
(https://github.com/bitcoin/bitcoin/pull/29420#issuecomment-2445134031)
ACK 57529ac4dbb2721c1ad0a3566f0299dbdb5ca5c0
🚀 achow101 merged a pull request: "test: extend the SOCKS5 Python proxy to actually connect to a destination"
(https://github.com/bitcoin/bitcoin/pull/29420)
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1821427081)
Well, this really depends. `GetFanoutTargets` is called by `RelayTransaction`, which, in turn, may be called by `ProcessMessage` for a peer that has `NetPermissionFlags::ForceRelay`. So if force relay is used for both fanout and set reconciliation, as suggested in https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818575030, then this could be called twice leading to two different sorting if we don't do so deterministically
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1821431430)
Yeah, you're right. Given how small the two vectors are, calling `ShouldFanoutTo` with a merged vector should not incur any major overhead, so merging will make it easier to read
🤔 BrandonOdiwuor reviewed a pull request: "test: Don't enforce BIP94 on regtest unless specified by arg"
(https://github.com/bitcoin/bitcoin/pull/31156#pullrequestreview-2402922051)
utACK e60cecc8115d3b28be076792baa5e4ea26d353a6
💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1821276305)
nit: would be nice to check the changeset exists before applying it
🤔 instagibbs reviewed a pull request: "cluster mempool: Implement changeset interface for mempool"
(https://github.com/bitcoin/bitcoin/pull/31122#pullrequestreview-2402586962)
LGTM e2324779781b695024d3b17340001fa2da5d3c7f

with non-blocking nits

will do some testing
💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1821226055)
8e2aac1e87b67a5d28cdc507df9bca85614cc97d it's using the same cached value, so how can it return something new?

this disappears next commit of course...
💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1821344367)
copy/past error or something?
💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1821390942)
it's not called from ATMP or tests anymore, thankfully it's private now
💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1821388766)
while we're moving the comment, can we fix the grammar: "must updated"?
💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1821393533)
not your fault but: I don't think `setMemPoolParents` exists in master even

I feel like these paragraphs are a minefield of old information
💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1821389449)
comment above this isn't for `Apply`.
💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1821407375)
c8b896067d42001f2a47cd837c0aed07bb39f329 commit message could probably use a little massaging since removals can happen in a changeset? I'm assuming these are all block/trimming related
💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1821342151)
`GetAddedTxn` ?
💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1821435939)
text is better thanks
💬 instagibbs commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1821436409)
still curious about this if you figure out how it was hit
💬 jonatack commented on issue "`Wunused-member-function` in test each commit":
(https://github.com/bitcoin/bitcoin/issues/31180#issuecomment-2445195488)
As a reviewer, my preference is for the member or method to be used in the commit where it is added. This helps to more easily understand why it was added, and aids in reviewing the change without needing to flip repeatedly between the commits or git rebasing to squash them together.