💬 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.

💬 stringintech commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2532058549)
This can be simplified:
```suggestion
return btck_BlockValidationState::create();
```

💬 stringintech commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2532068254)
Not sure but "Must be destroyed ..." docs may not be necessary as we have not documented this for the rest of the owned returned pointers (e.g. `btck_Block*`).

<details>
<summary>diff</summary>

```diff
diff --git a/src/kernel/bitcoinkernel.h b/src/kernel/bitcoinkernel.h
index 89b69b849d..a980000968 100644
--- a/src/kernel/bitcoinkernel.h
+++ b/src/kernel/bitcoinkernel.h
@@ -918,7 +918,7 @@ BITCOINKERNEL_API const btck_BlockTreeEntry* BITCOINKERNEL_WARN_UNUSED_RESULT bt
* @brief Re
...

💬 stringintech commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2532063484)
We have usually documented the non-owned pointers this way: `The returned X is unowned and only valid for the lifetime of Y.`
```suggestion
/**
* @brief Get the previous block hash from block header. The returned hash is unowned and only valid for the lifetime of the block header.
*
* @param[in] header Non-null header
* @return Previous block hash
*/
BITCOINKERNEL_API const btck_BlockHash* BITCOINKERNEL_WARN_UNUSED_RESULT btck_block_header_get_prev_hash(
const b
...

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2532087454)
The above suggestion fails to compile on my machine:
```
src/rest.cpp: In function ‘bool rest_block_part(const std::any&, HTTPRequest*, const std::string&)’:
src/rest.cpp:505:43: error: expected ‘{’ before ‘[’ token
505 | } catch (const std::runtime_error& e) [[unlikely]] {
| ^

$ gcc --version
gcc (Ubuntu 13.3.0-6ubuntu2~24.04) 13.3.0
```