< benthecarman> Can someone point me to a good source to learn about p2sh
< sipa> bip16
< hidden> ReserveKeyFromKeyPool: unknown key in key pool (code -1) :(
< phantomcircuit> sipa, why have a bunch of functions that modify a struct instead of a class?
< sipa> phantomcircuit: wut?
< phantomcircuit> the rng stuff
< sipa> i don't understand
< sipa> you mean why is RNGState a struct and not a class?
< phantomcircuit> yes
< sipa> you know what the difference is between the two?
< phantomcircuit> like AddDataToRng could be a method instead of calling GetRNGState
< sipa> sure
< phantomcircuit> sipa, public/private default, but i meant more the style change that typically happens with a class
< sipa> maybe i should make the internal state orivate
< sipa> *private
< sipa> that seems like a nice guarantee anyway
< sipa> to avoid bugs that accidentally wipe the state
< gmaxwell> can the state get stored in the mlocked pool, or is there some initalization order issue there.
< sipa> that's actually pretty doable
< sipa> done
< sipa> that was surprisingly easy
< wumpus> we should probably restart the discussion about wallet recovery at some point #10540 #8745 #10991 it kind of bled out
< gribble> https://github.com/bitcoin/bitcoin/issues/10540 | [WIP] Salvage wallet should not set the aggressive flag on Db::verify() by jnewbery · Pull Request #10540 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/8745 | [PoC] Add wallet inspection and modification tool "bitcoin-wallet-tool" by jonasschnelli · Pull Request #8745 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/10991 | -salvagewallet seems to be causing more issues than it fixes, it should be removed · Issue #10991 · bitcoin/bitcoin · GitHub
< wumpus> it would still make sense to do the separate wallet recovery tool
< promag> sipa: ping https://github.com/bitcoin/bitcoin/pull/15107#issuecomment-452821812 in case you missed the notification
< sipa> I'll probably miss the wallet meeting today, i'm at real world crypto
< meshcollider> #startmeeting
< lightningbot> Meeting started Fri Jan 11 19:00:14 2019 UTC. The chair is meshcollider. Information about MeetBot at http://wiki.debian.org/MeetBot.
< lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
< 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
< jnewbery> hi
< instagibbs> hi
< phantomcircuit> hi
< meshcollider> First wallet meeting of 2019
< meshcollider> Any topics?
< achow101> hi
< achow101> what's the status on #14491
< gribble> https://github.com/bitcoin/bitcoin/issues/14491 | Allow descriptor imports with importmulti by MeshCollider · Pull Request #14491 · bitcoin/bitcoin · GitHub
< instagibbs> I've been mostly paying attention to HWI sorry :X
< achow101> same
< meshcollider> I've got to address a few comments there, and split up one of the commits
< jnewbery> I think might still be some rebase issues with 14491. I commented in the PR
< meshcollider> I'm travelling for the next few days unfortunately so I'm not sure how soon I'll have time
< jnewbery> I might have some time next week to try to rearrange the commits
< instagibbs> open Q I brought up in HWI: You cannot actually import two addresses if the wall already "understands the script" and then it complains about private keys. Is that an intentional limitation?
< instagibbs> say first one is native segwit
< instagibbs> second is p2sh wrapped, it doesn't let you import the second
< instagibbs> is this just a Core thingy we'll wait until "new wallet" to fix?
< * instagibbs> finding the exact test case I ran into and had to work around
< meshcollider> Is it a case of the wallet saying it already contains the script
< instagibbs> i reordered the keys to generate a new script here, but to test you can unscramble
< instagibbs> we can discuss offline
< achow101> i think it's the wallet saying it already has the script
< instagibbs> im sure that was the intended message
< meshcollider> Because of the "learn relayed scripts" hack
< meshcollider> Related*
< instagibbs> right, so might be "core problems" until revamp
< meshcollider> I can look into it further if you want, if you're not convinced its that
< instagibbs> if you think it's that it's probably that
< instagibbs> realistically you shouldn't be reusing same core script anyways
< instagibbs> other topics?
< meshcollider> But you're right the revamp will fix that
< instagibbs> indeed
< meshcollider> Looks like no other topics for now then
< meshcollider> #endmeeting
< lightningbot> Meeting ended Fri Jan 11 19:16:46 2019 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< luke-jr> is there a good reason we have 3 different "decompose transaction" functions? :x
< luke-jr> (all with subtle differences!)
< marcinja> 1
< phantomcircuit> luke-jr, hmm?
< luke-jr> phantomcircuit: wallet.cpp has one used by RPC; the GUI has one for the tx list, and then another for the tx details
< luke-jr> I guess maybe RPC needs a separate one for higher compatibility requirements, but.. :/
< luke-jr> at least once send-to-self is gone, I think what we want is identical anyway?
< phantomcircuit> luke-jr, oh you mean the CTransction -> json things
< phantomcircuit> yeah an the rpc results are weird as a result
< luke-jr> well, ->JSON is just one of 3 implemetnations
< luke-jr> the others go ->GUI and ->HTML