👍 hebasto approved a pull request: "guix: use GCC tool wrappers"
(https://github.com/bitcoin/bitcoin/pull/27345)
ACK 4133c8104f522c403c55d26bd03436a8149ff106

Guix builds:
```
df90584fe5c1c68ec6a547ba04b81b66fae61035d5d4b586bc2c70f19176bc62 guix-build-4133c8104f52/output/aarch64-linux-gnu/SHA256SUMS.part
ba5553f64f73d84850ffe4e2c1712bd803c6e6f13b9f57cbc82eecab8e9e82c1 guix-build-4133c8104f52/output/aarch64-linux-gnu/bitcoin-4133c8104f52-aarch64-linux-gnu-debug.tar.gz
316e07c7baae2ff35e929c25d472ef0b66077b275c2c55f4b24acdd015947e80 guix-build-4133c8104f52/output/aarch64-linux-gnu/bitcoin-4133c8104f
...

🚀 fanquake merged a pull request: "ci: Use TSan new runtime (llvm-16, take 3)"
(https://github.com/bitcoin/bitcoin/pull/27298)

💬 ismaelsadeeq commented on pull request "test: use address_to_scriptpubkey instead of RPC call":
(https://github.com/bitcoin/bitcoin/pull/27349#issuecomment-1486878414)
> ACK [c73a065](https://github.com/bitcoin/bitcoin/commit/c73a0658c2c5c43d4ba2d8dbf2af71abe84b40e8)
>
> Nice to see a standard function for getting the `scriptPubKey` instead of a mix of RPC calls. Removing the RPC call _should_ make the tests run faster, but considering it wasn't used in many places, I doubt it makes a material difference. Left two non-blocking nits.
>
> I also noticed the CI was failing, but when I rebased on master and ran the failing functional test locally, everything
...

💬 josibake commented on pull request "wallet: add `seeds` argument to `importdescriptors`":
(https://github.com/bitcoin/bitcoin/pull/27351#issuecomment-1486913782)
Concept ACK

Regarding the approach, my first thought was why not extend `sethdseed` to support BIP93, but I'm guessing this doesn't work out of the box per your findings in https://github.com/bitcoin/bitcoin/pull/27337 ?

💬 josibake commented on pull request "test: use address_to_scriptpubkey instead of RPC call":
(https://github.com/bitcoin/bitcoin/pull/27349#issuecomment-1486923482)
ACK https://github.com/bitcoin/bitcoin/pull/27349/commits/01e0a1679fb1deeae4daf674730de2e1a270139e

thanks for taking the suggestions! CI failure seems unrelated from what I can tell :shrug:

💬 josibake commented on pull request "test: use address_to_scriptpubkey instead of RPC call":
(https://github.com/bitcoin/bitcoin/pull/27349#discussion_r1150643887)
nit: should be a space here

```suggestion
ms_unlock_details = {"scriptPubKey": address_to_scriptpubkey(self.ms_address).hex(),
```

💬 ismaelsadeeq commented on pull request "test: use address_to_scriptpubkey instead of RPC call":
(https://github.com/bitcoin/bitcoin/pull/27349#discussion_r1150647051)
Thanks

💬 MarcoFalke commented on issue "test: `wallet_importdescriptors.py --descriptors` failure":
(https://github.com/bitcoin/bitcoin/issues/27282#issuecomment-1486934274)
Is this still an issue with a recent version of Bitcoin Core? If yes, what are the steps to reproduce?

💬 RandyMcMillan commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#issuecomment-1486945327)
Concept ACK

💬 josibake commented on pull request "test: wallet_create_tx.py fix race":
(https://github.com/bitcoin/bitcoin/pull/27318#issuecomment-1486945822)
ACK https://github.com/bitcoin/bitcoin/pull/27318/commits/8aab5157c55249c9023ae4e9654f5d42aaa4f314

💬 apoelstra commented on pull request "wallet: add `seeds` argument to `importdescriptors`":
(https://github.com/bitcoin/bitcoin/pull/27351#issuecomment-1486947079)
@josibake Correct. Let me give a quick overview of alternate approaches, which I realize I forgot to do:

* Extend the existing `sethdseed` RPC. This RPC is legacy-only and not supported by descriptor wallets, because it depends on the legacy "bag of keys" model where you could stick keys into the wallet and it'd "just work", often in surprising ways.
* Add a new descriptor-wallet-based `importseed` RPC or something. This doesn't work for more-or-less the same reason; the descriptor wallet as
...

💬 josibake commented on issue "wallet_create_tx.py "Not solvable pre-selected input" exception":
(https://github.com/bitcoin/bitcoin/issues/27316#issuecomment-1486956388)
seen here: https://cirrus-ci.com/task/5086775773757440

💬 ryanofsky commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#issuecomment-1486957262)
@furszy, thanks for the thorough review. I will start implementing your suggestions, but do you think you could help me with your comments about encapsulation like https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1150541159?

The comments are just a little vague from my perspective so it'd be great if you could provide specific code suggestions and I will apply or implement them. For background, I've been a little detached from the address encapsulation attempt, and personally like m_
...

⚠️ fanquake opened an issue: "index: ThreadSanitizer: data race on vptr "
(https://github.com/bitcoin/bitcoin/issues/27355)
Seen with master @ 220008604f15d5078092dea28be3e3f7f11b6c8f on aarch64. Follow up from #27298. See also #26188.

```bash
2023-03-28T13:52:47.882295Z (mocktime: 2020-08-31T15:34:12Z) [test] [dbwrapper.cpp:158] [CDBWrapper] Opened LevelDB successfully
2023-03-28T13:52:47.882390Z (mocktime: 2020-08-31T15:34:12Z) [test] [dbwrapper.cpp:183] [CDBWrapper] Using obfuscation key for /tmp/test_common_Bitcoin Core/0959fba80d599fe246b0b5ced04b4d1ecd504db9812edbff4ada2d88c6b218ae/regtest/indexes/txindex:
...

💬 RandyMcMillan commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#issuecomment-1486965158)
A note on the merging idea:

There seems to be a lot of potential issues when "merging" configs that use blocknotify/walletnotify/alertnotify - i would lean toward a clean override - mitigating user over sight of any conflicting automation.

💬 josibake commented on pull request "wallet: add `seeds` argument to `importdescriptors`":
(https://github.com/bitcoin/bitcoin/pull/27351#issuecomment-1486973748)
> This RPC (`sethdseed`) is legacy-only and not supported by descriptor wallets

Great point, I had forgotten about this

> for the reasons stated in the PR description -- I like my descriptors to be public, and my secret data to be limited to 16 bytes, and I think this is a common mode of operation for people coming from other wallets

This is a very compelling argument, and your approach seems like the path of least resistance towards supporting this use case.

> the descriptor wallet
...

👍 theStack approved a pull request: "test: wallet_create_tx.py fix race"
(https://github.com/bitcoin/bitcoin/pull/27318)
ACK 8aab5157c55249c9023ae4e9654f5d42aaa4f314

💬 hernanmarino commented on pull request "Wallet : Allow user to navigate options while encrypting at creation":
(https://github.com/bitcoin-core/gui/pull/722#discussion_r1150694189)
Changed to "Back"

💬 hernanmarino commented on pull request "Wallet : Allow user to navigate options while encrypting at creation":
(https://github.com/bitcoin-core/gui/pull/722#discussion_r1150695953)
I changed the code to reuse already existing strings, can you confirm it's okey in this new way ?

💬 hernanmarino commented on pull request "Wallet : Allow user to navigate options while encrypting at creation":
(https://github.com/bitcoin-core/gui/pull/722#issuecomment-1486987682)
Updated the code as per @hebasto suggestions.