Bitcoin Core Github
43 subscribers
123K links
Download Telegram
📝 nichechristie opened a pull request: "Claude/create coin 011 c uz rux6 kwdft yev8 a5 zsg"
(https://github.com/bitcoin/bitcoin/pull/33844)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...
hebasto closed a pull request: "Claude/create coin 011 c uz rux6 kwdft yev8 a5 zsg"
(https://github.com/bitcoin/bitcoin/pull/33844)
🤔 janb84 reviewed a pull request: "kernel: trim Chain interface"
(https://github.com/bitcoin/bitcoin/pull/33820#pullrequestreview-3443942718)
ACK 66978a1a95379a2fe5d41032682dedfaddc99db9

Was already wondering why these 2 functions where present in the code. With the iteration ability I agree that there is no need for these "speciality" functions. The functionality can be implemented downstream if needed.

steps taken:
code review,
build,
ran all tests,

(small contemplation; chain.tip is 'often' used in the rest of the bitcoin code, is removing the `tip` function from the header not a performance burden downstream to ca
...
💬 janb84 commented on pull request "kernel: trim Chain interface":
(https://github.com/bitcoin/bitcoin/pull/33820#discussion_r2511078966)
```C
auto genesis_block_raw{chainman->ReadBlock(genesis_index).value().ToBytes()};
auto first_index{chain.GetByHeight(0)};
auto first_block_raw{chainman->ReadBlock(genesis_index).value().ToBytes()};
```

Not sure if this part of the test still makes sense, the double retrieval of the data of block 0 `auto first_block_raw{chainman->ReadBlock(genesis_index).value().ToBytes()};`
we are now testing if `chainman->ReadBlock` will retrieve the same block twice not if it will get the
...
💬 achow101 commented on pull request "log: reduce excessive "rolling back/forward" messages during block replay":
(https://github.com/bitcoin/bitcoin/pull/33443#issuecomment-3512621236)
ACK 1fc7a81f1f5f30ba3ebe305ac2a520c7b4afb596
📝 maflcko opened a pull request: "ci: Enable experimental kernel stuff in ASan task"
(https://github.com/bitcoin/bitcoin/pull/33845)
Base the task on --preset=dev-mode to ensure maximal coverage and add the following:

```
bitcoin-chainstate (experimental) ... ON
libbitcoinkernel (experimental) ..... ON
kernel-test (experimental) .......... ON
```

Currently a draft to figure out https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3511082065
🚀 achow101 merged a pull request: "log: reduce excessive "rolling back/forward" messages during block replay"
(https://github.com/bitcoin/bitcoin/pull/33443)
💬 maflcko commented on pull request "ci: Enable experimental kernel stuff in ASan task":
(https://github.com/bitcoin/bitcoin/pull/33845#issuecomment-3512701642)
Continue from https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3511082065:

> > I was thinking about permitting nullptr when size=0, or even just permitting nullptr for any size, when the data pointer is followed by a size?
>
> Afaict our de-serialization can handle this case (nullptr when size=0) correctly, but fails when passed a nullptr with non-zero size. Maybe a null check in `btck_block_create(...)` would be better? What do you think?

Yes, either should be fine. To clarif
...
💬 maflcko commented on pull request "ci: Enable experimental kernel stuff in most CI tasks":
(https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3512704107)
Split the failing CI tasks out to https://github.com/bitcoin/bitcoin/pull/33845 and https://github.com/bitcoin/bitcoin/pull/33842
💬 achow101 commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#issuecomment-3512710368)
ACK 01cc20f3307c532f402cdf5dc17f2f14920b725b
💬 kevkevinpal commented on pull request "test: Ignore error message give from python because of PYTHON_GIL":
(https://github.com/bitcoin/bitcoin/pull/33795#issuecomment-3512720950)
Added a TODO to remove the warning suppression once `capnp` can run safely without the GIL

[5b7778c](https://github.com/bitcoin/bitcoin/pull/33795/commits/5b7778ca6d00db3fef92384c373e67261abb65a0)
fanquake closed an issue: "log: "Replaying blocks" / "Rolling forward" logging is rate-limited"
(https://github.com/bitcoin/bitcoin/issues/33769)
💬 fanquake commented on issue "log: "Replaying blocks" / "Rolling forward" logging is rate-limited":
(https://github.com/bitcoin/bitcoin/issues/33769#issuecomment-3512779755)
Closing given #33443.
achow101 closed an issue: "p2p: Inefficiency in block download / stalling algorithm"
(https://github.com/bitcoin/bitcoin/issues/32179)
🚀 achow101 merged a pull request: "p2p: Advance pindexLastCommonBlock early after connecting blocks"
(https://github.com/bitcoin/bitcoin/pull/32180)
💬 achow101 commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#issuecomment-3512800884)
ACK 060bb55508245776bb6a39c8b7849769ee588d69
💬 achow101 commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2511292626)
In 26442fec572da263765c576c080186c84cee1615 "bench: Add wallet encryption benchmark"

We've generally not had "measure overhead" in the benchmarks. If this is because encryption is actually a very small amount of time, I would suggest generating significantly more keys that need to actually be encrypted.
💬 achow101 commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2511269902)
In 7176b26cde7cbaffdd92af9c25f85f8e5233e78a "refactor: Generalize derivation target calculation"

This comment does not seem helpful, I'm not sure what information it is trying to convey.
💬 achow101 commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2511273130)
In 26442fec572da263765c576c080186c84cee1615 "bench: Add wallet encryption benchmark"

This part of setup is unnecessary. Encryption does not encrypt destinations or transactions. Generating new destinations does not result in more things to be encrypted.

If the idea was to have a bunch of keys to be encrypted for the benchmark, then several independent descriptors would need to be imported.
💬 fanquake commented on pull request "build: add `-W*-whitespace`":
(https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3512823435)
> Thanks. I've pulled those two in here.

Looking at the CI. Those commits are not going to work as implemented (essentially anywhere we are using Clang).