💬 fanquake commented on pull request "refactor: replace `const char*` exceptions with `std::runtime_error`":
(https://github.com/bitcoin/bitcoin/pull/33569#issuecomment-3381316115)
> but it seems this is a workaround for an upstream bug?

Yep, looks like upstream has confirmed it's an upsteam bug: https://github.com/mstorsjo/llvm-mingw/issues/522#issuecomment-3381159016.

💬 maflcko commented on pull request "Bugfix: QA: rpc_bind: Skip nonloopback test if no such address is found":
(https://github.com/bitcoin/bitcoin/pull/33433#discussion_r2413754626)
oh, I got the logic exactly reversed

💬 maflcko commented on pull request "Bugfix: QA: rpc_bind: Skip nonloopback test if no such address is found":
(https://github.com/bitcoin/bitcoin/pull/33433#issuecomment-3381368024)
review ACK 79b4c276e7b9b526fa8f563b1e09b2b970baece6 🏑

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 79b4c276e7b9
...

💬 Eunovo commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2413787797)
Is there value in making the `ThreadPool` safe for recursive task submission? We might not be able to implement certain divide and conquer algorithms, but we might not need to.

💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3381463897)
Rebased 64d1449ff47dcb2d51a25f4178845538061c8644 -> 2f6d5637d8cd1dc1a1444e31fdfb2dfb13152500 ([kernelApi_69](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_69) -> [kernelApi_70](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_70), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_69..kernelApi_70))


* Fixed silent merge conflict with #32998.
* Some smaller improvements to the c++ header.

💬 andrewtoth commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2413851837)
I don't think so. In that case, should your test have some warnings that this can be unsafe if the thread pool is stopped or destroyed before all calls to `Submit` have returned?

💬 hodlinator commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-3381560071)
I hate spam too. Thanks @hsdredgun for running a node.

> this change is catastrophic.

No. Data can already be encoded in witness data at a ~4x discount compared to OP_RETURN.

> Fix the witness discount problem.

Not sure what you are referring to here, changes like #28408 lead to an endless cat & mouse game. Spammers can release new spam-encoding schemes faster than new node software can be released. Even if we had some kind of frequently updated dynamic filtering mechanism that was a
...

💬 Eunovo commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2413885650)
We can modify the test to make the parent task wait for the child task to complete before returning. `Stop` waits for all works to complete before clearing the workers vector. That is, if @furszy determines that there is any point to a recursive test at all, since we are not designing for that

💬 furszy commented on pull request "doc: how to update a subtree":
(https://github.com/bitcoin/bitcoin/pull/33568#discussion_r2413892101)
yeah, that's what I meant.

🤔 furszy reviewed a pull request: "doc: how to update a subtree"
(https://github.com/bitcoin/bitcoin/pull/33568#pullrequestreview-3314862355)
ACK a1226bc760c70a22ef4a197d5690aca4d83cb74c

💬 Eunovo commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2413902914)
https://github.com/bitcoin/bitcoin/pull/26966/commits/0afb05cad29e4c30a2f9eb4295e997967129dc88:

Is it not beneficial for a generic ThreadPool to support assigning tasks in batches to a thread? I expect this to be useful for many small tasks.

💬 plebhash commented on issue "[`v30.0rc3`]`bitcoin-node` aborts with mining IPC interface usage":
(https://github.com/bitcoin/bitcoin/issues/33554#issuecomment-3381673850)
I have a question related to the approach I started taking with the insights coming from the crashes reported here.

following your advice, I'm no longer using one single server thread for everything... now there's one "static" server thread that's responsible for all non-blocking calls (e.g.: fetching some template data or submitting solution to some template)

and for `waitNext`, there's dedicated server thread(s)

the thing is, I still need to cater for incoming Sv2 `CoinbaseOutputConstraints
...

🤔 ismaelsadeeq reviewed a pull request: "kernel: Introduce initial C header API"
(https://github.com/bitcoin/bitcoin/pull/30595#pullrequestreview-3304903446)
Halfway through review, I will update this as I go on.

- [x] [kernel: Introduce initial kernel C header API](https://github.com/bitcoin/bitcoin/pull/30595/commits/166138374857eab7d4717471a8aacaa9d6ff2218)166138374857eab7d4717471a8aacaa9d6ff2218
- [x] [kernel: Add logging to kernel library C header](https://github.com/bitcoin/bitcoin/pull/30595/commits/2c686196e70465a5211e866c0ae73804015f1c4e)
- [x] [kernel: Add kernel library context object](https://github.com/bitcoin/bitcoin/pull/30595/commit
...

💬 ismaelsadeeq commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2406541898)
In "kernel: Introduce initial kernel C header API" d192006d7f4e8f8ec7371f5921c7ea154c45f2d9


Why do we change amount to value here?
also there is inconsistency later in `btck_script_pubkey_verify` where we call it amount all through, might be better to just stick with value?

💬 ismaelsadeeq commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2406532719)
In "kernel: Introduce initial kernel C header API" d192006d7f4e8f8ec7371f5921c7ea154c45f2d9

Will prevented double cast here by caching the vouts by reference first and then using in assert and getting the output be better?

💬 ismaelsadeeq commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2406561682)
In "kernel: Introduce initial kernel C header API" d192006d7f4e8f8ec7371f5921c7ea154c45f2d9

Is passing `spent_outputs_len` and verifying it is the same size with the tx output size necessary here? at first pass I see no harm in iterating through the tx outputs and appending them to the vector.

💬 ismaelsadeeq commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2406655796)
In "kernel: Introduce initial kernel C header API" d192006d7f4e8f8ec7371f5921c7ea154c45f2d9

Shouldn't the * operator return a reference and -> operator return a pointer, maybe I am missing something?

💬 ismaelsadeeq commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2406606561)
In "kernel: Introduce initial kernel C header API" d192006d7f4e8f8ec7371f5921c7ea154c45f2d9


Should we have an opaque script type? then btc_scriptpubkey_create will return it, later if we want to represent scriptSig or witness script we dont have to define their own opaque type.

💬 ismaelsadeeq commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2406574903)
In "kernel: Introduce initial kernel C header API" d192006d7f4e8f8ec7371f5921c7ea154c45f2d9

```
The flags very combined in an invalid way.
```

This comment is ambiguous what do you mean?

💬 ismaelsadeeq commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2406770176)
In "kernel: Introduce initial kernel C header API" d192006d7f4e8f8ec7371f5921c7ea154c45f2d9

Why aren't returning *this by reference and making a copy here same in the move assignment operator?