Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 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.
💬 hebasto commented on pull request "Remove confusing "Dust" label from coincontrol dialog":
(https://github.com/bitcoin-core/gui/pull/719#discussion_r1149227025)
Isn't Qt::AlignLeading a synonym for Qt::AlignLeft?
💬 hebasto commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1149235477)
Naming nit:
```suggestion
std::vector<std::string> parseHelper(const std::string& str_command);
```
💬 hebasto commented on pull request "Bugfix: Address broken things around Peers detail view":
(https://github.com/bitcoin-core/gui/pull/677#discussion_r1149249983)
> ping seen. nothing to do here though.

This PR modifies `src/net_processing.cpp`, and we review such changes in the main repo, generally.

Mind moving this PR or the first commit from this repo into the main one?
💬 TheCharlatan commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#discussion_r1149161983)
Nit: This is currently printing the line instructing the user to set `warningnoredconf=1` even when it is already set.
🚀 fanquake merged a pull request: "clang-tidy: Add more `performance-*` checks and related fixes"
(https://github.com/bitcoin/bitcoin/pull/26642)
💬 hebasto commented on pull request "[WIP] GUI: exception safe drop-in replacement for QTimer::singleShot":
(https://github.com/bitcoin-core/gui/pull/666#issuecomment-1485109886)
Closing this due to lack of activity. Feel free to reopen.
hebasto closed a pull request: "[WIP] GUI: exception safe drop-in replacement for QTimer::singleShot"
(https://github.com/bitcoin-core/gui/pull/666)
🚀 fanquake merged a pull request: "guix: import/sync python-lief (0.12.3) package definition from upstream"
(https://github.com/bitcoin/bitcoin/pull/27296)
⚠️ fanquake opened an issue: "-Wmaybe-uninitialized warnings under LTO"
(https://github.com/bitcoin/bitcoin/issues/27343)
Building under GCC `13.0.1` with `-flto` (Fedora Rawhide) currently results in a number of `-Wmaybe-uninitialized` warnings. Some of these are things we might not want to "fix", i.e in test code. Some come from leveldb etc. Open this issue to document them, as some exist with current versions of GCC, and GCC 13.0 will be released soon:

```bash
In member function 'operator=',
inlined from 'Seed.constprop' at test/util/setup_common.cpp:91:33:
random.cpp:687:19: warning: 'D.19516.bitbuf'
...
💬 fanquake commented on pull request "fix initialization in FastRandomContext: removes undefined behavior & uninitialized read":
(https://github.com/bitcoin/bitcoin/pull/23169#issuecomment-1485121825)
> is not relevant any more / was fixed (as far as I can say)

There are still some `-Wmaybe-uninitialized` warnings produced with GCC 13.0.1 and `-flto`, some in random.cpp. Opened #27343 to document them + others.
💬 rajarshimaitra commented on pull request "rpc, wallet: add ability to retrieve all address book entries":
(https://github.com/bitcoin/bitcoin/pull/26174#issuecomment-1485139620)
Concept ACK. This is useful for various situations where a bunch of imported spks needs to be retrieved from the wallet. Maybe to compare with an external source and check if any new spks need to be imported into the watchlist. Previously it could be done by making redundant labels just for this call, but having this option to dump everything seems useful.

I will go through the code changes in more detail. Adding one simple, functional test to check the behavior will be helpful.
🚀 fanquake merged a pull request: "guix: combine and document `enable_werror`"
(https://github.com/bitcoin/bitcoin/pull/27326)