< bitcoin-git> [bitcoin] gwillen opened pull request #15508: Refactor analyzepsbt for use outside RPC code (master...feature-separate-analyzepsbt) https://github.com/bitcoin/bitcoin/pull/15508
< meshcollider> remember the wallet meeting is tomorrow/today
< hyperwang> f
< provoostenator> meshcollider: awesome
< fanquake> meshcollider how long until that?
< provoostenator> fanquake: 19:00 GMT / Iceland time
< provoostenator> I like how this is almost an ad for GMT: https://time.is/nl/GMT
< provoostenator> "No daylight saving time, same UTC offset all year"
< tryphe> provoostenator, Iceland has darkness saving time =p
< fanquake> provoostenator right, so same as usual meeting time? 3am
< fanquake> I played around with the deterministic builds for HWI today, got stuck with wine and page faults.
< jonasschnelli> hmm... "bitcoin-addcon invoked oom-killer"
< jonasschnelli> Ran current master on a 4GB ARM with -dbache=2000
< fanquake> promag is 15478 part of, or the fix for 15453 ?
< jonasschnelli> What could be the reason why bitcoind uses 3GB of ram (RES) when running with -dbcache=2000?
< instagibbs> gwillen, TIL we wrapped boost optional in core :)
< jonasschnelli> I'm aware that the maxmempool gets added to the cache during IBD
< jonasschnelli> 3GB - 300MB mempool - 2GB dbcache makes still 700MB for core without the mempool...
< jonasschnelli> OMG... ignore me. I should sleep more, I set the dbcache to 20GB
< * jonasschnelli> blames the stupid and broken MacBookPro keyboard
< provoostenator> Why buy a reliable keyboard if you can buy more RAM?
< jonasschnelli> heh
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/29c24b05fb71...e8612adc5d07
< bitcoin-git> bitcoin/master fa852f0 MarcoFalke: test: Bump timeout on tests that timeout on windows
< bitcoin-git> bitcoin/master e8612ad MarcoFalke: Merge #15507: test: Bump timeout on tests that timeout on windows
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #15507: test: Bump timeout on tests that timeout on windows (master...1903-testTimeoutWindows) https://github.com/bitcoin/bitcoin/pull/15507
< bitcoin-git> [bitcoin] MarcoFalke pushed 6 commits to master: https://github.com/bitcoin/bitcoin/compare/e8612adc5d07...a6d7026a45c9
< bitcoin-git> bitcoin/master 7aa6a8a Pieter Wuille: Add ParseRange function to parse args of the form int/[int,int]
< bitcoin-git> bitcoin/master 6b9f45e Pieter Wuille: Support ranges arguments in RPC help
< bitcoin-git> bitcoin/master 4566011 Pieter Wuille: Add support for stop/[start,stop] ranges to scantxoutset
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #15497: rpc: Consistent range arguments in scantxoutset/importmulti/deriveaddresses (master...201902_rpcconsistentrange) https://github.com/bitcoin/bitcoin/pull/15497
< bitcoin-git> [bitcoin] MarcoFalke opened pull request #15509: [rpc] deriveaddresses: add range to CRPCConvertParam (master...1903-rpcDeriveAddress) https://github.com/bitcoin/bitcoin/pull/15509
< bitcoin-git> [bitcoin] Sjors opened pull request #15510: [rpc] deriveaddresses: add range to CRPCConvertParam (master...2019/02/rpcconsistentrange) https://github.com/bitcoin/bitcoin/pull/15510
< MarcoFalke> lol
< provoostenator> Oops
< bitcoin-git> [bitcoin] MarcoFalke closed pull request #15509: [rpc] deriveaddresses: add range to CRPCConvertParam (master...1903-rpcDeriveAddress) https://github.com/bitcoin/bitcoin/pull/15509
< promag> wat!
< instagibbs> awkward
< promag> provoostenator: MarcoFalke: well played, you don't fool us X)
< MarcoFalke> eh? My pull was opened earlier! xD
< fanquake> race
< * MarcoFalke> fixes race in deriveaddress
< promag> MarcoFalke: true! then provoostenator commits fast
< MarcoFalke> promag: ERROR: deriveaddresses argument 1 is named range in vRPCConvertParams but ['begin'] in dispatch table
< MarcoFalke> I guess named args don't work then
< promag> where do you get that?
< promag> fanquake: thanks, just saw it too
< fanquake> promag you might have missed my question above^. Is 15478 part of, or the fix for 15453 ?
< promag> yes the goal of that should fix 15453
< fanquake> ok
< bitcoin-git> [bitcoin] MarcoFalke pushed 2 commits to master: https://github.com/bitcoin/bitcoin/compare/a6d7026a45c9...dc251de6a5c4
< bitcoin-git> bitcoin/master aeb7fbf Chun Kuan Lee: appveyor: Don't build debug libraries instead of "build and delete"
< bitcoin-git> bitcoin/master dc251de MarcoFalke: Merge #15506: appveyor: fix cache issue and reduce dependencies build time...
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #15506: appveyor: fix cache issue and reduce dependencies build time (master...appveyor-release-only) https://github.com/bitcoin/bitcoin/pull/15506
< darosior> Hi, I saw that in init.cpp the service NODE_NETWORK_LIMITED is set with NODE_NETWORK at the begining, but I wonder why it is not reset if we did not pruned the block chain ? (https://github.com/bitcoin/bitcoin/blob/b4fc5257b7dc106ff210d170397d4ce0e024f2c0/src/init.cpp#L1652)
< provoostenator> Surprising how many testnet peers are still stuck at the CVE-2018-17144 exploit block. https://bitcoin.stackexchange.com/questions/79662/solving-bitcoin-cores-activatebestchain-failed
< gmaxwell> darosior: limited means the node can ALSO serve recent blocks, services are additive.
< provoostenator> My node shows lots of "Disconnecting outbound peer 5 for old chain, best known block = 00000000210004840364b52bc5e455d888f164e4264a4fec06a514b67e9d5722"
< darosior> gmaxwell : Thank you.
< gmaxwell> provoostenator: most testnet nodes are just forgotten old software, this has more or less always been true. :)
< darosior> gmaxwell : It is indicated "only" instead of "also" in the comment https://github.com/bitcoin/bitcoin/blob/a6d7026a45c915794338c178b7f95d5c1f8e977f/src/protocol.h#L267
< gmaxwell> darosior: you're misunderstanding the comment.
< provoostenator> getchaintips shows this particular block as "headers-only", which I guess is why it doesn't immediately disconnect from nodes with this block?
< gmaxwell> darosior: limited = NETWORK-old_blocks so NETWORK + LIMITED = NETWORK
< darosior> gmaxwell : Ok, thank you.
< provoostenator> (it took 22 minutes from connecting to disconnecting)
< gmaxwell> provoostenator: it won't attempt to fetch it because its clearly a losing chaintip.
< gmaxwell> provoostenator: it also wouldn't be good to disconnect immediately because that would prevent the peer getting blocks from you and reorging to your chain (if it could)
< provoostenator> Ah ok, so it's altruistic intended behavior then. That also applies even if we did know it was invalid?
< gmaxwell> provoostenator: it's incorrect to call that altruistic, as getting the rest of the network on to your chain is very much in your interest... but the getting other people part is mostly accomplished by not doing that kind of strict behavior on inbound.
< gmaxwell> if they actually send something invalid, we'll punt right away.
< bitcoin-git> [bitcoin] darosior opened pull request #15511: RPC : More user-friendly services field in getnetworkinfo and getpeerinfo (master...services_for_humans) https://github.com/bitcoin/bitcoin/pull/15511
< MarcoFalke> I got the "lists.linuxfoundation.org mailing list memberships reminder", so at least it is not all-down
< gmaxwell> lol
< gmaxwell> I suspect the list itself works, but it's all moderated and the moderation interface is down...
< bitcoin-git> [bitcoin] jonasschnelli opened pull request #15512: Add ChaCha20 encryption option (XOR) (master...2019/03/chacha) https://github.com/bitcoin/bitcoin/pull/15512
< gwillen> instagibbs: yeah, I assume we wrap boost::optional so we can replace it with the std one when the time comes
< sipa> indeed
< instagibbs> roger that
< sipa> the source code says "
< sipa> //! Substitute for C++17 std::optional"
< gwillen> although I noticed when I was messing around in psbt.h that PartiallySignedTransaction has a boost::optional in it
< gwillen> I left it alone but I'll change it if someone complains
< bitcoin-git> [bitcoin] MarcoFalke pushed 3 commits to master: https://github.com/bitcoin/bitcoin/compare/dc251de6a5c4...f9dbb319d26f
< bitcoin-git> bitcoin/master 2fa85eb Adam Jonas: add rpc_misc.py, mv test getmemoryinfo, add test mallocinfo
< bitcoin-git> bitcoin/master f13ad1c Adam Jonas: modify test for memory locked in case locking pages failed at some point
< bitcoin-git> bitcoin/master f9dbb31 MarcoFalke: Merge #15485: add rpc_misc.py, mv test getmemoryinfo, add test mallocinfo
< bitcoin-git> [bitcoin] MarcoFalke merged pull request #15485: add rpc_misc.py, mv test getmemoryinfo, add test mallocinfo (master...test_rpc_misc) https://github.com/bitcoin/bitcoin/pull/15485
< meshcollider> wallet meeting time :)
< meshcollider> #startmeeting
< lightningbot> Meeting started Fri Mar 1 19:00:45 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.
< provoostenator> hi
< sipa> present
< 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
< instagibbs> hi
< meshcollider> Topics for this week?
< achow101> Hi
< sipa> i guess there isn't anything wallet-related for 0.18
< provoostenator> #15453
< gribble> https://github.com/bitcoin/bitcoin/issues/15453 | Starting bitcoin-qt with -nowallet and then opening a wallet does not show the wallet · Issue #15453 · bitcoin/bitcoin · GitHub
< provoostenator> Is tagged with 0.18 anyway
< provoostenator> With a cliff-hanger from promag
< meshcollider> That's a Qt issue though right
< achow101> It only effects people who self compile
< achow101> As the issue is with qt versions
< provoostenator> It seems so, in which case it should be OK as long as we backport the fix onto the 0.18 branch.
< meshcollider> Mhm
< meshcollider> Other than that, yeah, we managed to get all the wallet PRs for 0.18 reviewed and merged pretty efficiently so nice work
< provoostenator> Proposed topic: descriptor based wallets
< provoostenator> (briefly anyway, since everyone is focussed on 0.18)
< provoostenator> sipa: one thing we talked about before that would be useful for my HWW PR is away to compose descriptors in code
< provoostenator> If you're able to whip up an example for e.g. sh(wpkh(xpub)) I can probably figure out the rest
< provoostenator> It's just my C++ is a bit rusty when it comes to unique_ptr and abstract classes.
< sipa> provoostenator: it's not that trivial anymore in the current code
< sipa> i briefly had a look, but it would need some restructuring
< provoostenator> Ok, glad to know it's not just me :-)
< provoostenator> The use case here is that we fetch an xpub from a hardware wallet and then construct a descriptor with that.
< sipa> with the 0.18 things out of the way, i'll try to find time to do the ismine/keypool abstraction i've talked about before
< provoostenator> ismine/keypool stuff also very useful.
< provoostenator> An alternative approach would be to be require the hardware wallet (driver) to give a full descriptor, or a set of potential desrciptors from which we pick one or more.
< meshcollider> #topic descriptor based wallets
< provoostenator> But that choice shouldn't be determined by implementation details such as this I think.
< sipa> provoostenator: you can always construct the descriptor in string form
< sipa> i agree that's suboptimal, but it means having an API to construct descriptors shouldn't be a blocker
< provoostenator> That's what I'm doing in the prototype.
< instagibbs> I've done this previously as well, constructing manually
< provoostenator> There's not really any blockers left in that PR, so I'm mostly focussing on getting it merge-worthy.
< provoostenator> Although it would be safer when combined with native-descriptor wallets, because the behavior of getnewaddress doesn't jive well with BIP44/49/84 that wallets use.
< provoostenator> (that hardware wallets tend to use)
< instagibbs> provoostenator, you're safe as long as you don't use getnewaddress with a watchonly wallet :D
< meshcollider> are hardware wallets working on implementing things like returning descriptors?
< provoostenator> instagibbs: so you use deriveaddresses?
< instagibbs> provoostenator, no I just assume people aren't going to dust my cold wallet :P
< provoostenator> meshcollider: not that I know, but HWI python coud easiliy do that.
< instagibbs> meshcollider, unlikely to get that standardized, no
< meshcollider> Mhm
< instagibbs> ledger has basically said they won't bother supporting PSBT like ever
< meshcollider> Oh, thats a shame
< instagibbs> I can't imagine they'd do descriptors
< sipa> well they don't need to
< instagibbs> sipa, right, it's fine
< instagibbs> just giving context
< sipa> i mean: i wouldn't even recommend that hw wallets directly implement psbt
< sipa> all you need is a software driver that supports it
< sipa> same with descriptors
< sipa> they're a software (and bitcoin core specifically...) kinda thing
< provoostenator> Basically Bitcoin Core could give an account number and then HWI would, if the device supports SegWit return three descriptors: "pkh([44'/0'/account]xpub/0/*])", "sh(wpkh([49'/0'/account]xpub/0/*]))" and "wpkh([84'/0'/account]xpub/0/*])" (plus change), and then our wallet imports one or all
< achow101> hwi already uses descriptors
< provoostenator> But the current importmulti logic doesn't handle the above correctly, native descriptor wallets would.
< provoostenator> So then you can get a bech32 address and it will grab it from the wpkh(84' tree, or fail if that's absent.
< provoostenator> We probably only need two address types: default and wrapped-segwit. Default is either legacy pkh() or native segwith (wpkh or script hash).
< sipa> you'd mark certain records as the provider for certain types of addresses
< sipa> if you don't have one for native segwit addresses, then requesting a native segwit address will fail
< provoostenator> Right, I guess it's fine to hold on to the current address_type concept then.
< provoostenator> legacy, p2sh_segwit and bech32
< provoostenator> But simply fail if the right descriptor isn't available.
< provoostenator> Do descriptors needs to "marked" for that? Or can you just infer it?
< meshcollider> The default ones would need to be marked in case you have others too
< sipa> well i'm imagining you can have multiple records
< provoostenator> One default record in case there's multiple records of the same address type.
< sipa> (especially if you have a wallet that at some point switched over to a new key/descriptor/hwwallet for whatever reason, but still want to keep access to old transaction history with the previous ones)
< provoostenator> I was thinking of adding a purpose enum: receive, change, other
< sipa> yes
< provoostenator> For each address type there can only be one descriptor with receive/change
< sipa> why?
< provoostenator> Because that indicates the default
< sipa> what a descriptor is used for should be independent of whether it's the default imo
< provoostenator> That makes sense
< provoostenator> So then in addition to purpose, we also need to track the default descriptors.
< sipa> if you transition to a new one, the one you used for change before doesn't stop being change
< provoostenator> How do we identify descriptors if there's more than one? Just an increasing int as we add them, or by their checksum (seems brittle) or something else?
< sipa> seems like an implementation detail
< provoostenator> Well, it matters for RPC arguments, but for now I'm avoiding that problem by not allowing more than one of each purpose :-)
< sipa> do you need to expose internal identifiers?
< sipa> (i haven't thought much about this)
< meshcollider> Identify them by checksum :p
< sipa> if so, it seems best that records have a unique user-provided name, and you use that name in rpc to refer to them
< provoostenator> sipa: getnewaddress "Buy new cow" bech32 [how do I tell which non-default descriptor to use]
< instagibbs> why not force them to switch first, then call getnewaddress
< sipa> provoostenator: is that necessary?
< provoostenator> Name makes sense
< sipa> provoostenator: you can't getnewaddress from an old keypool now either
< meshcollider> Other records are more for things like watch-only, etc
< meshcollider> Internal stuff
< provoostenator> True, if non-default descriptors can only be used for archival purposes then there's no need to support them in getnewaddress
< sipa> that's what i was thinking yes; they're just there to not lose old transaction history
< sipa> or to watch
< provoostenator> It also depends on how much we want to push for 1-wallet-1-goal
< provoostenator> Rather than put-whatever-you-want-in-a-single-walllet
< sipa> right; but in a pure watch-only wallet (where "watch only" truly means you're watching someone else's wallet, not just "i don't have the private key right here"), you probably don't want to have any "default newaddress provider"
< provoostenator> I also prefer keeping wallets simple in that sense, especially to keep the GUI sane.
< sipa> because there shouldn't be a way to create new addresses
< provoostenator> Agree that getnewaddress should require >= IsSolveable
< sipa> descriptors are always solvable
< sipa> (except the "raw" and "script" ones, but those can't be used to derive anything anyway)
< provoostenator> Ah yes
< sipa> solvability is not the point
< meshcollider> thats the difference between is "mine" and is "spendable"
< sipa> so right now "watchonly" means "i don't happen to have the private key in my wallet.dat file"
< sipa> i think that's a mistake
< meshcollider> Agree, this rework is the perfect time to tighten up definition of things like that too
< sipa> the distinction between "my money" and "watched money" is useful, but it's somewhat independent from where the private keys live
< sipa> especially in a hwwallet/offline storage/... kinda world... the case where the private key is actually in your wallet.dat file (even for your own coins) should be the exception
< sipa> oh, i guess i should bring this up here, i did a talk at SBC19 about miniscript, and i'll work on getting support for it in core
< sipa> so that our signer (including psbt, etc) can deal with more complicated scripts than just single key and multisig
< provoostenator> So we can do things like GreenAddress style multisig or timeout & single sig?
< sipa> yeah
< meshcollider> Was the talk recorded
< meshcollider> Thanks
< provoostenator> I wonder what the setup UX would look like if e.g. you had two hardware wallets and wanted such a 2-of-2 or timeout & 1-of-2.
< sipa> provoostenator: that's a question for you :)
< sipa> i'm perfectly fine with psbt rpcs :P
< instagibbs> gui coin control tho
< achow101> sipa: replacing the signer with miniscript would probably be a good time to also distinguish between signing and finalizing
< sipa> achow101: hmm?
< provoostenator> Right, I guess there would be an RPC where you give it a meta-descriptor or miniscript, then it goes and fetches xpubs from both devices in sequence and imports the right descriptors
< provoostenator> Plus asking some sanity check questions to the driver like "can you sign stuff with nlocktime"
< achow101> ProduceSignature makes both the signature and scriptSig/witness
< instagibbs> provoostenator, the descriptor will be parsed into miniscript, afaik
< sipa> provoostenator: the workflow is that you'd use the miniscript compiler to convert a policy to a descriptor
< instagibbs> ^
< sipa> and then import the descriptor
< instagibbs> yeah
< sipa> the compiler could be built-in, but doesn't need to be
< provoostenator> Do you put xpubs in the miniscript or does that happen later?
< achow101> But under the psbt model, those should be distinct.
< sipa> provoostenator: yeah
< sipa> the compiler just passes those though, i imagine
< sipa> (miniscript is just the name of the subset of script; i use "policy" language to refer to the input of the compiler)
< sipa> i know, i suck at naming things
< sipa> and this is genuinely confusing because there are several things that look kinda similar
< provoostenator> signerfetchkeys "miniscript with rules $device_fingerprint_1 $device_fingerprint_1"
< sipa> achow101: yeah, i guess it can simplify things if we split up the signer and finalizer in the code too
< provoostenator> Piece of cake
< meshcollider> Is there anything else anyone wants to talk about in the last few minutes
< meshcollider> #endmeeting
< lightningbot> Meeting ended Fri Mar 1 19:58:53 2019 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)