19:01:42 <meshcollider> #startmeeting
19:01:42 <lightningbot> Meeting started Fri Dec 14 19:01:42 2018 UTC.  The chair is meshcollider. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:01:42 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
19:01:49 <sipa> gwillen: i always assumed that it did
19:02:02 <jnewbery> feowertyne niht
19:02:04 <meshcollider> #bitcoin-core-dev Wallet Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier jl2012 achow101 meshcollider jnewbery maaku fanquake promag provoostenator aj Chris_Stewart_5 dongcarl gwillen jamesob ken281221 ryanofsky gleb
19:02:29 <meshcollider> Ok topics?
19:02:49 <kanzure> hi.
19:02:58 <jnewbery> high priority for review?
19:02:59 <sipa> yay, some stuff got merged
19:03:07 <instagibbs> yes yay
19:03:13 <provoostenator> Probably topics:
19:03:18 <jnewbery> #14565 is wallety
19:03:19 <provoostenator> 1. progress towards descriptor wallets
19:03:23 <gribble> https://github.com/bitcoin/bitcoin/issues/14565 | Overhaul importmulti logic by sipa · Pull Request #14565 · bitcoin/bitcoin · GitHub
19:03:26 <provoostenator> 2. progrress towards hardware wallets
19:03:46 <jnewbery> #11082 isn't wallety, but is blocking provoostenator's wallety PRs
19:03:49 <gribble> https://github.com/bitcoin/bitcoin/issues/11082 | Add new bitcoin_rw.conf file that is used for settings modified by this software itself by luke-jr · Pull Request #11082 · bitcoin/bitcoin · GitHub
19:04:07 <achow101> hi
19:04:30 <meshcollider> #topic high priority
19:04:30 <provoostenator> jnewbery: it's blocking my future walety PR's (once I start on the GUI side of hardware wallets)
19:05:18 <meshcollider> jnewbery: they are both already on the list right
19:05:36 <provoostenator> Correct
19:05:37 <jnewbery> 14565 looks good. It was missing tests, but now looks pretty well covered (thanks sipa!)
19:05:47 <jnewbery> yes, both there already: https://github.com/bitcoin/bitcoin/projects/8
19:06:10 <provoostenator> Once that's in, I would say #14491 become priority.
19:06:13 <meshcollider> Yes 14565 is very nearly ready I think
19:06:13 <gribble> https://github.com/bitcoin/bitcoin/issues/14491 | Allow descriptor imports with importmulti by MeshCollider · Pull Request #14491 · bitcoin/bitcoin · GitHub
19:06:36 <jnewbery> reminder that 14565 blocks PRs from meshcollider and achow101, so it'd be nice to move it towards merge
19:07:59 <meshcollider> Yes
19:08:08 <meshcollider> I had one last little question on there
19:09:23 <meshcollider> Is there anything else wallet related that should go onto high priority list now?
19:10:08 <meshcollider> #topic progress towards descriptor wallets (provoostenator)
19:10:33 * provoostenator looks as sipa
19:10:39 <provoostenator> *at
19:10:53 * sipa stares back
19:11:21 <instagibbs> various descriptor support, like in listunspent?
19:11:22 <meshcollider> sipa: have you thought more about it recently or been mostly focused on the PRNG stuff
19:11:53 <sipa> meshcollider: no, sorry - i've been busy with a few other projects
19:12:36 <meshcollider> that's fine of course :) I guess that is your update provoostenator
19:12:51 <meshcollider> There was an issue tracking which RPCs to add descriptor support to iirc
19:12:53 <provoostenator> Well, I'd like to know what's next, but we do have enough review work already I guess.
19:13:00 <sipa> next step is moving IsMine and related functions to be part of the wallet or some other abstraction, rather than free functions
19:13:14 <sipa> so they can later be extended to be descriptor based
19:13:20 <provoostenator> Ok
19:13:27 <provoostenator> Anything about the Keypool we can improve?
19:13:58 <meshcollider> I can take a stab at the ismine stuff
19:14:01 <sipa> that needs to be part of the same interface, i expect
19:14:05 <provoostenator> For example I'm a bit worried about #14075 interacting with the keypool from RPC code.
19:14:08 <sipa> but maybe a later step
19:14:08 <gribble> https://github.com/bitcoin/bitcoin/issues/14075 | Import watch only pubkeys to the keypool if private keys are disabled by achow101 · Pull Request #14075 · bitcoin/bitcoin · GitHub
19:14:39 <sipa> that looks like it may interact, indeed
19:14:57 <provoostenator> Though it can be refactored after that, it's nice if we at least have an idea of what the final thing needs to look like.
19:15:11 <meshcollider> So instead of the keypool being generated by a single descriptor, it will have imports in it too
19:15:48 <provoostenator> Right now the keypool, when it expands itself, makes strong assumptions about the wallet and just uses the HD structure.
19:16:14 <provoostenator> Whereas what we want probably is for the keypool (or something like it) to expand a specific descriptor.
19:16:27 <sipa> well there won't be a keypool anymore
19:16:43 <sipa> it's just a descriptor, which some entries cached, and some not (yet)
19:17:04 <sipa> the hard part is integrating the existing logic into such a structure
19:17:16 <bitcoin-git> [13bitcoin] 15MarcoFalke pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/b53573e5c6c7...6723d8e3a694
19:17:16 <bitcoin-git> 13bitcoin/06master 14fa30a0e 15MarcoFalke: test: mempool_persist: Verify prioritization is dumped correctly
19:17:17 <bitcoin-git> 13bitcoin/06master 146723d8e 15MarcoFalke: Merge #14931: test: mempool_persist: Verify prioritization is dumped correctly...
19:17:52 <bitcoin-git> [13bitcoin] 15MarcoFalke closed pull request #14931: test: mempool_persist: Verify prioritization is dumped correctly (06master...06Mf1812-testMempoolPrio) 02https://github.com/bitcoin/bitcoin/pull/14931
19:18:12 <provoostenator> Any prose or pseudo-code describing how such integration would work is most welcome (in a Github issue).
19:18:36 <sipa> i think we'll want to see the existing keypool/keys/... as a special "legacy" descriptor that doesn't really have a text representation
19:19:00 <meshcollider> sipa: to cache them, would you expand() them with the cache function during the existin topup function
19:19:20 <sipa> meshcollider: right, exactly
19:19:43 <provoostenator> Why wouldn't it have a text representation?
19:20:10 <sipa> provoostenator: well the text representation is the entire set of keys, pubkeys, scripts, hdpaths, ... that are currently stored in the wallet :)
19:20:26 <sipa> i guess you could dump it in hex or something
19:20:58 <provoostenator> A migration wizard should be able to, at least for standard wallets, turn that into a series of regular descriptors no?
19:21:15 <sipa> that may be possible, but i don't think that's the priority now
19:21:50 <meshcollider> It'd be quicker and safer to just move-only the code type of thing into legacy functions
19:21:56 <sipa> (because then you have to worry about all existing RPCs, and their effect on those descriptors)
19:22:11 <provoostenator> Right, just depends on what's easier in practice. Personally I suspect it'll be easier to make the wallet _only_ have descriptors, just from a writing tests point of view.
19:22:23 <sipa> i think it's not too much work to just have a legacy subsystem, and a new system - and a wallet can contain just one of them, or both
19:22:23 <bitcoin-git> [13bitcoin] 15MarcoFalke pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/6723d8e3a694...9133227298ad
19:22:24 <bitcoin-git> 13bitcoin/06master 14c84c2b8 15practicalswift: tests: Test for expected return values when calling functions returning a success code
19:22:24 <bitcoin-git> 13bitcoin/06master 149133227 15MarcoFalke: Merge #14935: tests: Test for expected return values when calling functions returning a success code...
19:22:33 <provoostenator> But the RPC thing could be a pain yes.
19:22:34 <sipa> maybe we even want to forbid mixing them in one wallet
19:23:06 <bitcoin-git> [13bitcoin] 15MarcoFalke closed pull request #14935: tests: Test for expected return values when calling functions returning a success code (06master...06test-return-values) 02https://github.com/bitcoin/bitcoin/pull/14935
19:23:07 <sipa> but forced migration may getting it accepted much harder
19:23:58 <sipa> so my idea is that every record is a descriptor + some metadata (like gap limit, whether it's change or no), plus some cached keys... and there can be a special "legacy" record that's just all the existing keypool/ismine logic
19:24:00 <phantomcircuit> the rw config is mostly because the qt stuff writes to random places for settings right?
19:24:14 <provoostenator> So then you might end up with two seperate wallet systems and a migration tool, where the old system only gets maintenance updates to be able to read from it.
19:24:24 <luke-jr> phantomcircuit: random? not really; just different from bitcoind
19:24:27 <sipa> provoostenator: maybe
19:24:40 <phantomcircuit> luke-jr, i mean it writes to reg on windows
19:24:45 <phantomcircuit> which is super annoying to change
19:24:52 <sipa> phantomcircuit: please stick to topic
19:24:55 <meshcollider> phantomcircuit: we are in a wallet meeting btw :)
19:26:04 <meshcollider> We could open an issue to discuss the alternative approaches, or just discuss which is easier as we start actually writing the code
19:26:09 <provoostenator> A new wallet subsystem might also let us cleanly long-term deprecate some wallet RPC methods and replace them with clean ones, that happen to support descriptors?
19:26:18 <sipa> provoostenator: yup
19:26:19 <provoostenator> And maybe move to Sqlite3 at the same time.
19:26:36 <sipa> i think that's completely orthogonal
19:26:49 <provoostenator> Could be, yes.
19:27:11 <luke-jr> phantomcircuit: I answered in #bitcoin fyi
19:28:05 <provoostenator> Anyway, we have some next actions now to continue progress, maybe next topic?
19:28:37 <meshcollider> #topic progress towards hardware wallets (provoostenator)
19:28:46 <jnewbery> IsMine moved from wallet to common here: https://github.com/bitcoin/bitcoin/commit/a25a4f5b04c3e045557e9e7e807b2af74ad75128 . Was that just because of the way the multisig and P2SH tests are constructed (ie could those tests just be rewritten)?
19:28:54 <provoostenator> Now that #14491 has been rebased, the `hww` branch I'm building off should also soon be rebased.
19:28:57 <gribble> https://github.com/bitcoin/bitcoin/issues/14491 | Allow descriptor imports with importmulti by MeshCollider · Pull Request #14491 · bitcoin/bitcoin · GitHub
19:29:02 <provoostenator> #14912
19:29:04 <gribble> https://github.com/bitcoin/bitcoin/issues/14912 | [WIP] External signer support (e.g. hardware wallet) by Sjors · Pull Request #14912 · bitcoin/bitcoin · GitHub
19:29:27 <provoostenator> I'll do some cleaning up of my ugly string concatenation descriptor code after rebase.
19:29:54 <provoostenator> People can already test it though. It compiles and actually works (at your own risk, so use testnet).
19:30:14 <meshcollider> jnewbery: looks like it?
19:30:47 <provoostenator> As in, you can create a new wallet, put read-only keys in it, show address on the device and spend coins. Using achow101's HWI library to talk to the device.
19:31:28 <provoostenator> So the idea is that users would download that library seperately and just launch bitcoind -signer=../HWI/hwi.py (or some other tool). That way we don't ahve to review individual hardware wallet code.
19:31:49 <meshcollider> provoostenator: cool \o/
19:32:30 <provoostenator> With very little code changes this could also work against a gRPC server, but there's some security trade-offs compared to calling commands. We talked about that a few weeks ago.
19:33:11 <provoostenator> I am trying to keep it generic enough to keep that possible, so e.g. the -signer= command could later also be a URL. But for now, it just executes a command and parses the JSON that command spits out to stdout.
19:34:01 <provoostenator> Next step for me is to work on GUI support for this. But there's already a pile of prerequisite stuff to review, so don't worry too much about that :-)
19:34:21 <provoostenator> I personally just like to see the big picture in action.
19:34:27 <meshcollider> Yes let's not stack too many PRs at once again ;)
19:35:47 <provoostenator> For GUI proof of concept I'd just like to nag promag about #13100, which we want anyway.
19:35:50 <gribble> https://github.com/bitcoin/bitcoin/issues/13100 | gui: Add dynamic wallets support by promag · Pull Request #13100 · bitcoin/bitcoin · GitHub
19:36:24 <provoostenator> (that's all I have)
19:37:52 <MarcoFalke> wget https://bitcointools.jonasschnelli.ch/data/builds/914/    ... failed: Connection refused
19:37:53 <meshcollider> that PR hasn't been rebased for 2 months, perhaps you'd like to take it up and rebased it yourself?
19:37:55 <MarcoFalke> ^ jonasschnelli
19:38:06 <provoostenator> He actually said he's working on it soon.
19:38:14 <meshcollider> Oh ok
19:38:48 <provoostenator> I think he needs this to go in first #14573, that's almost mergeable.
19:38:49 <meshcollider> I'm happy to review it as soon as its ready, its already tagged for 0.18
19:38:50 <gribble> https://github.com/bitcoin/bitcoin/issues/14573 | qt: Add Window menu by promag · Pull Request #14573 · bitcoin/bitcoin · GitHub
19:39:44 <meshcollider> Are there any other topics?
19:40:04 <provoostenator> luke-jr phantomcircuit did you want to discuss rw_config stuff?
19:40:31 <provoostenator> I noticed the rabase, so I'll rebase my Settings migration stuff as well.
19:40:47 <bitcoin-git> [13bitcoin] 15ch4ot1c opened pull request #14961: [docs] Root readme improvements (06master...06improvements/readme) 02https://github.com/bitcoin/bitcoin/pull/14961
19:40:56 <provoostenator> But it's not very wallety.
19:41:36 <meshcollider> Looks like that might be it for today
19:41:43 <meshcollider> Thanks provoostenator :)
19:41:51 <meshcollider> #endmeeting