--- Day changed Mon Aug 31 2015
01:50 -!- jtimon_ [~quassel@m952736d0.tmodns.net] has quit [Ping timeout: 265 seconds]
09:58 -!- jtimon [~quassel@md42736d0.tmodns.net] has joined #secp256k1
11:16 -!- jtimon [~quassel@md42736d0.tmodns.net] has quit [Ping timeout: 264 seconds]
12:53 -!- nullbyte [NSA@gateway/vpn/mullvad/x-pfwdhvcuaquketbq] has joined #secp256k1
14:18 < sipa> andytoshi, gmaxwell: see my 'sage' branch
14:19 < sipa> it correctly detects that the old gej_add_ge was incomplete, but i had to add an explicit test for the addition law with a.y == -b.y
14:22 < andytoshi> i wonder if we could tag the source comments so that you could autogenerate these formulae from the source
14:23 < sipa> they're a transliteration of the C code, not of the comments
14:23 < andytoshi> oh wow
14:23 < andytoshi> can we do that automatically?
14:23 < andytoshi> it's not a huge deal, obvs any changes to those functions would get flagged in review..
14:25 < gmaxwell> It would be ideal, but perhaps going the other way would be easier.  The stuff on hyperelliptic is apparently intended to do that.
14:25 < sipa> i think it's possible to detect the incomplete addition law
14:26 < sipa> but it will require some equation solving
14:26 < gmaxwell> sipa: there is also the possibility of testing via exaustive test using Z/Z31.
14:26 < sipa> currently there are 2 sources of "zero" and "nonzero" assumptions, the implementations and the laws
14:27 < sipa> they cancel out when one's zero assumptions conflict with the other's nonzero assumptions
14:27 < gmaxwell> I wonder what is the smallest %3/%4=1,3 field where the resulting curve has prime order?
14:28 < gmaxwell> as that would be an ideal mock field.
14:28 < sipa> via CRT, that means %12==7 ?
14:29 < sipa> so 12, 19, 31, 43, ...
14:29 < sipa> ah, the curve has prime order too
14:29 < andytoshi> 7, 19, 31, 43 you mean
14:29 < sipa> uhh, yes
14:29 < gmaxwell> yea, might not be any solutions, the prime would need to be <16 bits or so .. so that you could do n^2 tests in reasonable time.
14:33 < andytoshi> mod 7 our curve eqn is y^2 = x^3 which by my count has 7 points
14:33 < andytoshi> just by writing out all the squares and cubes on paper..
14:39 < gmaxwell> andytoshi: well you can't really represent the curve equation in F(7). :P
14:39 < andytoshi> oh, fine :P
14:39 < andytoshi> sipa: github won't let me comment on the branch directly but line 163 `assumeNonZeroAddingOpposite = assumeNonZero + [ax - bx, ay - by]` i think is wrong .... ax - bx should be assumeZero not assumeNonZero (?)
14:41 < sipa> andytoshi: the addingopposite is maybe confusing, but it is intended to test the ax!=bx ay==-by case
14:42 < andytoshi> oh i see
14:42 < andytoshi> and ax == bx  ay == -by is covered by regular Adding?
14:42 < gmaxwell> so there are 1636 7mod12 primes under 65536.
14:42 < gmaxwell> I'll test all the curves.
14:43 < sipa> andytoshi: everything is covered by everything
14:44 < gmaxwell> first one is 43
14:45 < gmaxwell> 67/79/127/163/311/547/823907/967/991 are all also solutions.
14:45 < andytoshi> sipa: i'm confused. i see like `assumeNonZeroAdding = assumeNonZero + [ay + by, ax - bx, ay - by]` which looks to me like the "standard add" case explicitly assumes all of: ay != -by, ax != bx, ay != by
14:46 < sipa> right, that's correct
14:46 < sipa> i was thinking about an earlier version of the code
14:46 < andytoshi> ah kk
14:46 < sipa> though you can remove that ay+by case from adding
14:46 < sipa> the adding formula also works for adding opposites
14:47 < gmaxwell> Probably the most useful mock field is the largest that an exaustive test finishes in reasonable time... as there may be behaviors that are trivally uninteresting for smaller fields.
14:48 < andytoshi> sipa: maybe we want to separate out adding opposites anyway in case we change the addition formula
14:48 < andytoshi> to one for which the "normal" branch doesn't cover adding opposites
14:48 < sipa> well, i'd like to get rid of the adding opposities case
14:48 < sipa> and combine it with adding
14:49 < sipa> and then go solve an equation to detect under which conditions it's producing unknown results
14:50 < sipa> which, for the last case, should find the ay=-by solution
14:58 < andytoshi> in any case, the current code has `ax - bx` in assumeNonZero for both Adding and AddingOpposite, so `ax == bx` is only covered by the doubling case. there you assume `ay == by`
14:58 < andytoshi> so i think P + -P isn't covered
15:01 < sipa> it explicitly isn't
15:01 < sipa> because P + -P has no affine solution
15:02 < sipa> it only tests the cases where both the inputs and the outputs are affine
15:02 < andytoshi> ahh gotcha
15:02 < sipa> s/affine/finite/
15:02 < andytoshi> affine and finite mean the same thing for us :)
15:03 < sipa> well affine is a representation, none of these formulas take an affine input
15:03 < sipa> they take a jacobian one that is representable in affine
15:04 < sipa> </nitpick>
15:04 < andytoshi> yeah, i see the "z != 0" assumptions everywhere, i shoulda clued into that
15:10 < andytoshi> sipa: overall the structure of this is really good, i like it
15:10 < andytoshi> i think you need some more comments (i guess you hope this is not the final form, that you can eliminate this branch business..)
15:12 < sipa> i don't think i'll be able to eliminate the branches
15:12 < sipa> you can't put boolean conditions inside polynomials
15:13 < sipa> well, i guess you can by introducing them as addition zero-or-one variables
15:13 < sipa> but i do want something that can detect incompleteness
15:14 < sipa> it shouldn't be hard
15:14 < sipa> just test whether expr.bot's nullspace is a subset of that implied by assumeNonZero
15:14 < sipa> or something
15:15 < andytoshi> i think that's exactly what you want
15:15 < gmaxwell> if you do them as zero or one and the other side of the branch you multiply up the order of the polynomial.
15:16 < gmaxwell> Which eventually results in sadness when you try to prove anything about it. :)
15:23 < andytoshi> sipa: ok, modulo missing comments i ACK the check_jacobian() function
15:23 < andytoshi> very clean, considering the number of cases you had to deal with
15:31 < andytoshi> sipa: in `prove` can you clarify why the line `nonzero = reduce((lambda x, y: x * y), [fastfrac(R, 1)] + assumeNonZero)` adds 1 to the list?
15:31 < andytoshi> does reduce() bomb out if you give it an empty list or something?
15:32 < andytoshi> oh, i guess it wouldn't know the type of its return value..
15:32 < andytoshi> never mind :)
15:34 < sipa> yeah, it makes the base element a fastfrac rather than a ring element
15:39 < gmaxwell> oh interesting: the commit on github fails w/ my copy of sage
15:39 < gmaxwell> duh, it's supposted to.
15:40 < gmaxwell> Probably should .. uh .. print that its supposted to fail. :P
15:40 < gmaxwell> rename old to "broken" :)
15:42 < sipa> glad somebody noticed that :p
15:42 < gmaxwell> would have before, but I only have sage on a single sometimes-on host. :)
15:43 < andytoshi> oh lol, i forgot i have sage on my laptop now
15:43 < andytoshi> (just replaced the drive with a big enough one a week ago)
15:43 < gmaxwell> sipa: would it make sense to break out the affine reference formulas?
15:44 < andytoshi> cool, i see the correct results
15:44 < sipa> how do you meam break out?
15:45 < sipa> ah, yes
15:45 < gmaxwell> oh it's broken out, never mind, I thought it was just implemented inline in check_jacobian.
15:46 < gmaxwell> oh its not.
15:46 < sipa> ideally we make a and b ring bases again
15:46 < sipa> and have the a=0 formulas specifically enforce that
15:47 < sipa> i'd like to try removing those reference formulas
15:47 < sipa> and only have implicit ones (colinear, tangetial, curveeq)
15:47 < sipa> but other things first
15:48 < gmaxwell> I want to add code that uses the formula over Fp_43 and constructs every possible pair of jacobian points, and verifies that their addition agress with the affine reference.
15:50 < andytoshi> gmaxwell: our `#include field_xyz_impl.h` model lends itself quite nicely to that
15:51 < gmaxwell> andytoshi: yea, I did think about doing that in the codebase... though I expect a bunch of things will break, e.g. the generator no longer fits in the resulting field.
15:52 < gmaxwell> and beta/lambda have different vaues, obviously.
15:52 < andytoshi> you'd need a separate tests_43.c i think with all the constant vectors removed or changed
15:52 < sipa> yuck
15:53 < andytoshi> and anyway it'd probably do pretty different stuff, just explicitly check everything and call it a day
15:53 < sipa> the code isn't particularly intended to have the field properties confogurable
15:53 < gmaxwell> alternatively it would be pretty easy to do the exaustive check in sipa's external verification. though not quite as nice as it's vulnerable to transcription errors.
15:55 < sipa> so we just need an interpretable group law implememtation data structure, and use it to generate the code, and for verification :)
15:55 < sipa> oh, descending towards EWR
15:55 < sipa> probably going to lose connection soon
15:55 < gmaxwell> connect safely. :)
15:56 < gmaxwell> yea, machine generating both would be nice.  The datastructure must represent the normalizations and such. bleh.
15:57 < gmaxwell> speaking of machine blah blah code. I wonder if I can find a clang-format set of settings that almost exactly matches our current code style and apply it immediately after the _t change.
16:15 < gmaxwell> Luke-Jr: do you still have a working x32 setup?  I'd like to know if libsecp256k1 is working there.
16:16 < Luke-Jr> I think so
16:16 < Luke-Jr> sec
16:16 < sipa> no reason why it wouldn't
16:16 < gmaxwell> (in particular, does it manage to use the 64-bit asm?  and does GMP work?)
16:16 < sipa> yes to both, iirc
16:18 < sipa> there were some changes made specifically to the 64-bit asm to make it work on x32
16:18 < sipa> though that may have been on an earlier generation of the code
16:19 < gmaxwell> yea, thats why I was suggesting testing!
16:21 < Luke-Jr> my x32 chroot is old, but no reason to think updates would break it AFAIK
16:22 < gmaxwell> Luke-Jr: agreed.
16:22 < sipa> just saying that at one point x32 worked in full glory... whether it still does is obviously to be tested
16:23 < gmaxwell> sipa: yea, thats what I was asking for. :)
16:23 < gmaxwell> I still don't know how to address the mixed gmp problem.
16:23 < midnightmagic> I have a few 32-bit machines on a 64-bit host, and I have a pure 32-bit machine (intel celeron m) also
16:23 < Luke-Jr> midnightmagic: that's not the same thing
16:23 < gmaxwell> midnightmagic: x32 is a special weird beast, it's x86_64 but with 32 bit pointers.
16:24 < midnightmagic> Ahhh, right. Ok.
16:25 < Luke-Jr> configure: Using assembly optimizations: x86_64 configure: Using field implementation: 64bit configure: Using bignum implementation: gmp configure: Using scalar implementation: 64bit configure: Using endomorphism optimizations: yes
16:25 < gmaxwell> Luke-Jr: and the tests pass?  turn on benchmarks too and see that they all run?
16:25 < Luke-Jr> (x32) ishibashi secp256k1 # ./bench_ecdh ecdh: min 57.7us / avg 58.7us / max 60.7us
16:25 < gmaxwell> probably should create a test script for what we want tested on weird systems.
16:26 < sipa> andytoshi: has a fancy test script
16:26 < gmaxwell> nice and fast.
16:26 < midnightmagic> Does it require a special CPU?
16:26 < gmaxwell> midnightmagic: does x32? no it works with any x86_64.
16:26 < andytoshi> my script is super slow, right now it tests 512 ./configure options..
16:26 < andytoshi> i also have a smaller one which checks like a dozen (in much more depth, it does valgrind and everything)
16:26 < Luke-Jr> gen_context is useful?
16:26 < gmaxwell> probably should run ./tests 1 but make sure to do _every_ build combination. (which there are thousands of now)
16:27 < gmaxwell> Luke-Jr: gen_context makes the signing table a static r/o text segment instead of being computed at inittime.
16:27 < Luke-Jr> tests & benchmarks http://0bin.net/paste/MLECKD911KJ1GlPM#C+aZlhkVzFoQCvlxVgUX6oDyYpIceIZjoWgyLlrClAZ
16:27 < andytoshi> gmaxwell: http://0bin.net/paste/9GI6lHjv6pZ9LwvI#l2JqKKuN7xgb+HVAAmqcccW15yrbhx5glq57ZE9bdk2 here's one that does that
16:27 < andytoshi> tho with only 512 combos
16:27 < Luke-Jr> src/bench_sign.c:35: test condition failed: secp256k1_ecdsa_signature_serialize_der(data->ctx, sig, &siglen, &signature)
16:28 < Luke-Jr> but tests passed..
16:28 < sipa> ugh
16:28 < sipa> that looks bad
16:28 < gmaxwell> sweet!
16:29 < gmaxwell> maybe we should make a verification subdirectory for something like andytoshi's thing and for the sage notebook?
16:30 < gmaxwell> andytoshi: if benchmarks are enabled it should run all of them. :)
16:30 < andytoshi> gmaxwell: that'd take days :)
16:31 < gmaxwell> we might want to make the benchmark durations variable and put that feature behind a switch.  But absent it, we'll end up shipping software with build configurations where no one has even run all the programs, which isn't so awesome.
16:34 < andytoshi> that's a good point
16:35 < andytoshi> oh, does `distcheck` run the full tests binary?
16:35 < andytoshi> maybe i meant just `dist`
16:35 < gmaxwell> and FWIW, I've done many many cpu years of testing on the software; some test suite that takes days to run isn't unreasonable to have.
16:36 < Luke-Jr> anything else I should do to help debug this?
16:36 < gmaxwell> Luke-Jr: whats your configure commandline exactly?
16:36 < Luke-Jr> (x32) ishibashi secp256k1 # ./configure --enable-benchmark --enable-endomorphism --enable-module-ecdh --enable-module-schnorr --enable-module-recovery
16:37 < gmaxwell> Luke-Jr: can you valgrind it?
16:37 < Luke-Jr> no, Valgrind does not support x32 :/
16:37 < Luke-Jr> (otherwise I'd probably run it native!)
16:38 -!- cfields [~quassel@unaffiliated/cfields] has joined #secp256k1
16:39 < Luke-Jr> (neither does LLVM, so AddressSanitizer is out..)
16:39 < gmaxwell> Luke-Jr: oh nice, fails for me too.
16:39 < Luke-Jr> O.o
16:39 < gmaxwell> on x86_64 with master and that commandline.
16:39 < Luke-Jr> I figured that was the typical "test everything" configure XD
16:40 < gmaxwell> It is, travis is likely screwed up in that we're not seeing a failure.
16:40 < cfields> what's the problem?
16:40 < gmaxwell> fails with --enable-benchmark  and nothing else.
16:40 < gmaxwell> cfields: recent commit introduced a benchmark failure. presumably no big deal.
16:41 < cfields> gmaxwell: travis didn't catch it, though?
16:41 < midnightmagic> https://github.com/bitcoin/secp256k1/ this is what you all are working off of right now?
16:41 < Luke-Jr> midnightmagic: yep
16:42 < gmaxwell> cfields: yea, maybe travis doesn't run bench_sign  or doesn't notice if it fails?
16:42 < gmaxwell> I don't know much about the travis config.
16:42 < cfields> gmaxwell: ah yea, i think it's disabled for time saving
16:43 < Luke-Jr> I don't see Travis's script even running tests.. just compiling
16:43 < cfields> gmaxwell: build failure or actual runtime bug? We could always build them but not run them
16:43 < gmaxwell> welp, it's busted; probably by 439d34adc6f145f91cbc99bcb332b323efe73829?
16:43 < gmaxwell> cfields: runtime.
16:43 < cfields> Luke-Jr: BUILD=check
16:43 < cfields> that's set by default
16:43 < gmaxwell> travis could back off ./tests to a smaller number like 1  and that would make things much faster.
16:44 < Luke-Jr> gmaxwell: ACK, confirmed 439d34adc6f145f91cbc99bcb332b323efe73829 is to blame
16:44 < andytoshi> ugh, im sure i ran my test matrix on that before acking it
16:44 < cfields> gmaxwell: it could also run benches on the fastest config, if you think that's worth doing
16:44 < andytoshi> ohh i did the big matrix because i expected that to be where failures appeared, but that one doesn't run benchmarks..
16:45 < andytoshi> sorry, process failure on my end
16:45 < gmaxwell> it's imperritive that something run them, doesn't have to be every config. Otherwise nothing at all tests the actual library that gets built.
16:45 < gmaxwell> (tests is a special compile)
16:45 < cfields> gmaxwell: ok, I'll PR that
16:45 < gmaxwell> andytoshi: If only someone suggested that it run the benchmarks! :P
16:45 < andytoshi> :P
16:46 < andytoshi> i'll add that now
16:46 < Luke-Jr> seems to me tests ought to do everything every benchmark does..
16:47 < gmaxwell> Luke-Jr: yes, more or less. It's not going to be, and shouldn't be exactly the same -- thats just a waste of time.
16:47 < gmaxwell> The tests, in particular, compile with a ton of magic instrumentation that probably means that any incorrect behavior resulting from miscompilation will not reliably show up.
16:49 < gmaxwell> I've felt this was okay in part because the benchmarks do checking, and I plan on having startup time test vectors (existing pull req for that is too slow, however)
16:50 -!- nullbyte [NSA@gateway/vpn/mullvad/x-pfwdhvcuaquketbq] has quit [Read error: Connection reset by peer]
16:51 < cfields> mm, sipa, what was the reasoning for splitting the subdirs into separate Makefile.in's ?
16:52 < cfields> i thought about bringing it up when you PR'd the first one, but it didn't seem worth discussing. Now that I'm looking to add a target for running the benches, it'd be much easier if they were all in one place
16:53 < cfields> if you don't have a preference, i'd like to have 'em under one roof
16:53 < gmaxwell> +1 for one makefile.in unless there is a concrete reason otherwise.
16:55 < andytoshi> cfields: you mean Makefile.am?
16:55 < andytoshi> i have no strong preference but i thought it "looked cleaner" to have them separate like that
16:55 < cfields> andytoshi: yea
16:55 < andytoshi> you'd know much better than i what actually makes sense
16:56 < cfields> andytoshi: if they were as complicated as bitcoin's i'd agree, but as-is it just kinda tangles things imo
16:57 < Luke-Jr> gmaxwell: maybe build a single test+benchmark binary both the super-instrumented and using-the-shared-library way?
16:57 < TD-Linux> gmaxwell, that parallel build bug is surprising, is gen_context.h not being picked up automatically as a dep?
16:57 < gmaxwell> Luke-Jr: the tests are full of access to the internals (and for good reason).
16:58 < gmaxwell> TD-Linux: not sure why it breaks. if I make clean then make -j1 then it seems to fix it even after another make clean.
16:58 < cfields> parallel build bug? I thought i fixed that with the last changes there
16:58 < gmaxwell> I am absoutely confident that master is having occasional parallel build failures as of right now.
16:59 < cfields> grrgh, ok
16:59 < cfields> gmaxwell: can you give me a configure config that you've hit it with by any chance?
17:00 < gmaxwell> of course I only encounter it when I'm trying to work on something else!
17:00 < gmaxwell> cfields: the failure I hit is on benchmarks.  trying to reproduce now to give you a report.
17:00 < cfields> ok
17:00 < sipa> cfields: i have also hit it occasionally
17:00 < sipa> after the last fix
17:01 < cfields> mm, ok, got it. that was easy
17:01 < cfields> ah, i bet i know
17:01 < gmaxwell> whew, glad to hear you say that as its not reproducing for me this second. :)
17:02 < cfields> gmaxwell: "make -j" makes parallel issues very shallow :)
17:02 < cfields> (bare -j, that is)
17:02 < gmaxwell> tried that!
17:06 < cfields> ok, fix is a little messy, but i'll fix it up to be cleaner if i can combine the makefiles
17:06 < cfields> sipa: ok with that? or did you have a specific reason?
17:07 < sipa> cfields: just ocxasionally after a make clean
17:08 < sipa> i always build with "make -j11 check"
17:08 < sipa> with benchmarks enabled
17:08 < cfields> sipa: yea, and only with benchmarks on i think
17:08 < cfields> right
17:08 < gmaxwell> I think I've only seen failures with benchmarks enabled.
17:10 < sipa> cfields: i had hoped to keep the modules as independent as possible, and move as much out of common code
17:10 < sipa> there are a few violations still, but i'd rather move in the other direction if possible :)
17:11 < cfields> ok
17:18 < cfields> this fixes: http://pastebin.com/raw.php?i=aJbiyM64
17:18 < cfields> unfortunately i don't see an easy way to avoid hard-coding
17:22 < gmaxwell> sipa: that failure is because benchsign does not re-increase the buffer size in the loop.
17:22 < gmaxwell> this is something of an API footgun-- a downside of an inout argument-- that maybe we should call out...
17:22 < gmaxwell> -    int siglen = 74;
17:22 < gmaxwell>      for (i = 0; i < 20000; i++) {
17:22 < gmaxwell> +        int siglen = 74;
17:22 < gmaxwell> is a sufficient fix.
17:22 < sipa> oh!
17:23 < andytoshi> excellent :)
17:24 < gmaxwell> probably for the sign api is that there is only a ~1:256 probablity of it coming out smaller or something like that... pretty easy to get that wrong.
17:25 < Luke-Jr> I wish C had a standard byte-counted buffer type :/
17:26 < Luke-Jr> I have a small set of inlines to manage one for BFGMiner, but I'm not sure if it makes sense to import it
17:27 < gmaxwell> meh, it's very hard to be somewhat higher level and not end up with something like Objective C once you've solved all the issues that crop up.
17:30 < TD-Linux> or follow that path to the logical extreme and wind up with rust
17:32 < gmaxwell> and again, rust is a very complex language (compared to C)
17:33 < TD-Linux> so dangerously complex that a second implementation isn't really feasable
17:34 < gmaxwell> I should redirect the pepole howling about multiple implementations of the bitcoin consensus rules and go have them do battle with the rust developers. :)
17:34 < TD-Linux> maybe they can write alternate implementations of each other's projects!
17:35  * TD-Linux suspects this would lead to a rust implementation in Java
17:37 < sipa> lol
17:38 < sipa> and a Java implementation in Bitcoin Script
17:42 < TD-Linux> wouldn't be worth it unless it was a JIT (with OP_EXEC or whatever it was called)
17:47 < cfields> gmaxwell: to confirm, have travis run all benchmarks for a single implementation? i assume the previous bench failure caused a crash?
17:49 < sipa> cfields: sg
17:50 < cfields> ok, I'll PR those two
17:59 < Luke-Jr> secp256k1.h: Why the magic to define SECP256K1_INLINE here rather than using autotools' stuff?
17:59 < Luke-Jr> hm, I guess it's possible the user builds with a different compiler they use later
17:59 < sipa> Luke-Jr: because having the header file independent from the implementation avoids a lot ofvproblems
17:59 < Luke-Jr> secp256k1_context_create flags param: maybe it should be at least unsigned?
18:00 < sipa> makes senae
18:00 < sipa> sense
18:00 < Luke-Jr> secp256k1_context_destroy: should NULL be allowed as a no-op like free()?
18:01 < Luke-Jr> (I'll put together a PR from responses here)
18:02 < sipa> not very compatible with needing exclusive access to it
18:03 < sipa> but i guess in fully single-threaded apps it may be useful
18:36 < Luke-Jr> ?
18:39 < Luke-Jr> secp256k1_context_set_illegal_callback: How do I restore the default?
18:39 < sipa> Luke-Jr: good question!
18:40 < sipa> NULL would be perfectly reasonable
18:40 < Luke-Jr> I'll make NULL restore it to the default?
18:40 < sipa> ack
18:40 < Luke-Jr> k
18:40 < sipa> Luke-Jr: in a muktithreaded application you cannot not know whether a context pointer is initialized or not, or you'd have a race error when using it
18:42 < Luke-Jr> sipa: use case of void reinit_ctx { mutex_lock; destroy(ctx); create(&ctx); } seems reasonable?
18:43 < sipa> ok, agree
18:44 < Luke-Jr> would it be bad to allow secp256k1_context_set_illegal_callback to accept const void *, and pass it to the callback as void *?
18:47 < Luke-Jr> Is there a reason size_t is not used for size types?
19:03 < sipa> Luke-Jr: can probably be changed; the size of anything that's intended to be reresentable in memory should be size_t
19:03 < sipa> l	btw, thanks a lot for doing API review, but note that there are are a few PRs with significant changes unmerged
19:04 < sipa> (order of arguments and removal of _t)
19:06 < Luke-Jr> why removal of _t?
19:06 < gmaxwell> because it's reserved by posix.
19:06 < Luke-Jr> ah
19:08 < gmaxwell> Luke-Jr: re INLINE: compatiblity with non-autotools builds; also inline is a C99 feature.  (in particular, only the very latest MSVC can do C99 inline.)
19:08 < Luke-Jr> O.o
19:09 < gmaxwell> MSVC is really a C++ compiler; C++ and C diverged at C89 and C++ is not a superset of C99. For a while MSVC refused to implement _any_ C99 features, and really only agreed to do any of them after C11 made some of C99's features optional.
19:10 < gmaxwell> (I think var arrays were made optional, for example)
19:10 < gmaxwell> there are also a lot of funky embeded compilers that are C89 'mostly'  (though every once I've seen does inline)
19:11 < gmaxwell> s/once/one/
19:11 < Luke-Jr> C++11?
19:11 < Luke-Jr> oh, nm, misread
19:13 < gmaxwell> Irritatingly, MSVC has inline, but calls it __inline.
19:13 < gmaxwell> (well I mean older versions)
19:17 -!- btcdrak [uid52049@gateway/web/irccloud.com/x-ivmwjyzzmbarleix] has quit [Quit: Connection closed for inactivity]
21:07 < Luke-Jr> cfields: gmaxwell: parallel build failure: http://0bin.net/paste/3tNEgI+g5Gk06cUm#rQaJLKF5UIUdVyA2TBTmWvj+GmuBenFw0KbrvcPgDTp
21:08 < cfields> Luke-Jr: https://github.com/bitcoin/secp256k1/pull/298
21:08 < cfields> Luke-Jr: would be great if you could verify
21:08 < gmaxwell> cfields: I saw your pull and thought "thats it?!" :)
21:09 < Luke-Jr> cfields: sadly, it is not reproducible
21:09 < cfields> gmaxwell: hehe, make stuff is apparently voodoo to people who don't hack on it often
21:12 < cfields> pretty straightforward in this case. bench_internal-bench_internal.o requires src/secp256k1.c which includes ecmult_static_context.h. So it must already exist. Usually that logic is handled automatically, but this is a weird case since ecmult_static_context.h is a generated file
21:14 < cfields> fwiw, this would be moot if it were a .c instead of a .h. What's the reason for including it directly? Surely it'd be preferred for it to end up in a single object?
21:35 < TD-Linux> the original reason was because all of secp256k1 compiles as one .o
21:36 < gmaxwell> It is kinda nice to be able to cc -o foo.o -Dx -Dy -Dx src/foo.c  and call it done, but one more object wouldn't really hurt that.
21:36 < TD-Linux> but... the static object made by that .h should have already made in to secp256k1's .o?
21:40 < cfields> i don't really have a preference there, and there's not much burden for keeping it as a  .h really, but speaking objectively i don't really see how a .o is any more useful than a .a/.so
21:41 < cfields> in fact, i'd say the opposite, as most linkers tend to drag in entire .o's at a time, so a monolithic one implies more bloat (without lto or ffunction-sections anyway)
21:50 < Luke-Jr> sipa: thoughts on changing int compressed arguments to unsigned int flags?
21:52 < Luke-Jr> secp256k1_ecdsa_signature_parse_der: BER violations are supported, but how do we enforce BIP66? could reserialised and memcmp, but maybe it'd be more optimal to have a flag?
21:54 < Luke-Jr> secp256k1_nonce_function_t: should 'attempt' be a fixed-width int, rather than 'int'?
21:56 < gmaxwell> Luke-Jr: probablity of failure for our curve is 1:2^255-ish per try.
21:56 < Luke-Jr> gmaxwell: ?
21:56 < gmaxwell> attempt should never be anything but 0 and certantly not more than 2.
21:57 < gmaxwell> at least if the nonce function isn't broken.
21:57 < Luke-Jr> gmaxwell: but it's used as input for the hashing function, right? isn't it bad if 0 yields 2 bytes on one platform, and 4 bytes on another?
21:57 < gmaxwell> No, its not.
21:57 < Luke-Jr> it's no longer deterministic in that case, I would think
21:58 < Luke-Jr> well, across platforms
21:58 < gmaxwell> It's not an input to the hash function.
21:58 < Luke-Jr> oh
21:58 < gmaxwell> someone could make PRNG function where it was, but it would be up to you to pad it to a constant size.
21:59 < Luke-Jr> secp256k1_nonce_function_default: Should this be const, or maybe there is a reason to allow the library-user to modify it?
21:59 < gmaxwell> RFC6979 defines an infinite stream, the attempt counter basically tells the function how much of the determinstic infinite stream to skip.
21:59 < gmaxwell> oh I think that should be const.
22:00 < Luke-Jr> k
22:00 < gmaxwell> e.g. the way the multi-attempt interface works is very inefficient, but thats irrelevant because any retries are all are unobservably rare.
22:05 < Luke-Jr> gmaxwell: any thoughts on those other 2 q's?
22:06 < gmaxwell> Unsigned is generally bad practice in C where not needed, as it causes surprise promotions.
22:09 < Luke-Jr> gmaxwell: it's currently a boolean-style int
22:43 < gmaxwell> cfields: woah, you have an emulator that has from the future intel features running?
22:44 < gmaxwell> I don't have any ADX hardware right now, but it's actually shipping at least.
22:45 < gmaxwell> cfields: for bitcoin core, I think a lot of the sha256 time is in hashtrees which _could_ effectively use 4-way SSE sha2. .. but, ugh.
22:45 < TD-Linux> I *had* TSX :(
22:46 < gmaxwell> lol
22:58 < Luke-Jr> https://github.com/bitcoin/secp256k1/pull/299
22:59 < Luke-Jr> TD-Linux: I had HCF once, I think.
23:00 < gmaxwell> Luke-Jr: oh dear, you didn't base that on top of the _t patch?
23:00 < gmaxwell> I feel sorry for you son.
23:00 < Luke-Jr> gmaxwell: <.<
23:00 < gmaxwell> cause that patch gonna need to get redone.
23:01 < Luke-Jr> meh, at least the hard part is over
23:01 < Luke-Jr> (review)
23:46 -!- btcdrak [uid52049@gateway/web/irccloud.com/x-kjoryacxoksdlykg] has joined #secp256k1