Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 Sjors commented on pull request "Add checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2109284630)
This doesn't compile, but I assume you also meant to pass `view_dummy` into the `ConnectBlock` call below? I don't remember what the original was based on, so took the suggestion.
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific targets":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2109307871)
1. The [change](https://github.com/bitcoin/bitcoin/compare/16c69e927cccc1e93beaa10c621afd6a8f422e6c..0c3c7d07599d0086d5151949a69ffc12b6166d19) touches 8 files. The modification in `src/crypto/siphash.h` is a no-op from IWYU's perspective.

2. CMake integrates the IWYU into the compilation step. Therefore, it reports an error for each object file individually.
However, the diagnostic output includes both headers and the source file:
```
[12:32:50.757] Warning: include-what-you-use reported d
...
💬 pinheadmz commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#issuecomment-2912704473)
Is there a reason we don't just cache the hash as a member of `CBlockHeader` and compute-or-return that value in `GetHash()`?
🤔 instagibbs reviewed a pull request: "cluster mempool: add TxGraph reorg functionality"
(https://github.com/bitcoin/bitcoin/pull/31553#pullrequestreview-2854383890)
Reviewed through 538e9ff804f8dfba6f6a808e83572fdeab145ab8

Last big Q is about the cycles being generated in the implied graph, and what if anything we should do differently about it.
💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2098113781)
```Suggestion
const LinearizationChunking linchunking(m_depgraph, m_linearization);
```
💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2098239806)
nit: leave a comment that deps is also being added to here, briefly forgot
💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2098190802)
nit: 3 out of the N fields are being set here, could we be specific which ones for clarity?
💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2098240846)
```Suggestion
// Append explicit deps, and sort it by child.
```
💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2098254652)
```Suggestion
// Sort deps by parent by graph index. The order is unchanged from now on.
```
💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2098251956)
```Suggestion
// Fill m_deps_left in trim_data and initially populate trim_heap. Because of the sort above, all
```
💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2098265434)
```Suggestion
// a map from GraphIndex to TrimTxData via locate_fn, and its ordering will not change anymore.
```
💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2098308679)
```Suggestion
Assume(!ret.empty());
return ret;
```
💬 instagibbs commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2109385109)
https://github.com/instagibbs/bitcoin/tree/2025-05-applytrim

Worked on a test attempts to cover the claim here. It ended up being more complicated than that (claim a bit wrong?) due to the lazy application of dependencies. For this property to hold, I believe you have to touch the TxGraph in any way that applies dependencies when undersized, *then* transition to oversized graph for the circular dependencies to not occur in the implied graph.

Without the dependencies applied, the linearizat
...
🤔 sipa reviewed a pull request: "rpc: Round verificationprogress to 1 for a recent tip"
(https://github.com/bitcoin/bitcoin/pull/32528#pullrequestreview-2871433824)
utACK fab1e02086ceebd7d96417b7a386fe61158bfda9
💬 luke-jr commented on pull request "subprocess: Let shell parse command on non-Windows systems":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2912964132)
Probably should really go through the shell on Windows too...
💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2109501741)
Yes, but it loses the error information so you'd silently continue even if write failed leading to data corruption.
💬 Kixunil commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2109507090)
On the next run it'll crash failing to convert into `[u8; 8]`. While not catastrophic, it makes the program more annoying for end users.
🤔 l0rinc requested changes to a pull request: "improve MallocUsage() accuracy"
(https://github.com/bitcoin/bitcoin/pull/28531#pullrequestreview-2871139248)
Thanks for getting back to this change.
We should synchronize it with `CCoinsMap`, chich already claims to account for an "overhead of 1 or 2 pointers".
Also, the test should pass before the change as well, so it would be helpful to add it as a separate commit before the change to make it easy to see how it behaves before and after the change as well.
The test still contains a lot of repetition, please see my suggestion on how to compact it a bit more.
There are still uncovered parts of the
...
💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109307830)
Instead of empirically, can we add documentation or source code links here?
💬 l0rinc commented on pull request "improve MallocUsage() accuracy":
(https://github.com/bitcoin/bitcoin/pull/28531#discussion_r2109228294)
@LarryRuane I think you should probably be the author of the commit, given that you've already added @theStack as a co-author