💬 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.
(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)
(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?
(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 */
```
(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.
(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?
(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()};
```
(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
{
-
...
(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.
```
(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
...
(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)
(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?
(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
(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?
(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)
(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.
(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)
(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).
(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
(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);
```
?
(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);
```
?