💬 hebasto commented on pull request "build: Add CMake-based build system (1 of N)":
(https://github.com/bitcoin/bitcoin/pull/27060#issuecomment-1431827200)
@theuni

Thank you!

> Again, I suggest a staging repo for review. It could just be your personal repo where you PR to a branch. Review happens in chunks (as you're doing now) but to that branch, that way we don't have the temporary clutter in the master repo. Then, once all the chunks have been individually merged and reviewed, it is submitted as one giant pr to master where it can be tested in full before final merge. This is essentially the [Dictator and Lieutenants Workflow](https://git
...

💬 theuni commented on pull request "build: Add CMake-based build system (1 of N)":
(https://github.com/bitcoin/bitcoin/pull/27060#issuecomment-1431835177)
>
> Like [that](https://github.com/hebasto/bitcoin/pull/5)?

Exactly, thank you!

So, right, I'd like to request that everyone who has been involved in reviewing (technically) the other CMake PRs to come along with me to reviewing over at @hebasto's staging repo. Since CMake is apparently so urgent, I'm sure we'll have no trouble recruiting review over there :p

Maybe it won't end up working, but I think it's worth a shot. It's at least preferable to trying to do everything 1 PR here.

💬 achow101 commented on pull request "wallet: reuse change dest when re-creating TX with avoidpartialspends":
(https://github.com/bitcoin/bitcoin/pull/27053#issuecomment-1431846290)
ACK 14b4921a91920df25b19ff420bfe2bff8c56f71e

💬 furszy commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#discussion_r1107566345)
done

💬 furszy commented on pull request "refactor: wallet, remove global 'ArgsManager' dependency":
(https://github.com/bitcoin/bitcoin/pull/26889#issuecomment-1431851194)
Updated per feedback. Thanks.
Single line removal [diff](https://github.com/bitcoin/bitcoin/compare/fa1ce3dd62be739b579373287c5501d99fcf9c17..52f4d567d69425dfd514489079db80483024a80d).

💬 hebasto commented on pull request "build: Add CMake-based build system (1 of N)":
(https://github.com/bitcoin/bitcoin/pull/27060#issuecomment-1431855486)
> So, right, I'd like to request that everyone who has been involved in reviewing (technically) the other CMake PRs to come along with me to reviewing over at @hebasto's staging repo. Since CMake is apparently so urgent, I'm sure we'll have no trouble recruiting review over there :p

Just in case, I've made an announcement in #bitcoin-core-builds IRC channel.

💬 jonatack commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#discussion_r1107561322)
e1910f6743d73a extra newline at EOF

Unrelated, would it make sense to avoid a `CService` copy in the most frequent case?

```diff
CService MaybeFlipIPv6toCJDNS(const CService& service)
{
+ if (service.m_net != NET_IPV6 || service.m_addr[0] != CJDNS_PREFIX || !g_reachable_nets.Contains(NET_CJDNS)) return service;
CService ret{service};
- if (ret.m_net == NET_IPV6 && ret.m_addr[0] == CJDNS_PREFIX && g_reachable_nets.Contains(NET_CJDNS)) {
- ret.m_net = NET_CJDNS;
-
...

💬 ryanofsky commented on issue "build: multiprocess link issues on fedora aarch64":
(https://github.com/bitcoin/bitcoin/issues/26943#issuecomment-1431948144)
I don't understand why this would happen only for one platform, but based on the error message maybe the following change would fix it:

```diff
--- a/depends/packages/capnp.mk
+++ b/depends/packages/capnp.mk
@@ -11,6 +11,7 @@ $(package)_config_opts := --with-external-capnp
$(package)_config_opts += CAPNP="$$(native_capnp_prefixbin)/capnp"
$(package)_config_opts += CAPNP_CXX="$$(native_capnp_prefixbin)/capnp-c++"
$(package)_config_opts_android := --disable-shared
+$(package)_cxxflags
...

💬 pinheadmz commented on issue "Exposed / Compromised (must sweep) / Used / Imported (?) flags on addresses":
(https://github.com/bitcoin/bitcoin/issues/3314#issuecomment-1432007152)
I think this issue can be closed, 0.13.0 introduced HD wallets by default, and then recently descriptor wallets by default in 23.0.

💬 achow101 commented on issue "Exposed / Compromised (must sweep) / Used / Imported (?) flags on addresses":
(https://github.com/bitcoin/bitcoin/issues/3314#issuecomment-1432012553)
> I think this issue can be closed, 0.13.0 introduced HD wallets by default, and then recently descriptor wallets by default in 23.0.

I don't think HD wallets sufficiently resolves this issue. We can still import things, reuse addresses, have descriptors be compromised, etc. so having these annotations would still be useful.

💬 willcl-ark commented on issue "Exposed / Compromised (must sweep) / Used / Imported (?) flags on addresses":
(https://github.com/bitcoin/bitcoin/issues/3314#issuecomment-1432016546)
Do we still show addresses from the old SPKM(s) in the GUI after encrypting? My understanding was that they were no longer marked as `active` and so I assumed they would not show there (but I never tested it out).

💬 achow101 commented on issue "Exposed / Compromised (must sweep) / Used / Imported (?) flags on addresses":
(https://github.com/bitcoin/bitcoin/issues/3314#issuecomment-1432041881)
> Do we still show addresses from the old SPKM(s) in the GUI after encrypting?

Yes, the address book is a thing.

📝 jonatack opened a pull request: "net: remove orphaned CSubNet::SanityCheck()"
(https://github.com/bitcoin/bitcoin/pull/27106)
`CSubNet::SanityCheck()` was added in #20140, and not removed in #22570 when it became orphaned code.

Also, remove an out-of-date `snprintf` TODO that was resolved in #27036, and fix up 2 words to make the spelling linter green again.

💬 AdmiralNeo commented on issue "Stop the GPG verification madness":
(https://github.com/bitcoin/bitcoin/issues/25395#issuecomment-1432185897)
THX for your proposed fixes - you could add that the official download-page for Apple-version contains now already for several months an outdated signing key of the departed Wladimir van der Laan - no valid key is there to be found.

💬 sipa commented on issue "Stop the GPG verification madness":
(https://github.com/bitcoin/bitcoin/issues/25395#issuecomment-1432188474)
If you have questions about usage, see https://bitcoin.stackexchange.com.

If you have comments about the website, see https://github.com/bitcoin-core/bitcoincore.org

Please don't keep commenting in a closed issue about an unrelated topic.

💬 achow101 commented on pull request "refactor: Disable unused special members functions in `UnlockContext`":
(https://github.com/bitcoin-core/gui/pull/711#issuecomment-1432221567)
ACK 9fa43b5af6b180f4b5f76726f443ee60259d2cd0

👍 furszy approved a pull request: "wallet: SecureString to allow null characters"
(https://github.com/bitcoin/bitcoin/pull/27068)

🚀 achow101 merged a pull request: "refactor: Disable unused special members functions in `UnlockContext`"
(https://github.com/bitcoin-core/gui/pull/711)

💬 achow101 commented on pull request "prune, import: allow pruning to work during loadblock import":
(https://github.com/bitcoin/bitcoin/pull/24957#issuecomment-1432400864)
ACK 734355b470764c523ef08b25320a5fabb73358a6

💬 achow101 commented on pull request "prune, import: allow pruning to work during loadblock import":
(https://github.com/bitcoin/bitcoin/pull/24957#issuecomment-1432403217)
Looks like there's a silent merge conflict:

```
../../../src/validation.cpp: In member function ‘void Chainstate::LoadExternalBlockFile(FILE*, FlatFilePos*, std::multimap<uint256, FlatFilePos>*)’:
../../../src/validation.cpp:4518:21: error: ‘fPruneMode’ was not declared in this scope; did you mean ‘node::fPruneMode’?
4518 | if (fPruneMode && !fReindex && pblock) {
| ^~~~~~~~~~
| node::fPruneMode
In file included from .
...

💬 ponury1990 commented on pull request "Fix minor typo":
(https://github.com/bitcoin/bitcoin/pull/27102#issuecomment-1432652741)
Pierwszy raz w życiu to robię ja tylko chce zamienić dokumentacje wraz z pulą BTC jestem samouk z bardzo małym stażem