--- Day changed Sat Oct 24 2015
00:27 -!- GAit [~GAit@2-228-102-100.ip191.fastwebnet.it] has quit [Quit: It was good while it lasted!]
07:42 -!- fkhan [weechat@gateway/vpn/mullvad/x-pafengwxecqdlpgq] has quit [Ping timeout: 252 seconds]
07:48 -!- fkhan [weechat@gateway/vpn/mullvad/x-rymbotvculqltgnb] has joined #secp256k1
11:26 -!- maaku [~quassel@botbot.xen.prgmr.com] has joined #secp256k1
11:26 -!- maaku is now known as Guest6602
11:27 -!- Guest6602 is now known as maaku
13:34 -!- fkhan [weechat@gateway/vpn/mullvad/x-rymbotvculqltgnb] has quit [Ping timeout: 260 seconds]
13:36 < gmaxwell> sipa: I believe I told you that parse as fail was going to confuse people! :) https://github.com/bitcoin/secp256k1/issues/341
13:46 -!- fkhan [weechat@gateway/vpn/mullvad/x-yjxxwavjvubkbjnz] has joined #secp256k1
13:57 < sipa> gmaxwell: interested in his reaction :)
14:02 < gmaxwell> sipa: so its a little annoying that this parser is close enough to a STRICTDER test that you might mistake it as one, and e.g. accept a CHECKSIG NOT script that passes in a negative value signature.
14:59 -!- gmaxwell is now known as gmaxweIl
14:59 -!- gmaxweIl is now known as gmaxwell
16:16 < Luke-Jr> bug: gen_context appears to use GCC even when I set CC=clang for configure
16:26 < Luke-Jr> cfields: you do the build system for libsecp256k1? ^
16:29 < gmaxwell> Luke-Jr: set CC_FOR_BUILD
16:43 < Luke-Jr> oh, in case of cross-compiling, ok
16:44 < gmaxwell> yea, CC is used to build the library, CC_FOR_BUILD is used exclusively for host only tools to build some headers the library build needs.
16:45 < Luke-Jr> btw LLVM segfaults building libsecp256k1 on x32, but I'm pretty sure it's LLVM's fault
16:49 < gmaxwell> if LLVM is segfaulting, it's LLVM's fault, but thats pretty scarry.
16:50 < Luke-Jr> gmaxwell: I had to patch LLVM just to get it to build. libass also fails to build with the resulting LLVM (but simple test cases work).
16:51 < Luke-Jr> (eg, the only file that segfaults in libsecp256k1 is the giant secp256k1.c)
16:51 < Luke-Jr> it seems LLVM isn't really x32-ready yet
16:52 < gmaxwell> thats the only file it compiles though. :)
16:52 < Luke-Jr> then benchmarks and tests appear to be separate?
16:53 < Luke-Jr> the*
16:54 < gmaxwell> indeed. Does it compile the tests? the tests #include secp256k1.c
16:55 < Luke-Jr> hm, I guess not. only see: bench_ecdh.o bench_recover.o bench_schnorr_verify.o bench_sign.o bench_verify.o
16:58 < Luke-Jr> LLVM from git is [2205/2749] done, so we'll see if it improves any
17:11 -!- gmaxwell is now known as gmaxweii
17:11 -!- gmaxweii is now known as gmaxweiI
17:11 -!- gmaxweiI is now known as gmaxwell_
17:11 -!- gmaxwell_ is now known as gmaxwelI
17:12 -!- gmaxwelI is now known as gmaxwell
17:23  * Luke-Jr notes gmaxwell can do that in a separate IRC client :p
17:27 < Luke-Jr> oh good, LLVM git seems to compile and tests pass
19:32 < gmaxwell> sipa: so, I think xenog expects the parse suggess to equal STRICTDER passing, and it doesn't in that STRICDER _partially_ checks the ranges.
19:39 < sipa> right
19:39 < sipa> also, mosh is pretty nice!
19:40 < gmaxwell> Perhaps we should at least document that it's not the same as STRICTDER?
19:41 < sipa> i think we should document in BIP66 that STRICTDER is not exactly the same as DER
19:42 < sipa> no reason why a general-purpose crypto library's DER parser would contain any information specific to bitcoin...
19:43 < gmaxwell> The orginization name of "bitcoin" may throw some people off there..  Perhaps we should toss a BIP66 checker in contrib? (no reason that bitcoin software authors should be writing DER parsing code.)
19:45 < gmaxwell> sipa: Yea, I like mosh. Really I don't know that its much of an improvement, for me, over SSH on airhook (since I don't really care much about the prediction), but its nicer to use widely used software.
19:45 < sipa> BIP66 is a bitcoin consensus rules; Bitcoin full nodes implementations need an exact checker specified by BIP66 anyway
--- Log closed Sun Oct 25 07:14:16 2015
--- Log opened Sun Oct 25 07:14:53 2015
07:14 -!- kanzure [~kanzure@unaffiliated/kanzure] has joined #secp256k1
07:14 -!- Irssi: #secp256k1: Total of 24 nicks [1 ops, 0 halfops, 0 voices, 23 normal]
07:23 -!- Irssi: Join to #secp256k1 was synced in 551 secs
10:33 -!- maaku [~quassel@botbot.xen.prgmr.com] has quit [Remote host closed the connection]
10:36 -!- maaku [~quassel@botbot.xen.prgmr.com] has joined #secp256k1
10:36 -!- maaku is now known as Guest52285
10:37 -!- Guest52285 is now known as maaku
10:56 -!- maaku__ [~quassel@173-228-107-141.dsl.static.fusionbroadband.com] has quit [Read error: Connection reset by peer]
14:45 < gmaxwell> sipa: can we make _seralize fail for forced invalid signature objects?
14:46 < gmaxwell> I think that would have helped avoid the issue on github right now; the issue there is that the signature objects are supposted to be opaque and the caller should have _no_ idea that it's using zeros for forced invalid.
14:47 < sipa> that's a possibility, but then the 'forced invalid' internal value must be something that is not valid scalars
14:47 < sipa> as all values of a scalar can be in DER
14:48 < sipa> (including (0, 0))
14:50 < sipa> that does make sense
14:51 < sipa> as it's reasonable to except that if parse_der succeeds and its reserialization succeeds, the output will be identical to the input
14:51 < sipa> s/expect/expect/
14:51 < sipa> eh
14:51 < sipa> s/except/expect/
14:54 < gmaxwell> if we add an additional flag we'll make it no longer 64 byte alignable. Which would be sad.  We could have used an alternative formulation of this interface where instead of deseralizing to scalars we convert signatures to a compact form, and have the functions fail if the compact form would not round-trip, and then make verify consume the compact form.
14:55 < sipa> we can use 0xFFFFFFF.... as forced-invalid value
14:56 < gmaxwell> why do you prefer 0xFFFF... ? it too could be DER encoded.
14:58 < sipa> good point!
14:58 < gmaxwell> I don't think I have any argument against it other than zeros has some slight benefit in that caller error is more likely to hand us uninitilized objects that happen to be filled with zeros.
14:58 < gmaxwell> For ecdsa both R and S will never be zero with any curve.
14:59 < gmaxwell> (or to be more precise, neighter R nor S will be zero with any curve.)
14:59 < gmaxwell> As 0 is not a member of the multiplicative group.
15:01 < sipa> yes, i somehow thought that everything in the range of valid scalars had to roundtrip
15:01 < sipa> but that's the same mistake i blame xenog for: that the internal scalar type's values are somehow special
15:02 < sipa> so yes, making 000 fail to reserialize makes sense i think
15:02 < gmaxwell> It's arguable that if parse is successful, seralize should be successful, and it should round trip. But I think that if we aren't going to obey that (which I think we shouldn't because otherise signature objects must be arbritarily large), then I think it should round trip unless seralize fails.
15:04 < sipa> the only argument against is that the forced-invalid state should be unobservable to a user
15:04 < sipa> though we're already failing that due to non-roundtrip... making it cause a failure is much more explicit
16:30 <@andytoshi> hehe, i went to complain about rusty's change in the context flags and found that i'd concept-ACKed it weeks ago
16:31 < gmaxwell> Nothing obligates you to be consistent with your past-self.
16:33 <@andytoshi> phew
16:33 <@andytoshi> (actually, i still like this patch. i'd just forgotten about it and then was surprised when it got merged)
16:33 < sipa> oh, i should have pinged you on my rebase
16:34 < sipa> as you commented on the previous one
16:35 < gmaxwell> I worry a bit about our review uniformity. We have some limited evidence that our review is lacking, esp on re-review after 'minor' changes.
16:35 < gmaxwell> (e.g. the comment fixes I recently did are brown paper bags all around for the three of us. :) )
16:35 < sipa> yeah
16:35 < sipa> unfortunately, we don't have tests for comments
16:36 < gmaxwell> I suspect that after the last time they got rephrased no one actually read them except the author.
16:37 <@andytoshi> i read the comment fixes as i get emails for every commit that is pushed, with diffs
16:37 <@andytoshi> but i did not verify they were correct :/
16:38 < gmaxwell> What we should be doing is stepwise abstraction, which can also be applied to comments... but its annoying.
16:39 < gmaxwell> Basically stepwise abstraction is that you go through while reviewing and chunk at a time write what it does. (and I suppose for a comment, I suppose you'd restate what it says).  This forces you to actually read everything instead of slipping into glance-over mode.
16:42 <@andytoshi> that sounds exactly what we should be doing. and extremely time-consuming
16:44 < gmaxwell> a problem with review is that most people, if they don't understand some little thing that "looks typical", instead of stopping to understand it, or insist that a comment be added, they'll instead glance over. Only with continious conscious effort can you avoid doing this, and you can't always tell when you've slipped into that mode.
16:45 < gmaxwell> part of the reason I really dislike reviewing parsing code is that its full of these cases. Every array access you need to know for sure that it cannot be out of bounds, and unless the bounds check is on the same or prior line I find myself slipping into glance-over-mode.
16:46 <@andytoshi> right. i did exactly that with your comment changes, and thought "it's already merged, it must've been reviewed" plus "the DER changes are causing rust-bitcoin unit test failures, that is much more pressing"
16:46 <@andytoshi> and with emails once they're marked read i never read them again :/
16:47 <@andytoshi> so there is a lesson about reviewing while distracted, which is also hard to do
16:47 <@andytoshi> hard to not do*
16:48 < gmaxwell> For parsing code we should probably be leaning on formal proof for the bounds checks.
16:49 < gmaxwell> (sadly, languages like rust are not a replacement here; since we need to guarentee consistent error free behavior; and all rust will do is promise that if the code is wrong it will result in a task failure.)
16:50 <@andytoshi> very true. rust's static analysis around numerics are very lacking right now
16:50 <@andytoshi> e.g. you cannot use integers as template parameters, its match-statement exhaustiveness checking cannot prove things beyond "is the whole range of this type covered"
16:52 < gmaxwell> There is some PHD student that is working on more formalization of rust that would perhaps result in sound static analysis tools. e.g. "can this code result in task failure?"
16:52 < gmaxwell> but considering how slow this has gone in C, which is a much simpler language, I don't expect immediate results. :)
16:52 < sipa> review of #322 would be welcome
16:53 < sipa> if you just want the non-code version: https://github.com/sipa/secp256k1/blob/d0cb77f0da580148190c6177b7734df906546781/src/modules/schnorr/schnorr.md
16:54 < gmaxwell> sipa: can you also ask adam to review?
16:55 < sipa> ack, will do
16:57 < gmaxwell> He's been spinning his wheels on making CT smaller. I think he's actually finally come up with something that would make it smaller... but it means breaking out of the normal range proofs, and it also wants a base-3 number representation... which is kind of annoying.
17:12 <@andytoshi> sorry for missing discussion on this, but can the contrib/laxder functions validate the bitcoin and testnet chains?
17:12 <@andytoshi> (up to this point in time)
17:14 < gmaxwell> andytoshi: thats the intention. "up to this point in time" -> STRICTDER means they damn well better in the future!
17:15 <@andytoshi> ok, great. that does match my beliefs based on mostly-skimming your conversations
17:16 < gmaxwell> andytoshi: Pieter seems to be opposed to mentioning bitcoin in libsecp256k1 docs, :) but the intention of that function is 99% do what is needed for Bitcoin.  I dunno if pieter has tested against testnet though!
17:17 < sipa> good idea!
17:17 <@andytoshi> hehe, i might do this. for now at least i'll do the dozen or so testnet transactions that gave me trouble when i was first writing rust-bitcoin
17:17 < gmaxwell> sipa: can we mention bitcoin in contrib at least?  I do worry that if we do not people who need that won't use it. Or perhaps include w/ libsecp256k1  "application guides"  that cover "How to use libsecp256k1 with X".
17:17 <@andytoshi> though they are selected for weird script behaviour, not signature formatting..,
17:18 < gmaxwell> andytoshi: sipa has patches that switch bitcoin core to libsecp256k1, so he should only need reindex testnet with them.
17:18 < sipa> gmaxwell: that's fine
17:18 <@andytoshi> +1 to mentioning bitcoin in contrib. but i also prefer it not be mentioned anywhere else
17:19 < sipa> contrib is essentially "here is some code that may be useful in specific instances"
17:19 < sipa> i'm rebasing the secp256k1 patch now
17:20 < gmaxwell> other kinds of confusion, for example the joinmarket people were of the impression that they had to continue using the horrifying pybitcointools library because libsecp256k1 "did not support BIP32".  So "how do you do bip32 with this library" would be an example application note.
17:24 < gmaxwell> sipa: Would you be opposed to base58 encode/decode code in contrib?  (would make it possible to ship a BIP32 keygen)
17:26 <@andytoshi> gmaxwell: we also need sha2 and hmac no?
17:28 <@andytoshi> (i would actually like us to expose these functions. but i'm not strongly convinced it's more worthwhile than keeping the library's scope limited)
17:28 <@andytoshi> and i know pieter is opposed :)
17:29 <@andytoshi> i'm ok with hash/hmac specifically because they are so hard to screw up in a way that test vectors won't notice, and also really hard to put timing vectors into. both unlike basically any other crypto
17:40 -!- evoskuil [~evoskuil@c-73-225-134-208.hsd1.wa.comcast.net] has quit [Ping timeout: 260 seconds]
17:43 < sipa> gmaxwell: how about putting actually useful example programs in contrib?
17:43 < sipa> like a bitcoin keygen tool
17:45 < sipa> ah, but that needs sha256
17:46 < sipa> meh
17:57 < sipa> gmaxwell: seems the ber private key code in contrib you wrote doesn't actually compile outside out libsecp256k1... it uses the secp256k1_scalar type
17:57  * sipa fixes
18:10 -!- evoskuil [~evoskuil@c-73-225-134-208.hsd1.wa.comcast.net] has joined #secp256k1
18:38 < gmaxwell> sipa: oh darn, I intended to check for that and forgot it.
18:39 < gmaxwell> thats what I get for asking if you were okay with doing it before finishing it. :P
18:39 < sipa> gmaxwell: i should have checked of course
18:39 < sipa> but it needs a significant change
18:39 < sipa> uses a lot of internal functions (unnecessarily, it seems!)
18:40 < gmaxwell> I assume it should use the same approach as the other one (using the compact form)
18:41 < sipa> secret keys don't need internal serialization
18:41 < sipa> they're just 32 byte arrays
18:41 < gmaxwell> we have no tests for it either.
18:43 < gmaxwell> (well not absolutely none, but e.g. not enough to know if the parser is safe for hostile input)
21:04 < gmaxwell> reading the lax_der_privatekey_seralizer hurts my soul.
22:09 < sipa> i can do much better now :)
22:27 < gmaxwell> having it not emit excessively padded der would be nice at least.
22:28 < gmaxwell> Of course I can do this too, but I'm worried how deep the rabbit hole goes, we shouldn't spend much time on it.