💬 MarcoFalke commented on pull request "ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs #27321)":
(https://github.com/bitcoin/bitcoin/pull/27333#issuecomment-1484646255)
lgtm ACK f52d482408af68458f0b5c2775b8a1dffc380466

👍 MarcoFalke approved a pull request: "ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs #27321)"
(https://github.com/bitcoin/bitcoin/pull/27333)
left a question/nit, feel free to ignore

💬 MarcoFalke commented on pull request "ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs #27321)":
(https://github.com/bitcoin/bitcoin/pull/27333#discussion_r1148907877)
```suggestion
${CI_RETRY_EXE} dnf -y --allowerasing install $CI_BASE_PACKAGES $PACKAGES
```

I guess I know nothing about bash, but putting it this way may also work at no downside?

💬 MarcoFalke commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1484668108)
> FUZZ=process_message ./src/test/fuzz/fuzz ../qa-assets/fuzz_seed_corpus/http_request/75d231d5e022cc38377bbbf4549058d37b7c59a3

You'll need to set the correct fuzz target:


`FUZZ=http_request ./src/test/fuzz/fuzz ../qa-assets/fuzz_seed_corpus/http_request/75d231d5e022cc38377bbbf4549058d37b7c59a3`

💬 MarcoFalke commented on pull request "log: Check that the timestamp string is non-empty to avoid undefined behavior":
(https://github.com/bitcoin/bitcoin/pull/27317#discussion_r1148926296)
If there are steps to reproduce this, it would be good to write a detection for it at Bitcoin Core init time and refuse to start the program (if this isn't already done).

📝 MarcoFalke opened a pull request: "ci: Use Cirrus CI dockerfile env"
(https://github.com/bitcoin/bitcoin/pull/27340)
Currently the CI env has many intermittent issues:

* The Ubuntu package servers are frequently down
* Occasionally other stuff is down, such as dnf, pip, or the android sdk
* Installing packages is slower than downloading them, at least on Cirrus, which has a fast download speed

Fix all issues by using the Cirrus CI dockerfile env.

📝 MarcoFalke converted_to_draft a pull request: "ci: Use Cirrus CI dockerfile env"
(https://github.com/bitcoin/bitcoin/pull/27340)
Currently the CI env has many intermittent issues:

* The Ubuntu package servers are frequently down
* Occasionally other stuff is down, such as dnf, pip, or the android sdk
* Installing packages is slower than downloading them, at least on Cirrus, which has a fast download speed

Fix all issues by using the Cirrus CI dockerfile env.

💬 willcl-ark commented on issue "Not able to load multiple wallet more than X number of wallet from RPC":
(https://github.com/bitcoin/bitcoin/issues/15017#issuecomment-1484826665)
@adneerav As there has been no update on this since 2019 (aside from my own follow-up) and I was unable to reproduce this issue with the current version of Bitcoin Core, I'm going to close this for now.

Feel free to comment here if you are still experiencing this problem and want this re-opened to be investigated again.

✅ willcl-ark closed an issue: "Not able to load multiple wallet more than X number of wallet from RPC"
(https://github.com/bitcoin/bitcoin/issues/15017)

💬 hebasto commented on pull request "Fixes compile errors in MSVC build #27332":
(https://github.com/bitcoin/bitcoin/pull/27335#discussion_r1149059515)
Why this change is required?

💬 hebasto commented on pull request "Fixes compile errors in MSVC build #27332":
(https://github.com/bitcoin/bitcoin/pull/27335#discussion_r1149059179)
```suggestion
/* Assume that an MSVC build will get dependencies from vcpkg and thus use the latest version of libevent */
```

👍 hebasto approved a pull request: "Fix segfault when shutdown during wallet open"
(https://github.com/bitcoin-core/gui/pull/693)
ACK 9a1d73fdffa4f35e33bc187ac9b3afbba28b1950, I have reviewed the code and it looks OK.

💬 hebasto commented on issue "Segmentation fault when closing while loading wallets":
(https://github.com/bitcoin-core/gui/issues/689#issuecomment-1484922884)
@MarcoFalke

Can you confirm that https://github.com/bitcoin-core/gui/pull/693 fixes this issue for you?

💬 vasild commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1149028723)
nit:
```suggestion
const std::string strURI{hreq->GetURI()};
```

💬 vasild commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1149108682)
It it possible to enforce that this argument will never be null - if changed to reference. Simplifies the code a bit:

```diff
diff --git i/src/httpserver.cpp w/src/httpserver.cpp
index e9b68a36c1..1dccb196be 100644
--- i/src/httpserver.cpp
+++ w/src/httpserver.cpp
@@ -683,20 +683,18 @@ HTTPRequest::RequestMethod HTTPRequest::GetRequestMethod() const
return UNKNOWN;
}
}

std::optional<std::string> HTTPRequest::GetQueryParameter(const std::string& key) const
{
-
...

💬 vasild commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1149034300)
```suggestion
/** Get requested URI. Will always return non-nullptr.
```

💬 vasild commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1149110958)
`GetURI()` (which calls `evhttp_request_get_uri()`) could still return null. It seems too indirect to that we rely that we have called `evhttp_request_get_uri()` before on the same input and back then it returned non-null (from the constructor) so it must also return non-null now.

I think it would be better to save the result in a variable and document that it will never be null. This will also avoid calling `evhttp_request_get_uri()` two times. Like this:

```diff
diff --git i/src/httpser
...

🚀 fanquake merged a pull request: "depends: qrencode 4.1.1"
(https://github.com/bitcoin/bitcoin/pull/27312)

💬 hebasto commented on pull request "Wallet : Allow user to navigate options while encrypting at creation":
(https://github.com/bitcoin-core/gui/pull/722#discussion_r1149162701)
Could we avoid changes in translatable strings in this PR?

💬 hebasto commented on pull request "Wallet : Allow user to navigate options while encrypting at creation":
(https://github.com/bitcoin-core/gui/pull/722#discussion_r1149164776)
I'm not sure which label, "Go back" or just "Back", is more expected in user experience.

ping @GBKS