--- Log opened Mon Jul 01 00:00:41 2024
00:00 -!- achow101 [~achow101@user/achow101] has quit [Ping timeout: 268 seconds]
00:04 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has joined #secp256k1
00:06 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has quit [Read error: Connection reset by peer]
02:10 -!- achow101 [~achow101@user/achow101] has joined #secp256k1
03:57 < real_or_random> fanquake hebasto: there's an IRC meeting today at 15:00 UTC, where we could discuss https://github.com/bitcoin-core/secp256k1/issues/1549 (Config used by Core vs defaults and config in CI/testing) 
03:58 < hebasto> roger
05:49 -!- theStack [~theStack@95.179.145.232] has quit [Remote host closed the connection]
05:53 < elichai2> real_or_random: rust-secp256k1 should remove the 32 bytes requirement on BIP340 schnorr signatures, right? (Replacing `Message` with `&[u8]` in https://docs.rs/secp256k1/latest/secp256k1/struct.Secp256k1.html#method.sign_schnorr)
06:04 -!- jon_atack [~jonatack@user/jonatack] has joined #secp256k1
06:05 -!- jonatack [~jonatack@user/jonatack] has quit [Ping timeout: 264 seconds]
06:07 -!- theStack [~theStack@95.179.145.232] has joined #secp256k1
07:57 < real_or_random> elichai2: yes, the bip was updated to allow for arbitrary-length messages a while ago. (but not sure what the right decision is for rust-secp256k1 when it comes to API stability etc)
07:58 < real_or_random> in libsecp256k1, we have an additional convenience function _sign32 that takes exactly 32 bytes, because that is still what most people will need
08:01 < real_or_random> meeting  :)
08:01 < nickler> hi
08:01 < hebasto> hi
08:01 < sipa> hi
08:02 < real_or_random>    stratospher[m] theStack josie fanquake  (just randomly mentioning recent contributors ^^)
08:03 < real_or_random> topics?
08:03 < nickler> stratospher[m], theStack: thanks for the musig PR review, will address as soon as I can
08:03 < real_or_random> yes, I think musig is currently high prio for review but it seems this is on the right track
08:04 < real_or_random> I'd like to discuss https://github.com/bitcoin-core/secp256k1/issues/1549, which is about our test config vs what's used in core 
08:04 < hebasto> +1
08:05 < real_or_random> for example, we decided to make 22 kb the default for the new ecmult_gen, but core now uses 86 kb for performance 
08:06 < real_or_random> I wonder if this is a bad idea given that almost all our testing uses 22 kb. (or even a good idea because we get more coverage of the config if core uses a different config)
08:06 < sipa> i think we can switch to 86 as default in general
08:06 < sipa> it's faster on both x86_64 and armv8 at least (based on benchmarks in 693)
08:07 < hebasto> if no drawbacks, I'd agree with sipa
08:07 < sipa> on some systems (embedded ones?) smaller sizes may be more desirable, but i'd expect them to manually pick that anyway
08:08 < real_or_random> at the moment, it feels slightly arbitrary  to me. or at least we don't have an agreed upon rule ... there's 22 kb vs 86 kb, which is a real algorithmic issue, but on the other hand, we had insisted in the past to let core configure with valgrind support enabled to make sure that the binary matches exactly what with ctime tests 
08:09 < real_or_random> sure, we could make that change in that specific case, and that may be the best option
08:09 < hebasto> fwiw, in core, ctime_tests compiled, but not run during `make check`
08:10 < real_or_random> but I'd also like to collect some general thoughts on this... does it make sense to align our testing with core?
08:10 < sipa> not necessarily, actually
08:11 < sipa> i think we should primarily test what libsecp256k1 considers its default configuration
08:11 < real_or_random> I'd tend to say yes, though that makes the other configs second-class citizens in some sense. (but I think that's always the case in software: defaults are better tested)
08:11 < hebasto> but if it would, tests can be skipped in core
08:11 < sipa> but it may make sense to make libsecp256k1's default configuration match bitcoin core's?
08:12 < real_or_random> sipa: probably? I mean it makes sense to target standard desktop machines with our defaults 
08:12 < sipa> yeah
08:12 < real_or_random> if you're embedded or on special platforms, you'll anyway want to set parameters 
08:13 < real_or_random> hebasto: I'm not sure if it's a good idea to skip tests in core. there can always be differences, e.g., in compiler versions etc. 
08:15 < real_or_random> depends the meaning of "skip"... where to skip? if tests are annoying for core, then we could also reduce the iterations
08:15 < real_or_random> but yeah, I think these are good rules of thumb: 1) we'd like to test mostly our defaults. 2) it makes sense to align the defaults with core 
08:16 < real_or_random> and that will imply making 86 our default
08:16 < hebasto> now, they take quite significant time 
08:17 -!- jamesob15 [~jamesob@108.44.248.162] has joined #secp256k1
08:17 < hebasto> to what extent is it reasonable to reduce the iterations?
08:17 -!- jamesob1 [~jamesob@108.44.248.162] has quit [Ping timeout: 264 seconds]
08:17 -!- jamesob15 is now known as jamesob1
08:18 < nickler> real_or_random: sounds like reasonable rules of thumb
08:18 < real_or_random> hard to tell... you can certainly run ./tests 4 and save a lot of time
08:19 < real_or_random> and the tests will still be very useful
08:19 < hebasto> sure
08:19 < real_or_random> but it's hard to give a reasonable number. in the end, the default of 64 is also arbitrary
08:21 < real_or_random> 4 needs 8.5 seconds on my laptop... seems reasonable if you ask me?
08:21 < hebasto> yes
08:22 < real_or_random> a related question was if we want Core to run the ctime tests as part of their tests. 
08:22 < real_or_random> this has a history: there's a very old PR where I suggested to make the ctime tests part of `make check` https://github.com/bitcoin-core/secp256k1/pull/723 
08:23 < real_or_random> but that was controversial due to the fact that some people on strange platforms will get failures and we don't have the resources to fix everything. and we'll just educate people to ignore the test results
08:24 < real_or_random> sipa suggested:  Could it be enabled by default just on platforms where we expect it to work?
08:24 < sipa> sorry, had a step away for a second
08:25 < sipa> let's reduce the default number of tests iterations?
08:25 < real_or_random> maybe?
08:26 < real_or_random> I don't know.  this seems like a thing where our CI could deviate from the default and still run 64
08:26 < sipa> i think it makes sense that the default tests iterations are lower, and libsecp256k1's CI runs it with higher-than-default
08:26 < real_or_random> yep
08:28 -!- jon_atack [~jonatack@user/jonatack] has quit [Read error: Connection reset by peer]
08:28 < real_or_random> or just reduce to 4 in core? I think 64 is a more reasonable default for e.g. distributors 
08:30 < fanquake> also unclear that they even take a long time in Core currently
08:30 < sipa> do we have any objective way of judging what a good iteration count is?
08:30 < real_or_random> hebasto: have people complained about this?
08:30 < sipa> heh, they clearly take a long time for me!
08:30 < sipa> when testing core
08:30 < hebasto> 30+ seconds
08:31 < real_or_random> sipa: I don't think there's an objective way when the goal is assurance. I just think that 10 seconds get less in your way when developing on core. but if you create the official arch linux build, you probably won't spending a few minutes  even
08:32 < sipa> parse error
08:32 < real_or_random> *won't mind
08:32 < sipa> ah
08:33 < real_or_random> (I'm not suggesting to raise it to a few minutes :P)
08:33 < real_or_random> have people in Core complained?
08:34 < hebasto> I did not hear from others
08:34 < real_or_random> I assume it's also not easy to make Core run only ./tests 4 because it just invokes the entire test suite of the subproject 
08:34 < sipa> i haven't either, but i find it personally annoying (i usually ctrl-c when i get to that part of the bitcoin tests...)
08:34 < hebasto> perhaps, only me
08:36 < real_or_random> but I'm okay with reducing the default to 4. it's certainly pragmatic 
08:36 < real_or_random> nickler: opinions?
08:36 < sipa> i don't think that running at more than 1 has ever actually contributed to a bug being found :p
08:36 < sipa> (it's debatable whether running with more than 0 has...)
08:37 < real_or_random> that's a good point. the "constant" tests probably cover most of the library already
08:38 < real_or_random> I think that convinces me.
08:39 -!- jonatack [~jonatack@user/jonatack] has joined #secp256k1
08:39 < real_or_random> but do we want to suggest running ctime tests as part of Core? they're cheap. 
08:40 < real_or_random> I think they should at least be run as part of the official build process. it's almost a bug that this is not done currently
08:42 < hebasto> are platforms problematic to run ctime_tests known?
08:42 < real_or_random> s390 :P
08:44 < real_or_random> given that's the only one known, I think enabling it in core is basically equivalent to enabling it wherever we except it to work
08:45 < real_or_random> and okay, you'll know valgrind to run them (or msan). we could enable it whenever valgrind is available. 
08:47 < hebasto> the same condition is used now to build ctime_tests by default
08:48 < real_or_random> and power9
08:48 < real_or_random> (but power9 was on the old ECDH code, perhaps that's fixed now)
08:49 < real_or_random> I assume both of these platforms are unsupported in Core? :P
08:49 < hebasto> they are
08:50 < hebasto> supported
08:50 < hebasto> however, not tested in CI
08:51 < hebasto> and not released
08:51 < real_or_random> hm ok 
08:52 < real_or_random> sipa: what do you think?
08:53 < sipa> is the suggestion to suggest bitcoin core's  manually test libsecp's ctime testz
08:53 < sipa> or that we add ctime tests to our default make check/test/whatever, if knows to work on the architecture?
08:55 < real_or_random> you suggested the latter in the past. I like the idea but it means we'll need to maintain a list of archtitectures
08:55 < real_or_random> I hoped that the former gets us roughly the known-working archs
08:56 < fanquake> i assume we just maintain an exclude list, rather then an include list?
08:56 < fanquake> until someone actually tests/complains, assume it works everywhere?
08:57 < fanquake> Also a good way to find out if tests are even being run in less maintain places
08:57 < fanquake> *mainstream
08:57 -!- preimage [~halosghos@user/halosghost] has joined #secp256k1
08:57 < real_or_random> sounds okay to me.
08:58 < real_or_random> (we can still see whether we want to start with s390 and power9 on the list, where we had reports in the past, e.g., https://github.com/bitcoin-core/secp256k1/issues/784 but that's a detail )
08:59 < sipa> yeah
09:00 < fanquake> re the other PR that prompted the discussion. I think that was basically out of scope for secp256k1
09:01 < real_or_random> hebasto: which other PR?
09:01 < fanquake> (been mostly afk sorry)
09:02 < hebasto> I'm confused
09:02 < hebasto> did I refer to other PR?
09:03 -!- ghost43 [~ghost43@gateway/tor-sasl/ghost43] has quit [Remote host closed the connection]
09:03 < fanquake> it’s listed in the issue description
09:03 < fanquake> for this meeting
09:04 -!- ghost43 [~ghost43@gateway/tor-sasl/ghost43] has joined #secp256k1
09:05 < real_or_random> sorry I wanted to mention fanquake 
09:05 < fanquake> the one where ccache was listed as a reason to change build defaults and the secp256k1 CI
09:05 < real_or_random> there are multiple PRs
09:05 < fanquake> https://github.com/bitcoin-core/secp256k1/pull/1547
09:06 < hebasto> fanquake: it is closed now
09:06 < real_or_random> ok yes
09:06 < real_or_random> okay... we're over the over hour already
09:07 < real_or_random> I'll post a brief summary in the issue, and create issues for any tasks
09:08 < real_or_random> which are
09:08 < real_or_random>   - change default to 86 kib
09:08 < real_or_random>   - change default to 4 test iterations (not on CI)
09:08 < real_or_random>   - add ctime tests to suite except for archs known to be problematic 
09:08 < fanquake> Just wanted to bring that up, as I don't think Core "stuff" like that, should be getting upstreamed to secp256k1, or influencing things here
09:09 < sipa> i think there are all useful improvements to libsecp
09:09 < real_or_random> fanquake: yeah, I think these things can be decided on a case-by-case basis. let's see what core ends up doing for MSVC
09:09 < real_or_random> ok
09:09 < fanquake> sipa: from 1547? Seems odd to change the build type, based on 1 flag that's likely to have no influence, and ccache, which isn't used, and likely isn't even a good choice for msvc
09:10 < fanquake> or is that in reply to the summary (whoops)
09:10 < sipa> fanquake: oh no my response was to real_or_random's summary
09:11 < real_or_random> :)
09:11 < fanquake> agree with the summary as well
09:12 < real_or_random> end of meeting?
09:13 < sipa> sounds like it
09:13 < real_or_random> ok
09:14 -!- roconnor_ [~quassel@coq/roconnor] has joined #secp256k1
09:14 -!- roconnor [~quassel@coq/roconnor] has quit [Ping timeout: 268 seconds]
09:23 < bitcoin-git> [secp256k1] real-or-random closed pull request #723: Add valgrind constant-time test to `make check` (master...202003-ct-make-check) https://github.com/bitcoin-core/secp256k1/pull/723
09:42 < bitcoin-git> [secp256k1] real-or-random opened pull request #1563: doc: Add convention for defaults (master...202407-convention-defaults) https://github.com/bitcoin-core/secp256k1/pull/1563
09:51 -!- preimage [~halosghos@user/halosghost] has quit [Quit: WeeChat 4.3.3]
09:51 -!- preimage [~halosghos@user/halosghost] has joined #secp256k1
11:09 -!- roconnor_ is now known as roconnor
11:23 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has joined #secp256k1
11:33 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has quit [Quit: My iMac has gone to sleep. ZZZzzz…]
12:00 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has joined #secp256k1
12:52 < bitcoin-git> [secp256k1] hebasto opened pull request #1564: build, ci: Adjust the default size of the precomputed table for signing  (master...240701-default-emsize) https://github.com/bitcoin-core/secp256k1/pull/1564
13:50 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has quit [Quit: My iMac has gone to sleep. ZZZzzz…]
14:23 -!- preimage [~halosghos@user/halosghost] has quit [Quit: WeeChat 4.3.3]
15:14 < bitcoin-git> [secp256k1] hebasto opened pull request #1565: cmake: Bump CMake minimum required version up to 3.16 (master...240701-cmake-bump) https://github.com/bitcoin-core/secp256k1/pull/1565
15:25 -!- jon_atack [~jonatack@user/jonatack] has joined #secp256k1
15:27 -!- jonatack [~jonatack@user/jonatack] has quit [Ping timeout: 255 seconds]
17:10 -!- ghost43 [~ghost43@gateway/tor-sasl/ghost43] has quit [Remote host closed the connection]
17:10 -!- HumanG33k [~HumanG33k@82.66.65.160] has quit [Ping timeout: 268 seconds]
17:11 -!- ghost43 [~ghost43@gateway/tor-sasl/ghost43] has joined #secp256k1
19:24 -!- ghost43 [~ghost43@gateway/tor-sasl/ghost43] has quit [Remote host closed the connection]
19:25 -!- ghost43 [~ghost43@gateway/tor-sasl/ghost43] has joined #secp256k1
23:37 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has joined #secp256k1
--- Log closed Tue Jul 02 00:00:41 2024