💬 plebhash commented on issue "IPC via TCP Sockets":
(https://github.com/bitcoin/bitcoin/issues/32802#issuecomment-3543755211)
what about encryption?

assuming authentication is solved, would it be safe to use this outside of a controlled LAN environment?

💬 furszy commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2535484827)
I imagine the problem is that you haven't compared the current http code with the `ThreadPool` code, which is pretty much the same but properly documented and test covered. That's one of the reasons behind the not modernization of the underlying sync mechanism in this PR, and why I added the "Note 2" paragraph as well as wrote https://github.com/bitcoin/bitcoin/pull/33689#issuecomment-3444568607.

Look at the current `WorkQueue`:

```c++
void Run() EXCLUSIVE_LOCKS_REQUIRED(!cs)
{

...

💬 achow101 commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2535526914)
> > If the most recent evaluation of statement S failed, then sqlite3_finalize(S) returns the appropriate [error code](https://sqlite.org/rescode.html) or [extended error code](https://sqlite.org/rescode.html#extrc)
>
> I am not sure what is the best way to handle this. Ignoring the return value does not look good.

This text reads to me as `sqlite3_finalize` propagating any previous errors, e.g. from `sqlite3_step`. Since any errors from that should already be handled, I think it's ok to i
...

💬 achow101 commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2535561586)
Done

💬 achow101 commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2535561856)
Done

💬 achow101 commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2535562445)
Done

💬 achow101 commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2535562616)
Done

💬 achow101 commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2535562832)
Removed

💬 achow101 commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2535563023)
Done

💬 achow101 commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2535563262)
Done

💬 achow101 commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2535564145)
I've changed this to catch the exception and return `std::nullopt`.

💬 achow101 commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2535564673)
Changed to catch the exception.

💬 achow101 commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2535566375)
Done, returns an empty string now.

But the only time `sqlite3_column_text` should return NULL is if the column contains a NULL value. Empty strings are not NULL.

💬 achow101 commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2535566840)
Done

💬 achow101 commented on pull request "wallet, sqlite: Encapsulate SQLite statements in a RAII class":
(https://github.com/bitcoin/bitcoin/pull/33033#discussion_r2535567070)
Done

💬 achow101 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2535608194)
It's unnecessary to log this kind of information. This would be more appropriate as a comment, but it's really not necessary at all (yes this test does that elsewhere, but it shouldn't).

💬 achow101 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2535612380)
This log is unnecessary.

💬 achow101 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2535612003)
It's not necessary to generate `COINBASE_MATURITY` blocks since we're not spending those coins. This also makes the previously existing comment incorrect as that states only 10 blocks are being mined.

💬 achow101 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2535613879)
I don't think this test case is necessary, it doesn't seem to me to be testing anything that isn't already being tested.

💬 achow101 commented on pull request "Feature: Use different datadirs for different signets":
(https://github.com/bitcoin/bitcoin/pull/29838#issuecomment-3543990625)
Are you still working on this? This has needed rebase for a few months now.