--- Log opened Mon Dec 19 00:00:49 2022
03:25 -!- ghost43 [~ghost43@gateway/tor-sasl/ghost43] has quit [Remote host closed the connection]
03:26 -!- ghost43 [~ghost43@gateway/tor-sasl/ghost43] has joined #secp256k1
05:11 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has quit [Quit: My iMac has gone to sleep. ZZZzzz…]
05:29 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has joined #secp256k1
05:59 < real_or_random> nickler: mind reviewing https://github.com/bitcoin-core/secp256k1/pull/1178 ? it has one ACK already
06:39 -!- andytoshi [~apoelstra@user/andytoshi] has quit [Remote host closed the connection]
06:56 -!- andytoshi [~apoelstra@user/andytoshi] has joined #secp256k1
07:00 < real_or_random> meeting :)
07:01 < hebasto> hi
07:01 < real_or_random> nickler sipa hebasto fanquake 
07:03 < nickler> hi
07:03 < real_or_random> :) maybe pieter will still join but let's start with collecting topics
07:04 < nickler> I have nothing right now
07:04 < real_or_random> one thing I'd like to discuss is how we deal with the changelog in the future
07:04 -!- jonatack2 [~jonatack@user/jonatack] has quit [Ping timeout: 268 seconds]
07:05 < real_or_random> and what the focus for the next release is
07:05 < sipa> Hi.
07:07 < sipa> Well what are people working on? Or want to be working on?
07:08 < hebasto> after #1178 I'll be able to finalize #1113
07:09 < sipa> I'll have a look at 1178 when I get to the office.
07:09 < sipa> It's a nice cleanup.
07:09 < real_or_random> i don't know... my gut feeling is that I'd like some more "maturing" towards a 1.0.0. for example, we've been talking about better build docs forever. this is something that I'd personally suggest for a next release. Another issue of similar kind is improving the examples    
07:10 < sipa> I've been working on the ellswift PR (and a writeup about it: https://github.com/sipa/secp256k1/blob/wip_ellswift/doc/ellswift.md)
07:10 < real_or_random> but I'm open to basically everything. if we decide that the next release should have ellswift, this is another goal
07:10 < sipa> And generating a test vector set for SDMC.
07:10 < real_or_random> I mostly think that having a concrete goal makes it easier to organize and focus our efforts, so this is more a meta thing
07:11 < sipa> Right.
07:12 < real_or_random> (and we can have more than one goal) nickler: what would you like see in a next release?
07:14 < sipa> There is also refining the release process of course.
07:14 < real_or_random> indeed.
07:16 < real_or_random> maybe move to this to give nickler time to respond... one thing I'm not sure about is how we add things to the changelog. some PRs do it, and some don't  
07:17 < real_or_random> maybe this is just good enough. if the contributor does it, then great. if not, we'll add it in a separate PR.  
07:17 < real_or_random> (I think we talked about this earlier but I'm not entirely sure if I remember the conversation.) 
07:21 < sipa> I think that makes sense.
07:21 < sipa> When reviewing a user-impacting PR, we can suggest the author adds release notes for it.
07:22 < sipa> But if not, we can add them later.
07:22 < real_or_random> indeed. it may also depend on the responsiveness of the author, etc. no need for a general rule
07:23 < real_or_random> this may be a good tag even for closed PRs: needs-changelog-entry
07:24 < real_or_random> (still want to add tags but I had prioritized the work on the release)
07:24 < sipa> oh yes
07:24 < sipa> i like that idea
07:27 < real_or_random> sipa: what do you think of https://github.com/bitcoin-core/secp256k1/pull/1135 ? should we have this? if yes, I think it's basically ready
07:28 < hebasto> actually, it needs a release of LIEF v0.13.0
07:28 < real_or_random> ah right, and there's fanquake's comment about it
07:29 < real_or_random> which makes it work with LIEF 0.12.3 ?
07:29 < sipa> So maybe a question: what kind of errors/bugs is this protecting against?
07:30 < fanquake> I just don't really see the need, for Python, LIEF, a new script that I don't think anyone will run, when you could probably just sanity check the ELF lib using a couple lines of bash in the CI
07:30 < sipa> Because if it's using a grep/search mechanism to figure out what is supposed to be exported, what could even go wrong?
07:30 < fanquake> I dont think there'd be a probably affecting any lib, that you wouldn't catch doing that, and then the "rules" would actually be getting enfoced on PRs
07:30 < real_or_random> hebasto: actually I don't know why we need 0.13.0 it works for me with 0.12.2 but maybe that is because I'm on linux
07:30 < fanquake> *problem
07:31 < hebasto> real_or_random: for macOS
07:31 < real_or_random> hebasto: ok... I see
07:31 < sipa> It seems more likely that a mistake is a missing/superfluous SECP256K1_EXPORT macro, which this check won't catch.
07:31 < real_or_random> I think the main point where I see > 0 benefit is that we forget to make an internal function `static` 
07:31 < hebasto> ^ this
07:32 < sipa> Ah, I see.
07:32 < real_or_random> but yeah, this would be a problem across architectures, and then maybe a few lines of bash would indeed suffice?
07:33 < sipa> What about the asm functions in .s files? Those are not static, but also not SECP256K1_EXPORT.
07:34 < real_or_random> ha, I have no idea how this works
07:35 < real_or_random> maybe we should export if these are exported
07:35 < real_or_random> *check
07:35 < hebasto> what functions in `field_10x26_arm.s` are supposed to be exported?
07:36 < sipa> None.
07:36 < sipa> But they're also not static as they're in a different compilation unit.
07:36 < sipa> So I'm confused what static has to do with this.
07:38 < real_or_random> hmm ok, I tried dropping `static` from a function and the script won't complain
07:38 < real_or_random> I think this is because we pass special flags to the compiler?
07:38 < sipa> .libs/libsecp256k1.so: export of symbol secp256k1_fe_sqr_inner not expected
07:38 < sipa> $ ./tools/symbol-check.py .libs/libsecp256k1.so
07:38 < sipa> .libs/libsecp256k1.so: export of symbol secp256k1_fe_mul_inner not expected
07:38 < sipa> etc
07:38 < real_or_random> -fvisibility=hidden 
07:38 < fanquake> Isn't the exporting of funcs also mitigated by -fvisibility=hidden, which is used by default?
07:39 < sipa> When I cross-compile for arm with asm enabled.
07:39 < sipa> static control access from other compilation units
07:39 < sipa> export/import controls access from outside the library
07:40 < real_or_random> hm yes, so it seems all of this is more subtle
07:40 < real_or_random> if the user uses our autotools (in the future cmake), then we have the appropriate flags
07:42 < real_or_random> if the user compiles manually, then the flags may not be present. but then there's the question whether we care. packagers won't do this, and manual compilation is most useful for settings like embedded where you may not want a shared library at all 
07:42 < sipa> fanquake: we have a SECP256K1_API macro that overrides the visibility for supposed-to-be-exported symbols.
07:43 < fanquake> sipa: right, but if the macro is missing, I think that'll be hidden by the compile options, and mitigate the usefullness of any script?
07:43 < sipa> But what the script right now does is check that the exported symbols corresponds to the presence of that macro.
07:43 < sipa> Right.
07:43 < sipa> Using SECP256K1_API + static causes a compile warning.
07:44 < sipa> So I think the cases the script catches are (apparently correctly) the .s file symbols being exported, and possibly weird standard library issues, but I'm much less concerned about this for C than for C++.
07:45 < real_or_random> yep though I think the "interesting" case is "no SECP256K1_API but also no static "
07:45 < hebasto> the initial purpose of that script was to ensure that CMake-based build system do no break library interface
07:45 < sipa> Do we want to look into making the .s file functions not exported?
07:45 < real_or_random> sipa: yes, independently of whether we want some script, we should try not export the .s functions 
07:45 < hebasto> to addition to `-fvisibility=hidden`?
07:45 < sipa> hebasto: Which really boils down to... making sure that -fvisibility=hidden is set?
07:47 < hebasto> sipa: can't recall all details now, but it seems I got a bit more issues...
07:47 < hebasto> ... across all supported platforms
07:47 < sipa> real_or_random: Ah yes, neither being set is a good thing to test for... but I don't think the script does that?
07:48 < sipa> If I drop a "static" for an internal function, it doesn't cause it to be exported.
07:48 < sipa> as expected, because the default visibility is still hidden.
07:48 < real_or_random> indeed
07:49 -!- halosghost [~halosghos@user/halosghost] has joined #secp256k1
07:49 < sipa> hebasto: That'd be interesting to know, what it caught.
07:49 < real_or_random> as I said, this is mostly relevant for manual compilation then but it's really questionable how important that is
07:49 < real_or_random> hehe:
07:49 < real_or_random> ./configure CFLAGS=-fvisibility=default
07:50 < real_or_random> ...
07:50 < real_or_random> ❯ python tools/symbol-check.py .libs/libsecp256k1.so
07:50 < real_or_random> .libs/libsecp256k1.so: export of symbol secp256k1_ecmult_gen_prec_table not expected
07:50 < real_or_random> .libs/libsecp256k1.so: export of symbol secp256k1_ecdsa_sign_inner not expected
07:50 < real_or_random> .libs/libsecp256k1.so: export of symbol secp256k1_pre_g not expected
07:50 < real_or_random> .libs/libsecp256k1.so: export of symbol secp256k1_pre_g_128 not expected
07:50 < real_or_random> .libs/libsecp256k1.so: export of symbol secp256k1_pre_g not expected
07:50 < real_or_random> .libs/libsecp256k1.so: export of symbol secp256k1_pre_g_128 not expected
07:50 < real_or_random> .libs/libsecp256k1.so: export of symbol secp256k1_ecmult_gen_prec_table not expected
07:50 < real_or_random> .libs/libsecp256k1.so: export of symbol secp256k1_ecdsa_sign_inner not expected
07:50 < real_or_random> .libs/libsecp256k1.so: failed EXPORTED_SYMBOLS
07:50 < real_or_random> ok the last one is one I introduced now for testing but the others are real
07:51 < real_or_random> so ok. I believe that if we have this implicit coding rule that we should add static to non-exported functions, we should have some qa thing to ensure we really do this
07:51 < sipa> We should probably add static all over the tests.
07:51 < sipa> As they don't have any statics.
07:52 < real_or_random> we could use the current script plus ./configure CFLAGS=-fvisibility=default but I'm not sure if this is the best/simplest approach then
07:53 < real_or_random> hebasto: do you expect other issues where we have some strange exported functions on some special platforms, e.g., only on macos?
07:54 < real_or_random> https://gcc.gnu.org/wiki/Visibility argues that reducing the number of exported symbol is great for performance but if this is the only reason, it's not a big deal if there's a handful of symbols that the a strange compiler introduces for example  
07:54 < fanquake> iirc visibility is actually hidden by default in the macOS linker
07:54 < sipa> There is another potential issue, namespace clashes.
07:55 < hebasto> ^ +1
07:55 < sipa> If we were to have an internal function that doesn't have a nice secp256k1_ prefix, and isn't static, and somehow gets exported, there is a potential chance that a user application linking against it will have a symbol with the same name.
07:55 < sipa> But that's a lot of ifs.
07:55 < sipa> And it'll fail very cleanly.
07:57 < real_or_random> okay... here's a suggestion: pragmatically, I like the script because it's an independent and "end-to-end" test. it does not rely on the compiler etc and simply inspects the binary. I'm happy to add it, and add a CI test that builds with ./configure CFLAGS=default. The only concern I have is that the script will be difficult to maintaian with changes in LIEF
07:59 < real_or_random> but given that it exists and works right now (ok, after the LIEF release), I don't mind giving it a try
07:59 < sipa> Just for x86_64 and linux?
07:59 < sipa> What do you suggest we do about the secp256k1_pre_g_128 etc?
08:00 < fanquake> If the concern is maintenance, then nm/objdump + grep can acheive the same thing, as far as I can tell.
08:00 < real_or_random> for all platforms. If we want just x86_64 and linux, that's also a reasonable choice, but then objdump and grep will be nicer
08:00 < real_or_random> (I think the reason why hebasto used LIEF is that macos doesn't have objdump?)
08:00 < fanquake> why does nm/objdump + grep not work for all platforms?
08:00 < hebasto> the script can check dll now
08:01 < real_or_random> sipa: for secp256k1_pre_g_128... i don't know right because these are not functions, but I'm sure there's a way to fix it?
08:03 < fanquake> The objdump on macOS will be either objdump, or llvm-objdump. Would need to look at the CI to see what is already installed there.
08:03 < sipa> we can explicitly mark them as internal I guess
08:04 < sipa> removing the need to rely on -fvisibility=hidden
08:04 < sipa> s/internal/hidden/
08:04 < real_or_random> hebasto: or what was the reason why you took LIEF?
08:05 < real_or_random> sipa: we can also add a `#pragma GCC visibility` for the entire codebase but I guess this has other tradeoffs 
08:05 < sipa> but I'm not sure we care about that
08:05 < hebasto> real_or_random: Bitcoin Core has a similar script based on LIEF
08:07 < real_or_random> ok yes... do you think it's doable with objdump? the thing is: if the objdump has a different format on every platform, then lief + python may actually be easier 
08:07 < sipa> Does nm not work?
08:07 < fanquake>  I migrated our scripts to LIEF because we have a large number of additional symbol / security related checks, which were much harder, if not impossible to acheive using objdump & grep etc, and problems with intricate tool output changing across versions etc.
08:08 < hebasto> yes, it can be implemented in bash
08:08 < real_or_random> and I don't think a dependency on python in CI is  bad. we anyway depend on it already. and the user is not supposed to run it, it's really just ci and the devs
08:09 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has quit [Quit: My iMac has gone to sleep. ZZZzzz…]
08:10 < sipa> Another advantage is that it works for all libraries, not just matcing format ones.
08:11 < real_or_random> well ok, we're already over the hour. I don't know. I feel if LIEF i) currently works and ii) core uses it too, it's a good choice? (nm/objdump may be a tiny bit better but I don't care then?)
08:11 < sipa> Like objdump will only work for x86_64 elf linux binaries/libraries on x86_64 linux.
08:12 < real_or_random> hebasto: would you be willing to add a commit that call the script in CI, and maybe add builds with visibility=default?
08:12 < hebasto> real_or_random: ok
08:12 < real_or_random> at least for me, that looks like a reasonable way forward 
08:12 < sipa> And make it work without unreleased LIEF on macos, as fanquake suggestedm
08:12 < sipa> ?
08:13 < hebasto> sipa: ok
08:13 < real_or_random> yes, if the suggestion works, that will be neat 
08:14 < real_or_random> ok!
08:15 < real_or_random> and I'll open an issue about the visible symbols we need to fix (.s and a few others) 
08:15 < real_or_random> end of meeting then? 
08:16 < sipa> sg
08:17 < real_or_random> ok, end of meeting
08:28 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has joined #secp256k1
08:38 -!- jonatack2 [~jonatack@user/jonatack] has joined #secp256k1
08:46 < nickler> Sorry! I was stuck in various phone calls. RE focus: I'd focus on what users are requesting and ellswift seems to be a part of that.
08:46 < nickler> The release note process matches what we discussed before I think. Having a tag is a great idea.
08:51 -!- jonatack2 [~jonatack@user/jonatack] has quit [Ping timeout: 252 seconds]
08:51 -!- jonatack3 [~jonatack@user/jonatack] has joined #secp256k1
09:32 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has quit [Quit: My iMac has gone to sleep. ZZZzzz…]
09:36 < real_or_random> ellswift sounds good to me and aligns with the previous decision to look at the jacobian PRs first (among the "big" PRs),  so yes, a neat goal for a next release
10:02 -!- jonatack [~jonatack@user/jonatack] has joined #secp256k1
10:03 -!- jonatack3 [~jonatack@user/jonatack] has quit [Ping timeout: 246 seconds]
10:19 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has joined #secp256k1
10:47 -!- jon_atack [~jonatack@user/jonatack] has joined #secp256k1
10:49 -!- jonatack [~jonatack@user/jonatack] has quit [Ping timeout: 268 seconds]
14:00 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has quit [Quit: My iMac has gone to sleep. ZZZzzz…]
15:26 -!- halosghost [~halosghos@user/halosghost] has quit [Quit: WeeChat 3.7.1]
22:46 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has joined #secp256k1
22:49 -!- p2plife [~p2plife@vps-46773dd2.vps.ovh.net] has joined #secp256k1
22:57 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has quit [Quit: My iMac has gone to sleep. ZZZzzz…]
23:23 -!- tromp [~textual@92-110-219-57.cable.dynamic.v4.ziggo.nl] has joined #secp256k1
--- Log closed Tue Dec 20 00:00:50 2022