Bitcoin Core Github
43 subscribers
123K links
Download Telegram
🤔 ryanofsky reviewed a pull request: "multiprocess: Add IPC wrapper for Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30510#pullrequestreview-2310227385)
Rebased b95bb2179610183d9398d50d8c8fd24b1450ad4d -> f9245e4bb880a6d76bbb994a6e1907c6f8c9f205 ([`pr/mine-types.11`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.11) -> [`pr/mine-types.12`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.12), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine-types.11-rebase..pr/mine-types.12)) due to silent conflict with #30440
💬 ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1763530005)
re: https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1762668345

> [02c6a1e](https://github.com/bitcoin/bitcoin/commit/02c6a1ed58867fa14b1e0c42f476ef45e64ea7ea) `-> (result: BlockTemplate);`

Thanks, should be updated in latest push.
💬 maflcko commented on pull request "log: Use ConstevalFormatString":
(https://github.com/bitcoin/bitcoin/pull/30889#discussion_r1763536070)
Ok, done
💬 andrewtoth commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1763544909)
I still think this should be an enum named `EntryState` or something, with all combinations of flags like https://github.com/bitcoin/bitcoin/pull/30906/commits/ba52ebd41673abf63d4495215c84be7244469d30#r1760349038. That's the proper way to do this instead of defining static vars for each value of a char we care about.
💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1763548746)
Absolutely, that's ny plan for tomorrow - but needed to clean up the test before doing that
💬 maflcko commented on pull request "log: Use ConstevalFormatString":
(https://github.com/bitcoin/bitcoin/pull/30889#issuecomment-2356432684)
> Agree with ryanofsky that it would be good to document rough compile time impact if any.

Sure added a note that it is less than 1% of both memory and time. The time should be offset by dropping the corresponding stuff from the Python linter, which is slower than a compiler at iterating over strings.
💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763454509)
Thanks! Taken.
💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763448694)
Dropped this test for now, and left a todo to add more tests to exercise different methods of mempool submission and ensure we never violate the cluster limits.
💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763330087)
Removed (it was needed in an intermediate commit).
💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1761997599)
Done.
💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763536059)
Taken.
💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1761997443)
good point. i wonder if we should just drop the feerate diagram language altogether and leave it at "insufficient fee"?
💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763532380)
Thanks! Taken.
💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763554660)
Thanks, removed.
💬 sdaftuar commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r1763546504)
Ah, right. Given that this falls back to the vsize limit being exercised, I think this isn't really worth including? Will leave this for now.
👋 maflcko's pull request is ready for review: "ci: Use clang-19 in msan tasks"
(https://github.com/bitcoin/bitcoin/pull/30639)
🤔 jonatack reviewed a pull request: "Drop -dbcache limit"
(https://github.com/bitcoin/bitcoin/pull/28358#pullrequestreview-2310356555)
ACK bb3b980dfd96d9a89e6b04aef2859ce0555c6807

Agee with more freedom/responsibility for the user and the ability to reduce flushing to disk for better IBD performance.

It's probably a good idea to avoid using strike-through tags in PR descriptions, as they aren't rendered well by the merge script and look like emphasis instead.

Due to recent UTXO set growth, the current maximum value for `-dbcache` of 16GB is ~just months away from being~ insufficient (for those who wish to complete
...
🤔 instagibbs reviewed a pull request: "cluster mempool: extend DepGraph functionality"
(https://github.com/bitcoin/bitcoin/pull/30857#pullrequestreview-2306885844)
First run through.

suggestion for clusterlin_add_dependencies harness: https://github.com/instagibbs/bitcoin/commit/c5c44da5a0a630a6725ca85e90e4c67b2eb1f8bc

I think this gets similar or better coverage and is easier for me to understand?

Tried augmenting `RemoveTransactions` coverage with adding things back in post deletion, and it's tripping up SanityCheck. Take a look?

```
diff --git a/src/test/fuzz/cluster_linearize.cpp b/src/test/fuzz/cluster_linearize.cpp
index be06898a6e..05d
...
💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1761348362)
?
```Suggestion
assert((depgraph.Descendants(child) & children) == SetType::Singleton(child));
```