19:00:21 <meshcollider> #startmeeting
19:00:21 <lightningbot> Meeting started Fri Jun 21 19:00:21 2019 UTC.  The chair is meshcollider. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:00:21 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
19:00:38 <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
19:01:01 <kanzure> hi
19:01:05 <achow101> hi
19:02:13 <achow101> something I mentioned yesterday that we could discuss here: "I've been working on implementing the scriptpubkeymanager  and I noticed a bunch of things related to key generation rely on the wallet version and wallet flags. Thoughts on how to handle those without introducing a circular dependency?"
19:03:02 <achow101> sipa's suggestion was to have flags and version be part of the constructors for SPKmanagers, but these things get updated during key generation and imports
19:03:40 <achow101> I tried having the spkmanager write out the updates to the wallet file, but then the parent cwallet instance wouldn't know about the flag changes. it also felt like a layer violation
19:03:45 <achow101> any thoughts on that?
19:04:23 <meshcollider> #topic scriptpubkeymanager wallet flags (achow101)
19:05:50 <kanzure> managers are assigned a key, or managers make keys?
19:06:01 <achow101> managers make keys
19:06:38 <meshcollider> Can the wallet pass a pointer on construction instead?
19:07:01 <achow101> yes, but locks..
19:07:23 <achow101> IIRC cs_wallet needs to be locked to modify flags but spkmanagers won't have access to cs_wallet
19:09:18 <achow101> FWIW all of the wallet flags are things that spkmanagers care about, not the wallet. so maybe it does make sense to just have the wallet's flag stuff call through to the spkmanager's?
19:09:19 <meshcollider> Hmm, pass a callback function down from the wallet then?
19:10:07 <instagibbs> which things/flags get updated on import, just to job my memory
19:10:23 <meshcollider> Which SPKManager would provide the flags then
19:11:14 <achow101> instagibbs: WALLET_FLAG_BLANK_WALLET changes on import and key generation. WALLET_FLAG_KEY_ORIGIN_METADATA can change on load. WALLET_FLAG_DISABLE_PRIVATE_KEYS is checked before keys are generated
19:11:45 <achow101> meshcollider: I think they would all have the same flags, so it doesn't matter. but I guess there's an update problem with that too..
19:12:08 <achow101> the only flag that SPKManagers won't use is WALLET_FLAG_AVOID_REUSE
19:13:12 <achow101> my initial thought was to have each spkmanager also store a pointer to it's parent cwallet, but that's circular
19:14:14 <meshcollider> I guess BLANK_ and KEY_ORIGIN_ can be per-spkmanager and the wallet can itetate over them all with OR
19:14:44 <meshcollider> Iterate*
19:15:25 <meshcollider> Whereas DISABLE_PRIV doesn't change does it
19:15:37 <achow101> it doesn't
19:17:06 <meshcollider> Am I right in thinking these flags only ever get set, never reset
19:17:15 <meshcollider> So you can just OR them all
19:17:18 <achow101> yeah
19:17:21 <meshcollider> bitwise
19:17:31 <achow101> so should there be a new wallet record for spkmanager flags?
19:19:13 <achow101> I don't think that approach will really work though since there won't necessarily be a way to uniquely identify spkmanagers
19:19:29 <achow101> the whole loading part is kind of an issue
19:21:29 <meshcollider> Just because one SPKManager changes a flag doesn't mean they all need to update their flags do they? It doesn't matter if they all get the same flags at reload?
19:21:51 <meshcollider> As long as whichever one updates can write it to the wallet file
19:23:34 <achow101> I think so?
19:28:33 <achow101> so i guess the conclusion is that spkmanagers will handle the storage of BLANK, KEY_ORIGIN, an DISABLE_PRIV flags and CWallet handles the storage of AVOID_REUSE
19:29:25 <meshcollider> seems sensible
19:29:33 <achow101> i can foresee some concurrency issues with this
19:30:43 <meshcollider> Hmm I don't really see why there needs to be concurrency issues if they're not being used to communicate with each other at runtime
19:32:01 <meshcollider> Are there any other topics or is this the only one? Just so we aren't holding up the meeting if we continue discussing this
19:32:43 <meshcollider> Doesn't seem like attendance is very high today so I assume this is it :)
19:33:14 <achow101> the flags between cwallet and each spkmanager can be out of sync, so the update of flags on one object can undo an update from another one
19:34:19 <meshcollider> But like I said earlier, isn't each flag only one-way
19:34:39 <achow101> e.g. cwallet has BLANK set. spkmanager unsets BLANK because a seed is set so it writes that to the file. CWallet still has BLANK set. it updates the flags because AVOID_REUSE is changed so now the old BLANK is written back to the wallet
19:35:26 <achow101> not so much concurrency than not updating objects i suppose
19:36:17 <meshcollider> CWallet wouldn't store BLANK anyway
19:36:30 <meshcollider> If it needs to know blank, it would ask the spkmanagers
19:36:57 <achow101> yeah, but the wallet flags record is just a bitfield. it can't update it without updating the whole thing
19:37:06 <achow101> so it needs to know all flags in effect
19:41:35 <achow101> maybe function callbacks are the best option
19:43:43 <meshcollider> It seems like it might be, or can you use the built in atomic stuff to avoid locks
19:45:17 <achow101> i'll try it out
19:45:44 <meshcollider> Alright
19:45:57 <meshcollider> Yeah I don't see a particularly clean solution otherwise
19:46:21 <meshcollider> Ok let's end the meeting there then, too many people away today
19:46:24 <meshcollider> #endmeeting