💬 MarcoFalke commented on pull request "ci: Nuke Android APK task, Use credits for tsan":
(https://github.com/bitcoin/bitcoin/pull/27834#issuecomment-1582276308)
Another failure on a doc change: https://cirrus-ci.com/task/5213373952950272?logs=ci#L1404


```
CXX qt/libbitcoinqt_a-qrc_bitcoin.o
CXX qt/libbitcoinqt_a-qrc_bitcoin_locale.o
AR libbitcoin_node.a
AR libbitcoin_util.a
AR libbitcoin_wallet.a
AR libbitcoin_zmq.a
AR libbitcoin_cli.a
AR libbitcoin_common.a
AR libbitcoin_consensus.a
CXXLD crypto/libbitcoin_crypto_base.la
CXXLD crypto/libbitcoin_crypto_arm_sh
...

💬 dergoegge commented on pull request "ci: Nuke Android APK task, Use credits for tsan":
(https://github.com/bitcoin/bitcoin/pull/27834#issuecomment-1582306388)
> can you show an example to back up this claim: "I have only ever seen this task pass (even if all others fail)..."

@jarolrod I can't find the PR this happened on and I've only seen that one time but I swear I'm not making this up. The macOS task has other build flags, so it must have been some weird combination of that and tests/lint failing but some builds succeeding.

👍 dergoegge approved a pull request: "net, refactor: net_processing, add `ProcessCompactBlockTxns`"
(https://github.com/bitcoin/bitcoin/pull/26969#pullrequestreview-1469548500)
Code review ACK 686629d2be5545ef59cf0e97f4f3a74c6cde2efa

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1222817343)
Done

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1222817552)
Done

💬 dergoegge commented on pull request "[DEMO] Integrate `bitcoin-tidy` clang-tidy plugin - warn on uninitialized members of aggregate types":
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1582320665)
@theuni maybe the `\n` check for logging? Also suggested in https://github.com/bitcoin/bitcoin/issues/27825

💬 achow101 commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1582321920)
> My proposed fix is to disallow upgrade procedure for wallets created with master by setting `WALLET_FLAG_GLOBAL_HD_KEY` for all newborn wallets. Later we want to add a separate method to change HD key anyway to cover that use case.

I thought this was doing this already, but it turns out that we don't set the descriptor flag until after the upgrade stuff is run for creation, so it wasn't being set. I've added a commit which will set the flag for all new wallets.

💬 MarcoFalke commented on pull request "Remove confusing "Dust" label from coincontrol dialog":
(https://github.com/bitcoin-core/gui/pull/719#issuecomment-1582357495)
Concept ACK

💬 achow101 commented on pull request "wallet, tests: Expand and test when the blank wallet flag should be un/set":
(https://github.com/bitcoin/bitcoin/pull/25634#discussion_r1222883957)
I've added a commit with this comment.

💬 achow101 commented on pull request "wallet, tests: Expand and test when the blank wallet flag should be un/set":
(https://github.com/bitcoin/bitcoin/pull/25634#discussion_r1222884969)
Done as suggested

💬 achow101 commented on pull request "wallet, tests: Expand and test when the blank wallet flag should be un/set":
(https://github.com/bitcoin/bitcoin/pull/25634#discussion_r1222885500)
It's tested in the new test

💬 achow101 commented on pull request "wallet, tests: Expand and test when the blank wallet flag should be un/set":
(https://github.com/bitcoin/bitcoin/pull/25634#discussion_r1222914050)
I've split it up so that tests are introduced in the commits that fix the issues.

💬 achow101 commented on pull request "wallet: Migrate wallets that are not in a wallet dir":
(https://github.com/bitcoin/bitcoin/pull/26740#discussion_r1222919617)
For wallets that are not in a wallet dir, the db's filename will be the name of the wallet itself.

💬 vasild commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1222985986)
yes, that would be unnecessary

💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1222989539)
Thank you, I updated it.

💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1222991211)
I modify as you suggested 👍🏾

💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1222991573)
Thank you

💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1222991937)
sure fixed

💬 vasild commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1582537371)
On treating IPv4 and IPv6 as one network - I am ok either way. I see the merit in amiti's considerations above, but also on @ajtowns's. One more thing to consider - for an attacker it is not too difficult to get IPv6 connectivity but that is also the case for Tor. I mean it is not too difficult to get Tor connectivity either, but that is still _some_ extra effort.

💬 vasild commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1223007090)
Ok, I am just checking because elsewhere the intention is to treat clearnet as one network and this code does not do that. I do not have a strong opinion either way.