💬 sedited commented on pull request "kernel: Expose reusable `PrecomputedTransactionData` in script validation":
(https://github.com/bitcoin/bitcoin/pull/33891#discussion_r2568083521)
Can you also check that spent_outputs_len is 0 here? It might be the case that the passed in pointer is not null by chance, or UB for that matter, but the intent is still clearly communicated by passing a zero size.

💬 sedited commented on pull request "kernel: Expose reusable `PrecomputedTransactionData` in script validation":
(https://github.com/bitcoin/bitcoin/pull/33891#discussion_r2568162586)
I realize you are immitating my own patterns here, but I think it would be better to not rely too heaviliy on the move constructor here, and instead do the following:
```diff
diff --git a/src/kernel/bitcoinkernel.cpp b/src/kernel/bitcoinkernel.cpp
index 4102ee9305..2e86c3d8df 100644
--- a/src/kernel/bitcoinkernel.cpp
+++ b/src/kernel/bitcoinkernel.cpp
@@ -617 +617 @@ btck_PrecomputedTransactionData* btck_precomputed_transaction_data_create(
- PrecomputedTransactionData txdata{tx};
...

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2568229012)
Applied except the, IMO, overcrowded `if` condition. Not sure what is the difference between `Now<NodeSeconds>()` and `NodeClock::now()`. Do you suggest to use the former over the later?

💬 sedited commented on pull request "kernel: Expose reusable `PrecomputedTransactionData` in script validation":
(https://github.com/bitcoin/bitcoin/pull/33891#discussion_r2568239929)
Can you also add a case for passing in precomputed data for both the segwit and legacy transaction?

🤔 l0rinc requested changes to a pull request: "log: Use more severe log level (warn/err) where appropriate"
(https://github.com/bitcoin/bitcoin/pull/33960#pullrequestreview-3514733100)
Thanks for addressing these!
I have skimmed over the changes, I think some of the warning/error distinctions should be reconsidered.

💬 l0rinc commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#discussion_r2568175398)
Is there any particular reason for keeping the trailing newline here?

```suggestion
LogWarning("\n\n************************\n%s", message);
```

💬 l0rinc commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#discussion_r2568194967)
same:

```suggestion
LogWarning("[snapshot] Failed to restart index %s on snapshot chain", index->GetName());
```

💬 l0rinc commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#discussion_r2568173125)
hmmm, are you sure this is an error?

💬 l0rinc commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#discussion_r2568193209)
do we need to keep the `WARNING` prefix here?

```suggestion
LogWarning("SQLite is configured to not wait for data to be flushed to disk. Data loss and corruption may occur.");
```

💬 l0rinc commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#discussion_r2568197167)
```suggestion
LogWarning("Request rejected because http work queue depth exceeded, it can be increased with the -rpcworkqueue= setting");
```

💬 l0rinc commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#discussion_r2568179747)
If it's`Fatal`, shouldn't it be a `LogError`?

💬 l0rinc commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#discussion_r2568198946)
nit:
```suggestion
LogWarning("Unhandled HTTP request");
```

💬 l0rinc commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#discussion_r2568203370)
```suggestion
LogWarning("Cursor closed but could not finalize cursor statement: %s",
```

💬 l0rinc commented on pull request "log: Use more severe log level (warn/err) where appropriate":
(https://github.com/bitcoin/bitcoin/pull/33960#discussion_r2568217706)
So is this a warning or an error?

💬 m3dwards commented on pull request "depends, doc: Learn `x86_64-w64-mingw32ucrt` host and document it":
(https://github.com/bitcoin/bitcoin/pull/33857#issuecomment-3585450300)
> I can confirm the issue on `aarch64`

According to the Debian bug report, seems it's also an issue on RISC-V.

👍 sedited approved a pull request: "kernel: Add block header support and validation"
(https://github.com/bitcoin/bitcoin/pull/33822#pullrequestreview-3514976734)
ACK 10389bcfcf3eb92a08ff1fafab86db89749520c2

💬 sedited commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2568377264)
Nit (clang-format-diff): Needs a whitespace after `template`.

💬 sedited commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2568398257)
Nit (clang-format-diff): Missing whitespace to align the comment close.

💬 sedited commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2568391430)
Nit (clang-format-diff): Missing whitespace.

💬 optout21 commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#discussion_r2568419746)
Hmm, I see this as new code added by this commit, not present in the original.