💬 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
(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
(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).
(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.
(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;
-
...
(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
...
(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.
(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.
(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).
(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.
(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.
(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.
(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.
(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
(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)
(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)
(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
(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 .
...
(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
(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
💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#issuecomment-1432702025)
Just thinking aloud:
1. Subnetting does not make sense for tor/i2p/cjdns
2. `BanMan` entries are always subnets (`CSubNet` objects), even for single host bans
It follows that banman is doing something that does not make sense (e.g. subnetting tor). This is the root of the problem. I think if that is eradicated, then the rest will untangle by itself.
What this means - store two ban maps in `BanMan` - one for single hosts and one for subnets (only for IPv[46] subnets). This way also look
...
(https://github.com/bitcoin/bitcoin/pull/27071#issuecomment-1432702025)
Just thinking aloud:
1. Subnetting does not make sense for tor/i2p/cjdns
2. `BanMan` entries are always subnets (`CSubNet` objects), even for single host bans
It follows that banman is doing something that does not make sense (e.g. subnetting tor). This is the root of the problem. I think if that is eradicated, then the rest will untangle by itself.
What this means - store two ban maps in `BanMan` - one for single hosts and one for subnets (only for IPv[46] subnets). This way also look
...