Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 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.
👍 hernanmarino approved a pull request: "ci: cleanup of CI_EXEC & CI_EXEC_ROOT"
(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)
🚀 fanquake merged a pull request: "test: wallet_create_tx.py fix race"
(https://github.com/bitcoin/bitcoin/pull/27318)
fanquake closed an issue: "test: `wallet_importdescriptors.py --descriptors` failure"
(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.
💬 fanquake commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1487030103)
@brunoerg are you following up here?
💬 MarcoFalke commented on issue "index: ThreadSanitizer: data race on vptr ":
(https://github.com/bitcoin/bitcoin/issues/27355#issuecomment-1487049094)
Reproducible? If yes, what are the steps to reproduce?
💬 fanquake commented on issue "index: ThreadSanitizer: data race on vptr ":
(https://github.com/bitcoin/bitcoin/issues/27355#issuecomment-1487053086)
> Reproducible

100%. Running `time FILE_ENV="./ci/test/00_setup_env_native_tsan.sh" ./ci/test_run_all.sh` on a aarch64 machine running Fedora 37.
💬 pinheadmz commented on pull request "system: allow GUI to initialize default data dir":
(https://github.com/bitcoin/bitcoin/pull/27273#issuecomment-1487075138)
Closing for now, `strDataDir` is a bit annoying and we should consider getting rid of it.
pinheadmz closed a pull request: "system: allow GUI to initialize default data dir"
(https://github.com/bitcoin/bitcoin/pull/27273)
⚠️ CQuerini247 opened an issue: "Bitcoin"
(https://github.com/bitcoin/bitcoin/issues/27356)
When I put money into Bitcoin and there's tabs with more money than I get back what do I do?
💬 theStack commented on pull request "test: use address_to_scriptpubkey instead of RPC call":
(https://github.com/bitcoin/bitcoin/pull/27349#discussion_r1150776577)
stylistic nit: we usually keep two blank lines after imports, so should rather add another one than remove one here (see also https://peps.python.org/pep-0008/#blank-lines).