Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 rkrux commented on pull request "wallet, rpc: Return normalized descriptor in parent_descs":
(https://github.com/bitcoin/bitcoin/pull/32594#discussion_r2109107403)
Nit: Most likely this sentence was reworded midway because it ends abruptly on first line and the content is duplicated on the next.
👍 hebasto approved a pull request: "test: Add missing ipc subtree to lint"
(https://github.com/bitcoin/bitcoin/pull/32623#pullrequestreview-2871162934)
ACK fa2bf6bdb7e5f8940fe4bce1aaab29bf2e063b49.
💬 hebasto commented on pull request "test: Add missing ipc subtree to lint":
(https://github.com/bitcoin/bitcoin/pull/32623#discussion_r2109243775)
Maybe add a reverse reference to this list in `test/lint/test_runner/src/main.rs`?
💬 andrewtoth commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2109251771)
Does it matter though? If it crashes before sync_all we either have all zero in xor.dat or a random key that has not yet been used for xoring anything. On next run it will either rewrite a new key if still zero or start using the previously written random key.
💬 andrewtoth commented on pull request "contrib: add xor-blocks tool to obfuscate blocks directory":
(https://github.com/bitcoin/bitcoin/pull/32451#discussion_r2109256380)
`into_inner` flushes before returning. https://doc.rust-lang.org/std/io/struct.BufWriter.html#method.into_inner
🤔 jonatack reviewed a pull request: "blocks: avoid recomputing block header hash in `ReadBlock`"
(https://github.com/bitcoin/bitcoin/pull/32487#pullrequestreview-2871187569)
ACK 09ee8b7f278627b917f0784adf23cbc76cae5fa0
💬 jonatack commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#discussion_r2109259969)
If you retouch now or in another pull, would be clearer to state which blockhash is expected vs actual.

```suggestion
LogError("GetHash() doesn't match index at %s while reading block (actual %s != expected %s)",
```
andrewtoth closed a pull request: "contrib: add xor-blocks tool to obfuscate blocks directory"
(https://github.com/bitcoin/bitcoin/pull/32451)
👍 pinheadmz approved a pull request: "rpc: Round verificationprogress to 1 for a recent tip"
(https://github.com/bitcoin/bitcoin/pull/32528#pullrequestreview-2871206816)
ACK fab1e02086ceebd7d96417b7a386fe61158bfda9

Trivial code cleanup changes since last ack. Also re-built and tested on macos/arm64

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK fab1e02086ceebd7d96417b7a386fe61158bfda9
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmg1wvoACgkQ5+KYS2KJ
yTrrtBAAhzspf6yfD6do0zH7Cu0I4OMA1C/KlabXxcyA9O4Foa/sNjx//iXEKFIX
EO0fEfcGZ+cWwlpMGG5TD+NfPl6jKfVb376xxESSUT4HvUIOB/
...
💬 theStack commented on pull request "contrib: utxo_to_sqlite.py: add option to store txid/spk as BLOBs":
(https://github.com/bitcoin/bitcoin/pull/32621#issuecomment-2912629691)
@ajtowns: Thanks for your suggestions, I like the idea of having more flexibility and mostly agree with the naming. Took your patch (TIL about [argparse `choices`](https://docs.python.org/3/library/argparse.html#choices)) and adapted the functional test accordingly, to exercise all combinations of new options. As for txid format option naming, I think I'd intuitively prefer `raw` for storing them in the hash order, since that's AFAICT the most common txid byte-string serialization (I think I hav
...
💬 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
```