💬 theuni commented on pull request "utils: replace boost::date_time usage with c++20 std::chrono":
(https://github.com/bitcoin/bitcoin/pull/30462#issuecomment-2231717016)
> > Formatting in chrono is quite useful, so I don't think it makes sense to wait years until we require those compilers.
>
> Can you give some examples where it would be useful? Right now it is only used in a single place in the wallet.

I was assuming that it'd be useful in other places where it's simply cumbersome to deal with these conversions like logging. Or as a simplification for `FormatISO8601DateTime` in `util/time.cpp`. But sure, if we're not going to end up using any more of thi
...

🚀 achow101 merged a pull request: "rpc: Use CHECK_NONFATAL over Assert"
(https://github.com/bitcoin/bitcoin/pull/30429)

💬 achow101 commented on pull request "script/sign: avoid duplicated signature verification after signing (+introduce signing benchmarks)":
(https://github.com/bitcoin/bitcoin/pull/28923#issuecomment-2231730709)
ACK fe92c15f0c44d1405b9048306736bd0eae868506

💬 maflcko commented on pull request "utils: replace boost::date_time usage with c++20 std::chrono":
(https://github.com/bitcoin/bitcoin/pull/30462#issuecomment-2231739397)
> I started down this rabbit hole intending to simply rewrite this function, but ended up afraid of introducing bugs due to the locale footguns. Maybe I'm over-complicating it?

Happy to try myself, but https://en.cppreference.com/w/cpp/chrono/year_month_day (which can convert to system clock) is available, so using `Split` + `ToIntegral` + `year_month_day`, then adding the hours, minutes and seconds should do everything in about 20 lines of code?

💬 maflcko commented on pull request "doc: Drop description of LogError messages as fatal":
(https://github.com/bitcoin/bitcoin/pull/30361#issuecomment-2231748835)
> * It is not how `LogError` and `Level::Error` are used currently. Of 129 current uses only 58 cases are fatal according to [#30348](https://github.com/bitcoin/bitcoin/issues/30348)

The reason is that many stem from `error()`, see also https://github.com/bitcoin/bitcoin/pull/30364#pullrequestreview-2181072864.

> * "[T]here's not much benefit in a log that says "hey this error is about to cause the node to stop" -- you already get that information by seeing "Shutdown: in progress..." imme
...

📝 hebasto opened a pull request: "qa: Functional test improvements"
(https://github.com/bitcoin/bitcoin/pull/30463)
This PR includes changes split from https://github.com/bitcoin/bitcoin/pull/30454. They improve the functional test framework, allowing users to [run individual functional tests](https://github.com/hebasto/bitcoin/issues/146) from the build directory in the new CMake-based build system. This functionality is not available for out-of-source builds using the current Autotools-based build system, which always requires write permissions for the source directory.

💬 hebasto commented on pull request "build: Introduce CMake-based buid system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2231754276)
A couple of functional tests-related commits have been split to https://github.com/bitcoin/bitcoin/pull/30463, as suggested by @fanquake offline.

🚀 achow101 merged a pull request: "script/sign: avoid duplicated signature verification after signing (+introduce signing benchmarks)"
(https://github.com/bitcoin/bitcoin/pull/28923)

💬 maflcko commented on pull request "qa: Functional test improvements":
(https://github.com/bitcoin/bitcoin/pull/30463#issuecomment-2231767708)
The first commit could be split up as a scripted-diff? Also, formatting:

```diff
diff --git a/test/functional/interface_http.py b/test/functional/interface_http.py
index 567aa33cbe..dbdceb52d1 100755
--- a/test/functional/interface_http.py
+++ b/test/functional/interface_http.py
@@ -109 +109 @@ if __name__ == '__main__':
- HTTPBasicsTest(__file__).main ()
+ HTTPBasicsTest(__file__).main()
--
diff --git a/test/functional/rpc_getchaintips.py b/test/functional/rpc_getchaintips.py
...

💬 achow101 commented on pull request "Make it possible to disable Tor binds and abort startup on bind failure":
(https://github.com/bitcoin/bitcoin/pull/22729#issuecomment-2231770188)
ACK bca346a97056748f1e3fb899f336d56d9fd45a64

✅ achow101 closed an issue: "It is impossible to switch off binding for Tor"
(https://github.com/bitcoin/bitcoin/issues/22726)

✅ achow101 closed an issue: "No check is done whether the binding for Tor succeeded before using it for ADD_ONION"
(https://github.com/bitcoin/bitcoin/issues/22727)

🚀 achow101 merged a pull request: "Make it possible to disable Tor binds and abort startup on bind failure"
(https://github.com/bitcoin/bitcoin/pull/22729)

💬 achow101 commented on pull request "Fix cases of calls to `FillPSBT` errantly returning `complete=true`":
(https://github.com/bitcoin/bitcoin/pull/30357#discussion_r1680032048)
All of our PSBT updating operations should never remove any data from the PSBT. If the user decides to do something that invalidates a field in the PSBT, it's their duty to fix it themselves.

💬 achow101 commented on pull request "Fix cases of calls to `FillPSBT` errantly returning `complete=true`":
(https://github.com/bitcoin/bitcoin/pull/30357#issuecomment-2231778432)
ACK 7e36dca657c66bc70b04d5b850e5a335aecfb902ACK 7e36dca657c66bc70b04d5b850e5a335aecfb902

💬 brunoerg commented on pull request "fuzz: add target for `CoinsResult`":
(https://github.com/bitcoin/bitcoin/pull/30461#issuecomment-2231778444)
I think fuzzing it directly is good and there are some `CoinsResult` functions that are not fuzzed (`All`, `Erase`, `Clear`), but tbh just noticed they're used only in unit test, so the only benefit would be the performance of fuzzing it directly. I'm ok on closing it if case.

💬 brunoerg commented on issue "Fuzz coverage for wallet RPCs is zero":
(https://github.com/bitcoin/bitcoin/issues/30458#issuecomment-2231782205)
Added this in https://github.com/bitcoin/bitcoin/issues/29901.

💬 hebasto commented on pull request "qa: Functional test improvements":
(https://github.com/bitcoin/bitcoin/pull/30463#issuecomment-2231830637)
> The first commit could be split up as a scripted-diff? Also, formatting:
>
> ```diff
> diff --git a/test/functional/interface_http.py b/test/functional/interface_http.py
> index 567aa33cbe..dbdceb52d1 100755
> --- a/test/functional/interface_http.py
> +++ b/test/functional/interface_http.py
> @@ -109 +109 @@ if __name__ == '__main__':
> - HTTPBasicsTest(__file__).main ()
> + HTTPBasicsTest(__file__).main()
> --
> diff --git a/test/functional/rpc_getchaintips.py b/test/functio
...

✅ achow101 closed an issue: "RPC: Internal bug in `walletprocesspsbt` when non_witness_utxo is not provided and a witness signature is invalid"
(https://github.com/bitcoin/bitcoin/issues/30077)

🚀 achow101 merged a pull request: "Fix cases of calls to `FillPSBT` errantly returning `complete=true`"
(https://github.com/bitcoin/bitcoin/pull/30357)