< S3RK>
achow101: wrt #19137 what is the value in allowing users to create legacy-sqlite and descriptor-bdb? You mentioned that you want to tie descriptor wallets to sqlite, so why not go straight to new migratewallet command?
< achow101>
S3RK: it's a concession to those who want those but not an outright easy to use kind of thing
< achow101>
also because those commands are useful for some debugging stuff and I'd hate to have to put any actual wallet application logic on those commands
< S3RK>
hm.. I see that it could be useful for debugging. But what are the reasons for people to want those "non-standard" options?
< S3RK>
just curious
< achow101>
some people might want to make a legacy-sqlite because descriptor wallets are different enough that it has different behavior that they are concerned the migration will miss
< achow101>
I don't think descriptor-bdb will be something that people will want
< sipa>
agree
< sipa>
i may convert my bdb-legacy wallets to sqlite-legacy though, if that's somewhat supported
< achow101>
but also anyone who has made a descriptor wallet with master since the pr was merged will have descriptor-bdb and may want to make them descriptor-sqlite
< achow101>
sipa: it should work, but will be eventually unsupported (see #20160)
< sipa>
well you'll want to call old.IsMine() on each generated candidate to determine whether to import it in new
< sipa>
with more involved logic you can avoid that, but that'd mean duplication some of the IsMine logic
< achow101>
I think one of the other commits does that
< achow101>
the problem is moreso that a scriptPubKey is missing rather than a generated scriptPubKey is not IsMine
< sipa>
right
< achow101>
and that requires convincing yourself that your understanding of IsMine is correct
< sipa>
i think it's doable to get reasonable confidence in an ability to generate a superset
< sipa>
go over all standard script templates, and construct all of them with all the elements you have
< achow101>
I did that... and still missed something
< sipa>
anyway, no urgency for that, i think
< sipa>
ah, what did you miss?
< Bullit>
about 17000 comments
< achow101>
I think it was something about watchonly
< achow101>
might've been the thing where a multisig is not our unless we have all of the keys
< S3RK>
achow101: thanks for the explanation. Could you look at #20153 ? If I didn't miss anything than it could discourage people by breaking their descriptor wallets.
< gribble>
https://github.com/bitcoin/bitcoin/issues/20153 | wallet: do not import a descriptor with hardened derivations into a watch-only wallet by S3RK · Pull Request #20153 · bitcoin/bitcoin · GitHub
< vasild>
Just a thought - assuming we have support for I2P and the node has connections only via Tor or I2P (no clearnet IPv4 or IPv6 connections). Should the Tor icon be displayed then? The I2P icon?
< hebasto>
vasild: idk :)
< vasild>
me neither :)
< jonasschnelli>
MarcoFalke: are you going to open a GUI PR on the main repo with changes from the GUI repository?
< jonasschnelli>
Example: when I call generatetoaddress(100), it will not run in the background in the way that someone can execute another RPC call during the execution time of generatetoaddress?
< hebasto>
jonasschnelli: yes
< jonasschnelli>
I hope and guess it will just run outside of the GUI thread to allow someone to perform other tasks (looking at transactions etc.)
< jonasschnelli>
okay... will test
< hebasto>
jonasschnelli: RPC is executed in RPCConsole::executorThread
< hebasto>
that behaviour is nor changed
< jonasschnelli>
ack
< hebasto>
s/nor/not/
< bitcoin-git>
[bitcoin] jonatack opened pull request #20210: net: ensure CNode::m_inbound_onion is inbound, add getter, unit tests (master...CNode-IsInboundOnion) https://github.com/bitcoin/bitcoin/pull/20210
< fanquake>
jonasschnelli: there shouldn't be a need for any PRs from the GUI repo, the two repos should be staying in sync.
< fanquake>
Although there are a couple maintainers who aren't pushing to the gui repo when they push to the main repo yet. Just need to update their merge script.
< jonasschnelli>
Thanks fanquake... I think I finally got it.
< jonasschnelli>
fanquake: I guess githubmerge.pushmirrors must be set? right?
< fanquake>
jonasschnelli: correct. Mine is "git@github.com:bitcoin-core/gui.git"
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #20211: Use -Wswitch for TxoutType where possible (master...2010-WswitchTxoutType) https://github.com/bitcoin/bitcoin/pull/20211
< hebasto>
that pull does not interrupt rpc thread; it prevents blocking the gui thread, and show the final window properly
< hebasto>
*during shutdown
< bitcoin-git>
[bitcoin] vasild opened pull request #20212: net: fix output of peer address in version message (master...fix_version_message_print) https://github.com/bitcoin/bitcoin/pull/20212
< bitcoin-git>
bitcoin/master fa4074b Jonas Schnelli: Show name, format and if uses descriptors in bitcoin-wallet tool
< bitcoin-git>
bitcoin/master b46f37b MarcoFalke: Merge #20198: Show name, format and if uses descriptors in bitcoin-wallet ...
< bitcoin-git>
[bitcoin] MarcoFalke merged pull request #20198: Show name, format and if uses descriptors in bitcoin-wallet tool (master...2020/10/wallet_tool_sqlite) https://github.com/bitcoin/bitcoin/pull/20198
< MarcoFalke>
jonasschnelli: The gui repo is a monotree, so it is equal to the main repo and doesn't need to be merged back/forth
< MarcoFalke>
Oh, I see fanquake told you so already