--- Log opened Fri Sep 24 00:00:16 2021
06:26 -!- siv2r093008922 [~siv2r@103.77.37.176] has joined #secp256k1
06:44 -!- siv2r093008922 [~siv2r@103.77.37.176] has quit [Quit: The Lounge - https://thelounge.chat]
08:03 -!- RubenSomsen [sid301948@user/rubensomsen] has quit [Ping timeout: 250 seconds]
08:03 -!- RubenSomsen [sid301948@user/rubensomsen] has joined #secp256k1
08:04 -!- elichai2 [sid212594@id-212594.hampstead.irccloud.com] has quit [Ping timeout: 268 seconds]
08:05 -!- elichai2 [sid212594@id-212594.hampstead.irccloud.com] has joined #secp256k1
08:25 < real_or_random> I finally found time to continue the work on static-only signing tables
08:25 < real_or_random> here's an issue. the exhaustive tests fail now here https://github.com/bitcoin-core/secp256k1/blob/master/src/tests_exhaustive.c#L425 
08:26 < real_or_random> because now there are only static tables and those are for the real group and not for the small group... of course we could precompute for the small tables but that's probably overkill 
08:27 < real_or_random> sipa: was the purpose of this check to double-check the generated elements or is this supposed to test ecmult_gen?
08:27 < real_or_random> if it's the former, I could simply remove it or replace it by some known-output test
08:27 < gmaxwell> how do the exhaustive tests work at all unless they get their own tables?
08:29 < real_or_random> oh indeed
08:29 < sipa> i think it's testing ecmult_gen
08:29 < real_or_random> we need the tables also in the _sign etc tests of course
08:29 < sipa> and indeed, everything group-related should be substituted out
08:30 < gmaxwell> exhaustive tests will just need their own tables. Fortunately the ought not be that big. :P
08:30 < sipa> well, group-related constants
08:31 < real_or_random> gmaxwell: yeah not big but then if they're not big, they need special casing and this will add complexity again (that I hoped I can remove)
08:32 < real_or_random> but yeah, it seems we need them
08:33 < gmaxwell> Or continue to have the exaustive test generate the 'static' table on the fly at start. E.g. just make the 'static' table for exaustive a define of a global static variable.
08:33 < gmaxwell> and copy the table generation code into the exhaustive test harness.
08:33 < sipa> i think the issue is more that the ecmult_gen code inherently uses the table
08:33 < sipa> if you want to test ecmult_gen, its need to have a table, and ideally one that works without much other code being changed
08:34 < gmaxwell> thats also true for ecmult though.
08:34 < sipa> but maybe it's not unreasonable to just drop that test, or replace it with a more limited-scope one
08:34 < real_or_random> yep, so far I have removed the possibility to generate tables entirely from the lib code (moved to gen_context.c). but maybe I need to bring it back. or move it to its own file and call it in the exhaustive tests
08:35 < sipa> because it's not like the particular structure of ecmult_gen is going to be exercised much in an exhaustive test where all scalars are 4-bit integers
08:35 < real_or_random> sipa: we could drop that single test but as greg points out, then other tests will fail
08:35 < gmaxwell> sipa: it tests infinity handling, no? otherwise very hard to test.
08:35 < sipa> oh right
08:36 < sipa> and we certainly do want to test things like ecdsa and schnorr signing, which rely on ecmult_gen
08:36 < real_or_random> we could mock ecmult_gen but that's not any better ^^ 
08:36 < gmaxwell> real_or_random: yeah, just generating the table on the fly for it should be fine. 
08:36 < sipa> right, that's a possibility
08:36 < sipa> on the fly sounds annoying, as it means the whole infrastructure for runtime tables needs to stay just for this?
08:37 < sipa> i'd just have precomputed tables for ecmult_gen too
08:37 < sipa> for the exhaustive case
08:37 < sipa> ecmult has the same iirc
08:38 < sipa> yes; see the start of src/ecmult_static_pre_g.h
08:38 < real_or_random> hm yeah it forces the window size
08:39 < real_or_random> we could do this. my plan for the library was to ship all three tables for now (PREC_BITS = 2, 4, 8)
08:41 < sipa> the PREC_BITS=8 one is 512 KiB... that's not crazy
08:41 < real_or_random> right, but large already 
08:41 < sipa> but if it'll be replaced with the multicomb, maybe it's better to not have that permanently on our git history?
08:41 < real_or_random> my feeling was I didn't want to include the tables three times in the repo for group sizes 13 and 199.
08:42 < sipa> i don't think it's unreasonable to force PREC_BITS=2 for exhaustive
08:42 < real_or_random> well, idk. I can accept that repo size is a useful dimension to look at but I don't think git history size is 
08:42 < real_or_random> unless we commit GiBs ^^
08:43 < real_or_random> yeah that's what I was thinking. we could force PREC_BITS=2 for the exhaustive. that's in line with the ecmult module   
08:44 < sipa> clearly PREC_BITS=8 is pointless for exhaustive
08:45 < real_or_random> then we won't need to make ecmult_gen more complex just for the tests (also, this would be somewhat pointless because we would be testing the test code partly and not the real code)
08:47 < sipa> indeed
08:47 < real_or_random> but generating it on the fly is also an option. I don't think it's too annoying in the end, I need to refactor gen_context anyway to be able to generate tables depending on PREC_BITS dynamically
08:48 < real_or_random> then I could also make it accept the generator as an argument. 
08:48 < real_or_random> ok anyway, that was a helpful discussion!
08:48 < sipa> i guess just declaring the precomputed table as non-const in exhaustive mode could work
08:49 < sipa> and then computing it at the start of exhaustive main()
08:49 < real_or_random> right
08:49 < sipa> (as opposed to keeping a pointer in the ctx etc)
08:50 < real_or_random> right, the pointer is gone already and this is something I don't want to bring back ^^
08:51 < gmaxwell> thats what I meant before, just making the 'table' file declare a static global non-const... and shimming some generate code in the test harness.
08:52 < real_or_random> do we care that the exhaustive tests use malloc?
08:52 < gmaxwell> I think we don't care if they use malloc.
08:52 < real_or_random> when making stuff dynamic, i simply switched to the heap because it's clear we don't care for gen_context.c
08:52 < sipa> i don't care
08:53 < real_or_random> ok
08:54 < gmaxwell> I mean, if it's easy to not use malloc, better to not use malloc.  But realistically exhaustive_tests aren't likely to ever get run on embedded targets, and the problems they catch are primarily algorithim ones that won't be limited to specific targets.
08:54 < real_or_random> indeed
12:39 -!- luke-jr [~luke-jr@user/luke-jr] has quit [Quit: ZNC - http://znc.sourceforge.net]
12:40 -!- luke-jr [~luke-jr@user/luke-jr] has joined #secp256k1
13:12 -!- luke-jr [~luke-jr@user/luke-jr] has quit [Quit: ZNC - http://znc.sourceforge.net]
13:13 -!- luke-jr [~luke-jr@user/luke-jr] has joined #secp256k1
14:30 -!- belcher [~belcher@user/belcher] has quit [Ping timeout: 252 seconds]
14:43 -!- belcher [~belcher@user/belcher] has joined #secp256k1
--- Log closed Sat Sep 25 00:00:17 2021