Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 yuvicc commented on pull request "kernel, validation: Refactor ProcessNewBlock(Headers) to return BlockValidationState":
(https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2531493620)
The problem is that `ActivateBestChain` would overwrite the `state` result, when the background chain's `ActivateBestChain` succeeds, it would overwrite state with its own `state`. Then at line 4554, we'd be returning the background chain's `state` instead of the active chainstate's `state`. So by separating out, we preserve the active chainstate result.
💬 yuvicc commented on pull request "kernel, validation: Refactor ProcessNewBlock(Headers) to return BlockValidationState":
(https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2531525257)
Correct, we will definitely need that.
💬 yuvicc commented on pull request "kernel, validation: Refactor ProcessNewBlock(Headers) to return BlockValidationState":
(https://github.com/bitcoin/bitcoin/pull/33856#issuecomment-3538243414)
Thanks for the review @hodlinator
- Added `Assume(!headers.empty())` before instantiating validation state as suggested in [comment](https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2517354777)
- Removed shadow `state` [comment](https://github.com/bitcoin/bitcoin/pull/33856#discussion_r2517333716)
⚠️ yuvicc opened an issue: "interface_ipc functional test failing in CI"
(https://github.com/bitcoin/bitcoin/issues/33884)
## Summary

The `interface_ipc` functional test is failing in CI but passes when run locally.

Affected PR's: #33856 and #33878
💬 stringintech commented on pull request "kernel: Rename in-memory DB option setters and simplify API":
(https://github.com/bitcoin/bitcoin/pull/33877#discussion_r2531820732)
Yes they are. No strong opinion, but it might be better to also explicitly set the default values here (in the kernel code - `ChainstateManagerOptions` ctor) for options we have setters for. Looks more intentional and readable to me this way.
But `coins_error_cb = {}` is to make the [compiler](https://github.com/bitcoin/bitcoin/pull/33877#issuecomment-3536504585) happy.
💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#issuecomment-3538691286)
Updated PR description with query performance and external offset index size for mainnet:
<img width="1093" height="297" alt="image" src="https://github.com/user-attachments/assets/88ade527-c26d-4572-af9d-cdca166ba7dd" />
💬 pinheadmz commented on issue ""Send" text field has too little amount":
(https://github.com/bitcoin/bitcoin/issues/33883#issuecomment-3538709055)
Weird! Would you mind moving this issue to the GUI repo?

https://github.com/bitcoin-core/gui
💬 ofry commented on issue ""Send" text field has too little amount":
(https://github.com/bitcoin/bitcoin/issues/33883#issuecomment-3538748689)
Ah, it's already created by another user. https://github.com/bitcoin-core/gui/issues/906
💬 stringintech commented on pull request "kernel: handle null or empty directories in implementation":
(https://github.com/bitcoin/bitcoin/pull/33867#issuecomment-3538851999)
ACK 6657bcb
pinheadmz closed an issue: ""Send" text field has too little amount"
(https://github.com/bitcoin/bitcoin/issues/33883)
💬 pinheadmz commented on issue ""Send" text field has too little amount":
(https://github.com/bitcoin/bitcoin/issues/33883#issuecomment-3538857274)
Ok thanks, closing this as duplicate
🤔 andrewtoth reviewed a pull request: "rest: allow reading partial block data from storage"
(https://github.com/bitcoin/bitcoin/pull/33657#pullrequestreview-3470261623)
Since happy path performance is important here, maybe adding `[[unlikely]]` attributes for all error paths could yield a benefit.
💬 andrewtoth commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2532037331)
```suggestion
if (!tx_verbosity.has_value()) [[unlikely]] {
```
💬 andrewtoth commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2532036843)
```suggestion
if (*part_size > size || *part_size == 0) [[unlikely]] {
```
💬 andrewtoth commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2532037585)
```suggestion
if (!part_offset.has_value()) [[unlikely]] {
```
💬 andrewtoth commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2532038587)
```suggestion
} catch (const std::runtime_error& e) [[unlikely]] {
```
💬 andrewtoth commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2532036409)
```suggestion
if (*part_offset > blk_size) [[unlikely]] {
```
💬 andrewtoth commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2532037702)
```suggestion
if (!part_size.has_value()) [[unlikely]] {
```
🤔 stringintech reviewed a pull request: "kernel: Add block header support and validation"
(https://github.com/bitcoin/bitcoin/pull/33822#pullrequestreview-3470272388)
Approach ACK
💬 stringintech commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2532051597)
Indentation can be fixed here.