< 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?
< gribble> https://github.com/bitcoin/bitcoin/issues/19137 | wallettool: Add dump and createfromdump commands by achow101 · Pull Request #19137 · bitcoin/bitcoin · GitHub
< 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)
< gribble> https://github.com/bitcoin/bitcoin/issues/20160 | Proposed Timeline for Legacy Wallet and BDB removal · Issue #20160 · bitcoin/bitcoin · GitHub
< sipa> achow101: if there is a way to convert legacy to descriptor, sure :)
< achow101> it'd be a lot easier if there was some way to test whether a set of scripts exactly matches everything IsMine will match on
< sipa> all you need is a way to generate a strict superset of that
< sipa> and then run IsMine on all elements of the superset in both
< achow101> how does one make this superset?
< sipa> go over all keys, scripts, pubkeys, scripthashes, and do everything you can with all of them
< sipa> P2PK, P2PKH, P2WPKH, P2SH-P2WPKH for all pubkeys/script, raw/P2SH/P2WSH for all scripts, P2SH/P2WSH for all scripthashes, ...
< achow101> have you seem #19602
< gribble> https://github.com/bitcoin/bitcoin/issues/19602 | wallet: Migrate legacy wallets to descriptor wallets by achow101 · Pull Request #19602 · bitcoin/bitcoin · GitHub
< 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?
< hebasto> jonasschnelli: mind looking into https://github.com/users/hebasto/projects/5 ?
< jonasschnelli> hebasto: yes. will do. Thanks for pointing
< hebasto> jonasschnelli: thanks!
< jonasschnelli> hebasto: with https://github.com/bitcoin-core/gui/pull/59, the calls are still executed synchronously?
< 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
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/f5bd46a4cc6d...0f86e7f1285c
< bitcoin-git> bitcoin/master 2d5793c Luke Dashjr: Bugfix: chainparams: Add missing (disabled) Taproot deployment for Signet
< bitcoin-git> bitcoin/master 0f86e7f MarcoFalke: Merge #20157: Bugfix: chainparams: Add missing (always enabled) Taproot de...
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #20157: Bugfix: chainparams: Add missing (always enabled) Taproot deployment for Signet (master...signet_taproot_fix) https://github.com/bitcoin/bitcoin/pull/20157
< jonasschnelli> hebasto: I just tested https://github.com/bitcoin-core/gui/pull/59 and it still locks the GUI thread if I call generatetoaddress(1000, getnewaddress())
< hebasto> jonasschnelli: during shutdown?
< jonasschnelli> it's great how the current master is GUI lock free even during heavy sync
< jonasschnelli> hebasto: no... I though it does not lock the GUI? Or does it just allow shutting down during a RPC operation?
< jonasschnelli> hebasto: I also can't shut down during generatetoaddress(1000, getnewaddress())
< 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] MarcoFalke pushed 4 commits to master: https://github.com/bitcoin/bitcoin/compare/0f86e7f1285c...47fc883106fb
< bitcoin-git> bitcoin/master be38684 Elliott Jin: test: Replace use of (dis)?connect_nodes globals
< bitcoin-git> bitcoin/master 4b16c61 Prayank: scripted-diff: test: Replace uses of (dis)?connect_nodes global
< bitcoin-git> bitcoin/master 3c7d9ab Elliott Jin: test: Move (dis)?connect_nodes globals into TestFramework as helpers
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #19967: test: Replace (dis)?connect_nodes globals with TestFramework methods (master...connect-nodes) https://github.com/bitcoin/bitcoin/pull/19967
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/47fc883106fb...b46f37ba5ec4
< 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
< MarcoFalke> If you want to merge pull from the gui repo, you'll have to update to https://github.com/bitcoin-core/bitcoin-maintainer-tools/commit/4ac57664c899fbfb138229361ca444c9e832871e
< jonasschnelli> MarcoFalke: thanks! I think I got it now. :)
< MarcoFalke> jonasschnelli: and the ci failure in https://bitcoinbuilds.org/index.php?ansilog=25c20237-3134-4099-9b5a-d618161eae99.log#l5931 is due to RPC timeout. You can use --timeout-factor in your config or make a pull to bump the self.rpc_timeout in the source code
< jonasschnelli> MarcoFalke: Okay. Thanks. Let me set this.
< jonasschnelli> MarcoFalke --timeout-factor of 2 seems to fix it
< MarcoFalke> nice
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #20214: test: Fix rpc_net intermittent issue (master...2010-testFixNet) https://github.com/bitcoin/bitcoin/pull/20214
< bitcoin-git> [bitcoin] sushant-varanasi opened pull request #20215: Travis CI system link (https://travis-ci.org/) added for clarity (master...patch-1) https://github.com/bitcoin/bitcoin/pull/20215
< bitcoin-git> [bitcoin] ryanofsky closed pull request #19793: bitcoin-wallet salvage: Return false instead of asserting when a loaded tx isn't new (master...pr/badsalv) https://github.com/bitcoin/bitcoin/pull/19793