💬 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(),
```
(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
(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?
(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
(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
(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
...
(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
(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_
...
(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:
...
(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.
(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
...
(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
(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"
(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 ?
(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.
(https://github.com/bitcoin-core/gui/pull/722#issuecomment-1486987682)
Updated the code as per @hebasto suggestions.
👍 hernanmarino approved a pull request: "ci: cleanup of CI_EXEC & CI_EXEC_ROOT"
(https://github.com/bitcoin/bitcoin/pull/27333)
untested ACK b5ef1419ece77db77cc196eb541f99a72360a607. LGTM
(https://github.com/bitcoin/bitcoin/pull/27333)
untested ACK b5ef1419ece77db77cc196eb541f99a72360a607. LGTM
✅ fanquake closed an issue: "wallet_create_tx.py "Not solvable pre-selected input" exception"
(https://github.com/bitcoin/bitcoin/issues/27316)
(https://github.com/bitcoin/bitcoin/issues/27316)
🚀 fanquake merged a pull request: "test: wallet_create_tx.py fix race"
(https://github.com/bitcoin/bitcoin/pull/27318)
(https://github.com/bitcoin/bitcoin/pull/27318)
✅ fanquake closed an issue: "test: `wallet_importdescriptors.py --descriptors` failure"
(https://github.com/bitcoin/bitcoin/issues/27282)
(https://github.com/bitcoin/bitcoin/issues/27282)
💬 fanquake commented on issue "test: `wallet_importdescriptors.py --descriptors` failure":
(https://github.com/bitcoin/bitcoin/issues/27282#issuecomment-1487025612)
> If yes, what are the steps to reproduce?
Haven't seen this lately. Can close.
(https://github.com/bitcoin/bitcoin/issues/27282#issuecomment-1487025612)
> If yes, what are the steps to reproduce?
Haven't seen this lately. Can close.