💬 ryanofsky commented on pull request "Split CConnman":
(https://github.com/bitcoin/bitcoin/pull/30988#issuecomment-3483436641)
Concept ACK 69c015b258c3bbfb44af478d7a5a05ad0b0156b8 and I've started reviewing this. Overall I think this is a nice change that simplifies net.cpp by removing low level socket calls and details of establishing connections from implementation of the transport protocol. It also seems like it should make net.cpp code more portable if for example we wanted it to use libuv, or a future c++ networking library, or experiment with sandboxing, mesh networking, etc. The [sockman API](https://github.com/v
...

💬 yuvicc commented on pull request "rpc, test: allow multiple data outputs in `createrawtransaction` & `createpsbt`":
(https://github.com/bitcoin/bitcoin/pull/32790#issuecomment-3483658143)
Ready for review.

💬 delta1 commented on issue "dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us appears to be violating DNS seed policy":
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3483710103)
@luke-jr since you consider Core v30 to be “malware”, does that mean your seeder will never support returning v30 nodes?

🤔 yuvicc reviewed a pull request: "kernel: Introduce C header API"
(https://github.com/bitcoin/bitcoin/pull/30595#pullrequestreview-3302473141)
Approach ACK

Did a round of review while updating the [java-bitcoinkernel](https://github.com/yuvicc/java-bitcoinkernel/pull/4) bindings and testing the API.

Left some nits, mostly documentation consistency and minor code improvements.

💬 yuvicc commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2404587136)
Can we mark the context as const here?

```suggestion
const btck_Context* context) BITCOINKERNEL_ARG_NONNULL(1);
```

💬 yuvicc commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2488609371)
```suggestion
* `btck_ScriptVerificationFlags_WITNESS` flag is set in the flags bitfield, the
```

💬 yuvicc commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2488614705)
```suggestion
* @param[in] flags Bitfield of btck_ScriptVerificationFlags controlling validation constraints.
```

💬 yuvicc commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2488618692)
```suggestion
* @param[in] category If btck_LogCategory_ALL is chosen, all categories will be enabled.
```

💬 yuvicc commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2488601949)
```suggestion
* read block and undo data from disk, and validate scripts. The header is
```

💬 yuvicc commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2404583482)
I think we can use `int32_t` here like we've used in `btck_block_tree_entry_get_height` method?

```suggestion
BITCOINKERNEL_API int32_t BITCOINKERNEL_WARN_UNUSED_RESULT btck_chain_get_height(
```

💬 yuvicc commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2488671851)
Could be more specific
```suggestion
* @param[in] output_index The index of the transaction output to be retrieved.
```

💬 yuvicc commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2488694914)
```suggestion
/**
```

💬 yuvicc commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2488700233)
typo
```suggestion
throw std::runtime_error("Failed to write serialization data");
```

💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#issuecomment-3484111220)
Tested mainnet query performance using https://mempool.space/address/1BitcoinEaterAddressDontSendf59kuE
Assuming warm OS block cache, https://github.com/romanz/bindex-rs/pull/63#issuecomment-3484098889 takes <1s to fetch 5190 txs.

💬 frankomosh commented on pull request "test: Add bitcoin-chainstate test for assumeutxo functionality":
(https://github.com/bitcoin/bitcoin/pull/33728#issuecomment-3484126740)
ACK 3e7d272

💬 GBKS commented on issue "Async Payjoin":
(https://github.com/bitcoin/bitcoin/issues/33684#issuecomment-3484165074)
> > Given that this is adding new features to the GUI. Is the plan to add them to the "legacy" GUI, the new QML GUI, or implement Payjoin into both? cc [@hebasto](https://github.com/hebasto)
>
> cc [@GBKS](https://github.com/GBKS) [@johnny9](https://github.com/johnny9)

I am not aware of any plans, so this is probably something to be decided. Considering the limited resources that go into the QT and QML GUIs and how hard it is to just make one piece of software extremely good, I don't see it as
...

💬 GBKS commented on pull request "Increase tooltip wrap threshold from 80 to 100 characters":
(https://github.com/bitcoin-core/gui/pull/905#issuecomment-3484175160)
I think some examples would be helpful to evaluate this change. Could you possibly share before and after screenshots?

💬 TheCharlatan commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2488985360)
We could, but I think I prefer not to mark function arguments as const if their state is mutated internally.

💬 TheCharlatan commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3484270389)
Thank you for the review @yuvicc,

e95efc00842d5d0df96ee9294cdf818741be539e -> 6c7a34f3b0bd39ef7a1520aac56e12f78e5cc969 ([kernelApi_80](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_80) -> [kernelApi_81](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_81), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_80..kernelApi_81))

* Addressed a bunch of typo and documentation nits.
* Addressed @yuvicc's [comment](https://github.com/bitcoin/bitcoin/pull/30595#disc
...

✅ maflcko closed a pull request: "ci: Run unit tests parallel with functional tests"
(https://github.com/bitcoin/bitcoin/pull/33000)