< bitcoin-git>
[bitcoin] Sjors opened pull request #18220: psbt: AnalyzePSBT set next to FINALIZER when all inputs need finalizing (master...2020/02/analyze_psbt) https://github.com/bitcoin/bitcoin/pull/18220
< bitcoin-git>
[bitcoin] dangershony opened pull request #18223: Add new filer type p2wpkh to blockfilterindex (master...nutrino-p2wpkh-filters) https://github.com/bitcoin/bitcoin/pull/18223
< provoostenator>
Random thought: why doesn't Git have SegWit, so you can pre-ACK a rebase? :-)
< bitcoin-git>
bitcoin/master cbd345a Sebastian Falbesoner: test: test OP_CSV empty stack fail in feature_csv_activation.py
< bitcoin-git>
bitcoin/master 09f706a Sebastian Falbesoner: test: check for OP_CSV empty stack fail reject reason in feature_csv_activ...
< bitcoin-git>
bitcoin/master 5ffaf88 Sebastian Falbesoner: test: eliminiated magic numbers in feature_csv_activation.py
< bitcoin-git>
[bitcoin] MarcoFalke merged pull request #17921: test: test OP_CSV empty stack fail in feature_csv_activation.py (master...20200113-test-check-for-empty-stack-op_csv) https://github.com/bitcoin/bitcoin/pull/17921
< bitcoin-git>
[bitcoin] instagibbs opened pull request #18224: Make AnalyzePSBT next role calculation simple, correct (master...analyze_psbt_role_simple) https://github.com/bitcoin/bitcoin/pull/18224
< bitcoin-git>
[bitcoin] Sjors closed pull request #18220: psbt: AnalyzePSBT set next to"finalizer" when all inputs need finalizing (master...2020/02/analyze_psbt) https://github.com/bitcoin/bitcoin/pull/18220
< bitcoin-git>
[bitcoin] MarcoFalke opened pull request #18225: util: Fail to parse empty string in ParseMoney (master...2002-utilMoney) https://github.com/bitcoin/bitcoin/pull/18225
< bitcoin-git>
[bitcoin] instagibbs closed pull request #18214: wallet: Give slightly more understandable advice when needing -fallbackfee (master...fallback_msg) https://github.com/bitcoin/bitcoin/pull/18214
< achow101>
what's not clear to me is why we need a file format?
< provoostenator>
Well, so far it's just a JSON format, doesn't ahve to be a file
< achow101>
can't you just pass around a miniscript policy, maybe with placeholders, and let people add things to it?
< provoostenator>
But it's something you can pass around
< provoostenator>
It contains sub policies for each signer
< provoostenator>
And keys
< provoostenator>
And optionally a friendly name and info about capabilities
< provoostenator>
One of the participants can collect that info and combine it.
< provoostenator>
And then figure out the overall policy, miniscript and descriptor. And then send that back to the participants
< provoostenator>
It would be nice if miniscript supported actual placeholders though
< achow101>
I guess what I'm asking is why can't you just pass around a single miniscript policy string that people modify
< provoostenator>
Then you can announce the overal policy _before_ collecting info from indiviual signers.
< provoostenator>
Oh I see, that's possible too, but it requires that participants actually can parse miniscript, which I'm not assuming
< provoostenator>
Simple string concatenation is enough to handle the format I have so far.
< instagibbs>
oh hi
< meshcollider>
Is the assumption that all the participants are completely trustworh
< meshcollider>
Trustworthy*
< achow101>
but participants have to be able parse miniscript at the end anyways, no?
< provoostenator>
meshcollider: there's room for arbirary fields, so they don't have to be
< achow101>
you have to trust participants to not mess with other participant's policies
< provoostenator>
There's also room for e.g. musig related info, not something that would fit in a miniscript policy that you pass around
< meshcollider>
achow101: idk if that's an assumption we want to make?
< instagibbs>
meshcollider, sure seems like something an attacker might do
< achow101>
meshcollider: well at the end, you can verify whether you are still in the policy
< achow101>
and under what conditions your sub policy would be reached
< achow101>
that's the point of miniscript
< sipa>
provoostenator: the participants need to be able to reason about the policy of the final descriptor that comes out
< sipa>
miniscript enables that
< provoostenator>
You probably have to check the first receive address via some other channel to make sure everyone is looking at the same policy
< sipa>
without generic script.reasoning logic like that i don't think what you're trying is secure
< provoostenator>
Miniscript enables it in the general case.
< achow101>
provoostenator: but for musig, and taproot in general, I would expect there to be different miniscript things for that
< provoostenator>
But in the simple case you can still reason about thresh(2,pk(3442193e),pk(bd16bee5))
< instagibbs>
I think spelling out exactly what you're enabling and protecting against would help for your PoC
< sipa>
sure
< provoostenator>
I'm trying to make it useful pre-miniscript, but in a forward compatible format.
< sipa>
i suspect getting people to adopt a file format will be harder and slower than integration of miniscript :)
< provoostenator>
instagibbs: personally I'm happy if it can do m-of-n with devices that I initially trust
< sipa>
especially when its usefulness is extremely likited before that poijt in time
< instagibbs>
provoostenator, ok, that we can reason about with nothing too fancy :)
< instagibbs>
miniscript really does need network effects to be worth it
< provoostenator>
Yes, the ad hoc format used by ColdCard does the trick
< instagibbs>
meanwhile I think pressuring hww devs to support things like display xpub, register some sorta descriptor like thing, is the best thing to do
< meshcollider>
True
< instagibbs>
gets you usable n-of-m at least
< provoostenator>
So ColdCard registers xpubs, I don't think any other hww does anything similar
< instagibbs>
provoostenator, indeed, btchip says it's on the roadmap(no convincing needed at least)
< achow101>
I would prefer people to just use miniscript and then compose policies within a miniscript policy itself, rather than a file format
< provoostenator>
This may be a chicken-egg thing where people want a standard first, but a standard is hard to develop without practical experience.
< provoostenator>
achow101: first thing we'd need for that is xpub & origin support in descriptors
< provoostenator>
And ideally placeholder support, so a signer knows where they can insert stuff
< sipa>
it's already there?
< achow101>
in miniscript you mean?
< sipa>
(xpub and origin support)
< provoostenator>
sipa on your site I could only add pk(fingerprint)
< provoostenator>
And then wallet 2 fills in that spot,
< sipa>
the hard part is letting wallets verify that the resulting script/descriptor includes the policy they want
< sipa>
which isn't implemented in my c++ miniscript code
< sipa>
rust-miniscript may
< achow101>
sipa: I believe rust-miniscript lets you "pull up" a miniscript to the policy
< instagibbs>
"they want" seems like another patch of thorns
< achow101>
andytoshi also said it was trivial to do so
< sipa>
yeah, it is
< instagibbs>
not sure what "pull up" means exactly but I'll defer that to me actually learning miniscript
< sipa>
instagibbs: compiler goes from policy to miniscript
< sipa>
"pull up" means going the other direction
< sipa>
that step is easy
< achow101>
"decompile miniscript"
< sipa>
but then reasoning about the policy may not be
< instagibbs>
i see, you mean someone brings compiled miniscript, you can graft it in, sure
< sipa>
no
< instagibbs>
ohhh sorry misreading
< instagibbs>
way over-reading what achow said, ignore
< sipa>
it's just about: someome gives you a script, figure out what it "does", semantically
< instagibbs>
yes
< sipa>
like... someone "included" your policy in a compiled script
< meshcollider>
Because you don't only want to check your spending condition, you really need to check no other paths have been added that shouldn't be there
< sipa>
maybe they combined it with an and(X,false)
< sipa>
meshcollider: indeed
< instagibbs>
"should be" lots of worms in cans ;P
< instagibbs>
n-of-m is good or bad depending on who is in the set
< sipa>
or did they compile it into a ridiculously inefficient script?
< sipa>
i think what may be generically possible is where you have a super-policy super(A,B,C) that is agreed upon out of band (e.g. 2-of-3 multisig)
< sipa>
and then let participants fill in their own A, B C
< sipa>
the composability of policies means that you generally shouldn't care about what others' A B and C are
< meshcollider>
Isn't that what provoostenator did anyway
< meshcollider>
Well, limiting super = thresh
< instagibbs>
Provided you're talking to the right folks gathering A,B,C, I think so :)
< sipa>
the hard part in this case is where does the super-policy come from
< provoostenator>
meshcollider: not limiting the policy, but even limiting the compiler
< provoostenator>
*not only
< meshcollider>
Alright achow101 do you want to talk about descriptor normalisation now
< achow101>
sure
< meshcollider>
I think the multiwallet needs more thought out of meeting
< meshcollider>
Multisig wallet*
< instagibbs>
(topic for coredev)
< achow101>
we can add it to kanzure's list of discussion topics
< kanzure>
okay
< achow101>
I kind of tried to do this descriptor xpub normalization in #18163
< achow101>
closed it in favor of the xpub cache, but I think it might still be useful to do
< achow101>
basically if we get a descriptor with a xprv and a bunch of hardened steps, then we can make an equivalent descriptor which has the xpub at the last hardened step and the hardened steps and that xprv become the origin info
< achow101>
we lose the ability to round trip such descriptors, but I think it's still useful to be able to do this for things like exports
< provoostenator>
That seemed sane to me
< achow101>
we can also go a step further and do it to all descriptors with xpubs, just derive as far as possible
< achow101>
it's all the same at the end, just might be confusing to users
< meshcollider>
Derive even the non-hardened steps and just have the /* at the end?
< achow101>
yeah
< provoostenator>
I find hardened a more intuitive place to cut off
< provoostenator>
It also keeps the xpub in the expected place for BIP44/49/84 style descriptors
< meshcollider>
Yeah I don't think there's any point to doing work that anyone else could do anyway
< achow101>
it has the effect of making the xpub cache part of the descriptor
< achow101>
since in xpub cache, we derive as far as possible and cache that xpub
< provoostenator>
I think that cache policy should just be internal
< meshcollider>
Yep I can see maybe why xpriv/hardened -> xpub is useful but not other than that
< achow101>
less derivations to do
< provoostenator>
That seems like a tiny benefit compared to loading a wallet and expanding 1000 keys
< achow101>
so with just the hardened derivation, that's something people think we should still try?
< achow101>
I think the main concern is that we lose information
< sipa>
it's only human-relevant information
< sipa>
as the semantics of the normalized descriptor are the same as the original
< achow101>
right, but if getdescriptorinfo returned a normalized descriptor, that would probably confuse people
< sipa>
but i'm still hesitant to just always do it
< sipa>
agree
< sipa>
it seems unnecessary, except perhaps in certain opt-in cases
< achow101>
the main use is imports into our wallet, and exporting watch only to other wallets
< sipa>
but you could do it at export time?
< achow101>
it would require access to private keys
< achow101>
it'd be nice if it didn't
< sipa>
or to the xpub cache?
< achow101>
with the xpub cache, it would give the xpub at the end of derivation
< sipa>
which is just as good, no?
< achow101>
still confusing to users
< sipa>
not more so than an xpub in the middle?
< achow101>
and possibly to wallets that may try to interpret the derivation info to figure out change/not-change
< provoostenator>
Especially the latter
< sipa>
the origin info would still be there
< achow101>
(I suspect that would be something that wallets try to do)
< sipa>
which would have that information
< provoostenator>
E.g. with a ColdCard you register an xpub, which covers receive and change
< provoostenator>
So it would be confused by a desciptor that has the xpub 1 level down
< provoostenator>
Then again, you can't really export a single descriptor anyway
< achow101>
I suppose we can bring this up again once we get to allowing descriptor exports
< sipa>
yeah
< provoostenator>
I wouldn't mind being able to describe receive and change in single descriptor, but that's another can of worms.
< sipa>
yes :)
< achow101>
xpub cache covers what we need to do now, so we can think on this later :)
< bitcoin-git>
bitcoin/master 54be4e7 Sebastian Falbesoner: test: check specific reject reasons in feature_csv_activation.py
< bitcoin-git>
bitcoin/master 9aa8145 MarcoFalke: Merge #17959: test: check specific reject reasons in feature_csv_activatio...
< instagibbs>
oh we're 8 minutes over
< bitcoin-git>
[bitcoin] MarcoFalke merged pull request #17959: test: check specific reject reasons in feature_csv_activation.py (master...20200118-test-check-reject-reasons-in-feature-csv-activation) https://github.com/bitcoin/bitcoin/pull/17959
< achow101>
any other topics?
< instagibbs>
PSBT GUI review: do it
< gwillen>
apropos of that actually instagibbs, I saw you commented about showing change addresses
< gwillen>
I am pretty fuzzy on the story of "safely detecting change addresses" in this setting
< achow101>
Change detection is always fuzzy
< provoostenator>
On the send dialog you can know which one is change
< provoostenator>
On the load PSBT I wouldn't bother for now.
< instagibbs>
in normal sends it ellides those outputs... but PSBT signing is not the typical case, in many cases
< instagibbs>
provoostenator, right
< gwillen>
yeah, my assumption was not to bother for signing, and to show all addresses
< provoostenator>
It's probably some property on rcp that you can look at, the normal confirm dialog knows.
< sipa>
you can show the net balance effect a transaction has on your wallet, independent of knowing what is change or not, right?
< gwillen>
not when signing, no wallet
< achow101>
no wallet?
< sipa>
ah, without wallet you can't even talk about the concept of change
< instagibbs>
it might be a dumb key store
< instagibbs>
achow101,
< gwillen>
well, hm, I guess I am assuming that in general, when signing offline, you are just a dumb key store, yeah
< gwillen>
you may not have the blockchain, and you may only have keys to some subpart of whatever inputs you're signing for
< sipa>
gwillen: well a sane key store (one that can verify what it's signing) must have a pre-registered descriptor set
< achow101>
For change detection, you should be able to just ask the wallet if a particular destination IsChange and do your change detection like that
< achow101>
but that assumes the PSBT belongs to that wallet
< sipa>
gwillen: if you don't have that, talking about balance or change is meaningless
< gwillen>
sipa: do we have a sane key store, though, in the sense
< achow101>
gwillen: we will soon(tm)
< sipa>
gwillen: well, our wallet is
< gwillen>
in particular, the case I am most interested in is signing for a multisig
< provoostenator>
Right, fun fact about the current keystore: getrawchange address wil give you an address from the receive chain
< gwillen>
in which case there is a lot more information needed before one could safely conclude that some output is "change"
< sipa>
my point is just that if you want to do signing without such knowledge (which is a totally reasonable thing to do in some cases), you must accept that that means there is no such thing as change detection and shouldn't bother
< gwillen>
*nods*
< instagibbs>
gwillen, well, is today's wallet IsChange would fail for any multisig address
< gwillen>
anyway instagibbs this was apropos of your comment asking why we display the change
< instagibbs>
if its a descriptor wallet change-ness is stored
< instagibbs>
anyways, it's fine for now
< * sipa>
-> lunch
< gwillen>
the answer is that I'm assuming in almost any interesting case we can't tell
< gwillen>
and so there's no point in special-casing the boring cases where we can
< instagibbs>
gwillen, disagree I think?
< achow101>
gwillen: I don't think that's an assumption in descriptor wallets
< achow101>
since in descriptor wallets you import the descriptor and mark it as change or not
< gwillen>
well we don't have those yet, so
< gwillen>
when we have those I will revisit :-)
< sipa>
i don't see what descriptor wallets have to do with this, actually
< sipa>
either you have a wallet, and you can ask what it would consider change
< sipa>
or you don't
< achow101>
you can do the same change detection stuff now as you would in the future, it's exposed in the same way
< achow101>
we just won't detect multisig or funny script things as change
< instagibbs>
`IsChange()` works for random imports already AFAIK, you just can't put in keypool
< meshcollider>
Anyway I think let's end the official meeting
< meshcollider>
#endmeeting
< lightningbot>
Meeting ended Fri Feb 28 20:17:59 2020 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
< instagibbs>
i.e., importmulti, mark it as internal address, now it'll be marked as `IsChange` internally
< gwillen>
hmmmm, so if I have a Core wallet that I use offline, with a watchonly copy that I use online, with PSBT, change detection should actually Just Work
< gwillen>
well hm, though, with a normal Core wallet you do not have public derivation
< instagibbs>
obviously if you're thinking of a HD chain multisig it probably needs descriptor wallets to be ez
< gwillen>
so you can't meaningfully do this
< instagibbs>
right this only works with one-off keys
< gwillen>
I'm trying to think of a scenario where you can meaningfully do this
< gwillen>
and have it work without shenanigans
< gwillen>
well with one-off keys there is no such thing as change, unless you're reusing addresses
< gwillen>
unless you mean like, one-off exporting your keypool watchonly, and then doing it again manually each time you run out
< achow101>
gwillen: what I'm saying is that you should use IsChange and we will later refine what IsChange returns true for
< instagibbs>
yes there is, when manually creating tx
< instagibbs>
I agree it's not generally usable
< gwillen>
I am not going to use IsChange if there is not a single case where it actually works right now
< gwillen>
untested codepaths break
< achow101>
it will work for single key things
< instagibbs>
^
< gwillen>
so far it appears to me that we have failed to produce a case where that would be useful
< achow101>
your standard single hardware wallet case
< instagibbs>
I literally tested your branch and was befuddled when i was confronted with this
< achow101>
using importmulti, I import a bunch of receive keys into the keypool, and a bunch of change keys
< achow101>
when I do all the psbt things after signing, I look at the gui and it's showing me the change address
< gwillen>
you just manually import them, and then import again when you run out?
< achow101>
IsChange would tell you that the change address is change because I had imported those into the change keypool
< instagibbs>
import 10k or whatever yes
< achow101>
there's your usecase
< gwillen>
I mean, I'm happy to support this if it is something that anybody would actually do
< instagibbs>
I do it!
< gwillen>
heh, ok
< achow101>
me too!
< instagibbs>
thanks <3
< gwillen>
do you object to still displaying the change, but marking it as change
< achow101>
that's fine with me
< instagibbs>
if it's marked change somehow it's fine
< gwillen>
:+1:
< instagibbs>
more info for a power user is better
< gwillen>
I will also add a note, if we do not detect change, explaining that change is a nebulous concept
< gwillen>
and think about whether I should also include it even if we do
< gwillen>
(just explaining that anything we detect as change is change, but we may not detect all things a user would consider change)