💬 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

💬 hebasto commented on issue "Add go back button to `Confirm wallet encryption` window, add cancel button to `wallet to be encrypted` window":
(https://github.com/bitcoin-core/gui/issues/394#issuecomment-1484969506)
@jarolrod

Mind reviewing https://github.com/bitcoin-core/gui/pull/722?

✅ fanquake closed a pull request: "Torcontrol opt check"
(https://github.com/bitcoin/bitcoin/pull/25136)

💬 fanquake commented on issue "depends does not compile with clang-16 (fontconfig)":
(https://github.com/bitcoin/bitcoin/issues/27299#issuecomment-1484978452)
> Same for qrencode:
> The qrencode issue also with clang-15:

This portion of this issue should have been addressed by #27312.

🚀 fanquake merged a pull request: "depends: fix osx build with clang 16"
(https://github.com/bitcoin/bitcoin/pull/27328)

💬 prusnak commented on pull request "depends: qrencode 4.1.1":
(https://github.com/bitcoin/bitcoin/pull/27312#issuecomment-1484985732)
There is this library which is maintained and available in multiple programming languages (C and C++ are two of them).

💬 MarcoFalke commented on pull request "ci: Use Cirrus CI dockerfile env":
(https://github.com/bitcoin/bitcoin/pull/27340#issuecomment-1484988445)
No idea why CI fails. Seems to be related to https://superuser.com/questions/1642858/git-on-debian-10-backports-throws-fatal-unable-to-access-https-github-com-us

💬 hebasto commented on pull request "Switch RPCConsole wallet selection to the one most recently opened/restored/created":
(https://github.com/bitcoin-core/gui/pull/696#discussion_r1149178367)
1. Should it be declared as a slot?

2. In the `qt` subdirectory we usually follow Qt's naming convention. Mind renaming please
```suggestion
void setCurrentWallet(WalletModel* const wallet_model);
```
?

💬 hebasto commented on pull request "Switch RPCConsole wallet selection to the one most recently opened/restored/created":
(https://github.com/bitcoin-core/gui/pull/696#discussion_r1149184463)
Why `const`? The parameter of all connected signals is `WalletModel* wallet_model`, right?

💬 GBKS commented on pull request "Wallet : Allow user to navigate options while encrypting at creation":
(https://github.com/bitcoin-core/gui/pull/722#discussion_r1149186485)
I think both are fine. Personally, I would go for "Back", as the word "Go" doesn't add any additional information.

💬 josibake commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1149193825)
This line was really confusing for me at first. Perhaps something like this could make it more clear?

```suggestion
Dbt prefix_key(prefix.data(), prefix.size()), prefix_value{};
```

💬 MarcoFalke commented on issue "depends does not compile with clang-16 (fontconfig)":
(https://github.com/bitcoin/bitcoin/issues/27299#issuecomment-1485045088)
Maybe fontconfig can also be worked around with something like `+$(package)_cflags += -Wno-int-conversion -Wno-implicit-function-declaration`?

💬 hernanmarino commented on pull request "depends: qrencode 4.1.1":
(https://github.com/bitcoin/bitcoin/pull/27312#issuecomment-1485058722)
> There is this library which is maintained and available in multiple programming languages (C and C++ are two of them): https://github.com/nayuki/QR-Code-generator

I can take a look and attempt migrating to it, unless you e want to work on it yourself, @prusnak

💬 prusnak commented on pull request "depends: qrencode 4.1.1":
(https://github.com/bitcoin/bitcoin/pull/27312#issuecomment-1485061751)
> I can take a look and attempt migrating to it, unless you e want to work on it yourself, @prusnak

Feel free to explore that area.

📝 fanquake reopened a pull request: "depends: fontconfig 2.14.2"
(https://github.com/bitcoin/bitcoin/pull/27301)
We need to bump this as the current version doesn't compile under `clang-16`, which is blocking upgrading sanitizer/fuzzing infrastructure (see #27298).

Untested. Need to double-check the gperf/patch dropping.