--- Log opened Wed Apr 17 00:00:47 2019
05:46 -!- lukedashjr [~luke-jr@unaffiliated/luke-jr] has joined #secp256k1
05:48 -!- luke-jr [~luke-jr@unaffiliated/luke-jr] has quit [Ping timeout: 268 seconds]
05:50 -!- lukedashjr is now known as luke-jr
10:03 -!- dgpv [58631edb@gateway/web/freenode/ip.88.99.30.219] has joined #secp256k1
10:05 < dgpv> Hi!
10:05 < dgpv> A there working link to the logs for the channel ?
10:05 < dgpv> the one listed in the heading does not work
10:06 < dgpv> jonasnick pointed me to this channel and said that I should look into the discussion held here regarding my PR https://github.com/ElementsProject/secp256k1-zkp/pull/37
10:07 < dgpv> I would like to read that discussion before making any comments
10:13 < nickler> hm, looks like the botbot link in the topic doesn't work anymore
10:13 < dgpv> ok
10:17 < sipa> yeah, shut down due to GDPR...
10:18 < nickler> http://gnusha.org/secp256k1/2018-12-25.log works but no https
10:25 < dgpv> I described the contexts where headers are not available in the PR (basically everywhere c compiler is not involved.. FFI from the higher-level langs is the most obvious)
10:26 < dgpv> So as I understand, the issue with having both sizeof(secp256k1_surjectionproof) to use for stack allocation, and function/symbol to get the size for heap allocation is the fact that there would be two ways to get the same value, and this could be confusing/inconsistent.
10:27 < dgpv> (function/symbol for heap allocation when used through FFI without access to headers)
10:28 < dgpv> Is my understanding correct ?
10:35 < andytoshi> basically yes, and sizeof() is always available if you have the header
10:36 < andytoshi> nickler pointed out to me that there may be extra duplication for some FFI schemes where you're forced to copy the whole type into the FFI definitions to get sizeof(), vs just using an opaque type
10:36 < dgpv> maybe I am not understanding the scope of the library correctly. This only makes sense if you are including the library in your project
10:36 < andytoshi> ?
10:37 < andytoshi> secp256k1.h contains complete type definitions for all the non-opaque types
10:37 < andytoshi> and creation/destruction functions for the opaque ones (of which there is only one, secp256k1_context)
10:37 < dgpv> if sizeof(secp256k1_surjectionproof) changes in the future version of the lib
10:37 < dgpv> and you install the lib (via distro upgrade procedure for example)
10:37 < dgpv> you get mismatched sizes
10:38 < dgpv> if you do not recompile your code
10:38 < andytoshi> oh i see
10:38 < andytoshi> using an exported symbol means you don't need to recompile
10:38 < dgpv> yes
10:39 < dgpv> copying wole struct into ffi definition is also fragile - you will need to track the struct layout, and if the struct layout depends on some compile-time options, you may be in trouble
10:40 < andytoshi> ok, i found my logs for the last discussion about this
10:40 < andytoshi> which basically concluded "we should export create/destroy functions, not a size"
10:40 < dgpv> (you will need to track the changes in the struct layout in upstream software)
10:40 < dgpv> ok. makes sense.
10:41 < andytoshi> but also some discussion about apparently needing macros and/or sizeof to export the size (which may be true, given C's shitty rules about what a `const` variable can be)
10:41 < andytoshi> https://0bin.net/paste/YTHHAOQYxFkG6UyG#-+I67qw39H7cf0iEa0QZvHekyPMX19jB8VExNOLPSet
10:42 < andytoshi> hm, no, `const int exportme = sizeof(int)` appears to be valid C89 .. but anyway it'd be better to have create/destroy functions for people who don't want to use the complete type
10:46 < dgpv> I think you could have both the symbol SECP256K1_SURJECTIONPROOF_RAW_SIZE _and_ the macro of the same name
10:46 < dgpv> but that would mean there should be some tinkering with linker to rename the symbol
10:47 < dgpv> you include C header - you get the macro. You load dynamic library - you get the symbol
10:48 < dgpv> maybe that would be too hacky, though
10:48 < gmaxwell> Change in the size of a exposed type is an ABI change, inherently it needs recompilation.
10:48 < andytoshi> yeah, it'd be better to just wrap the allocation in its own function
10:48 < andytoshi> gmaxwell: even if that type is not actually used (except by pointer) by a given user?
10:50 < andytoshi> i guess, if we had versioning of the library it would require us to release a new major version because it would definitely break some possible users
10:55 < gmaxwell> andytoshi: it might always be the case that some ABI change might manage to not effect some caller, that doesn't make it not part of the ABI.
10:56 -!- elichai2 [uid212594@gateway/web/irccloud.com/x-zuqkvybchlaclmvs] has joined #secp256k1
10:57 < andytoshi> ok, sure. i guess then i'm arguing for a privileged class of caller (people who treat all types as opaque) and suggest we take measures to ensure they don't need to recompile for ABI changes that are limited to changing the exposed types
10:58 < dgpv> let's say secp256k1-zkp is included in debian as a package. user builds against secp256k1-zkp-dev headers, everything works. Then he does apt update. New version of the lib have different size of the struct. User gets crashes and does not know what happened.
10:58 < andytoshi> that should never happen without a major version change
10:58 < gmaxwell> if people use those variables they lose the advantages of the sizes being known at compile time in any case.
10:59 < andytoshi> gmaxwell: right. so it sucks that they still have the _disadvantages_ (more changes being breaking changes) of those size being known at compile time
10:59 < gmaxwell> dgpv: no that won't happen, if the sizes change it would be a new major version, and the linker would continue to provide the old version to old callers.
10:59 < dgpv> ok
11:02 < dgpv> so FFI code can actually assume the minimal needed size is the value that `printf("%d\n", sizeof( secp256k1_surjectionproof))` would give on 64-bit platform
11:03 < dgpv> (if you don't count the possibility that the lib could be built with VERIFY enabled, then int woule be bigger by sizeof(int)
11:05 < andytoshi> i think that's almost certainly a bad assumption
11:05 < andytoshi> which will be nearly impossible to test :)
11:05 < andytoshi> i do think we should expose alloc/dealloc functionality for a bunch of our types, to make it easier for FFI users who don't want to screw around with the exposed types (which are indeed tedious and error-prone to keep in sync)
11:10 < dgpv> andytoshi: I agree that this is bad assumption. but for perfectly aligned struct it would work in practice. most of the time :-)
11:13 < gmaxwell> andytoshi: I have never seen a C library do that.
11:14 < sipa> gmp
11:14 < dgpv> double the size and it will always work in practice. Still better than depending on C compiler to build small .so just to get this value
11:14 < sipa> i think
11:15 < sipa> i'm wrong.
11:16 < sipa> an alternative to forcing external code to use FFI to communicate and keep APIs in sync, it's also possible to include wrappers for those languages upstream (as we did for java)
11:16 < gmaxwell> andytoshi: it's essentially universal that there are typedefs in libraries, surely every FFI interface can handle them.
11:17 < sipa> though only for major languages, in cases where the wrapper code is reviewable and testable in CI...
11:18 < gmaxwell> the java wrapper we have is seems fairly sketchy even though we nomally have a couple contributors around who are java experts I guess they just aren't intereted in it.
11:18 < sipa> dgpv: what language are you invoking things from?
11:19 < sipa> gmaxwell: yeah
11:20 < dgpv> python
11:23 < dgpv> a port of Elements' BlindTransaction() so I have to call secp256k1_surjectionproof_initialize and pass a buffer of appropriate size
11:25 < gmaxwell> ffi.new("secp256k1_surjectionproof") ...
11:25 < dgpv> I use ctypes
11:28 < gmaxwell> apparently ctypes just stinks in this respect, https://stackoverflow.com/questions/13403587/referencing-opaque-types-with-python-ctypes
11:31 < dgpv> but is lightweight - I wanted to avoid extra dependencies
11:31 < sipa> gmaxwell: how would it work better with ffi?
11:32 < gmaxwell> sipa: in FFI ffi.new("secp256k1_surjectionproof") allocates a secp256k1_surjectionproof... or you can sizeof it. You have access to the headers.
11:33 < sipa> ah i see
11:33 < sipa> it parses the header file at runtime?
11:33 < gmaxwell> sipa: yes.
11:34 < dgpv> it could even invoke c compiler, if I'm reading the docs right
11:34 < dgpv> so-called 'API mode'
11:36 < dgpv> otherwise, "The C source is parsed internally (using pycparser)."
11:37 < sipa> that sounds pretty horrible
11:37 < dgpv> also still a big dependency. Other than this issue, handling everything re secp256k1-zkp with ctypes was easy
11:37 < sipa> certainly easier to make interact, but at a pretty crazy cost
11:38 < sipa> it'd be much easier to make things work if there were create/destroy functions for all data structures
11:38 < sipa> which allocate on the heap
11:39 < andytoshi> this is also true (though things are much nicer) in rust ... you can use an automated parser to translate the header into Rust, or you can do it manually
11:40 < andytoshi> the automated one is a bit scary and requires handholding; the manual process is made a bit easier (for some people) if we add alloc/dealloc functions
11:40 < andytoshi> but with rust, i think we do ultimately want to use the exposed types because we have no runtime forcing us to use allocation
11:40 < andytoshi> ...so we shouldn't.
11:41 < sipa> yeah,for rust it makes sense to aim for stack-allocatability
11:41 < sipa> for python i doubt it matters :
11:41 < sipa> :p
11:44 < gmaxwell> I don't see how ctypes could be used with libsecp256k1 as is, even ignoring the secp256k1_surjectionproof stuff...
11:44 < gmaxwell> except by recreating the typedefs in it, which is unsafe.
11:45 < gmaxwell> Ctypes could be used by a wrapper that was shipped with the library and kept lockstep with it, which I am guessing is the main audience for ctypes. (in particular python modules with c-parts)
11:46 < sipa> gmaxwell: right; that wrapper would essentially be exposing create/destroy functions that allocate on the C heap?
11:46 < sipa> or more?
11:47 < gmaxwell> sipa: at least that... it's kind of questionable though to provide a raw interface to a c-function in python though.
11:47 < gmaxwell> normally python code is unconditionally safe to call, you're not going to crash the interperter by doing it wrong.
11:47 < sipa> there'd be a python side to the wrapper as well, i imagine
11:47 < sipa> which gives a pythonic interface to those functions
11:48 < dgpv> you going to crash a c program by doing it wrong, so this just means if you program using ctypes you have to be as careful as in C
11:48 < sipa> rather than exposing the unsafe C function directly
11:48 < gmaxwell> sipa: right, thats what I meant.
11:49 < gmaxwell> dgpv: It isn't really acceptable to expect j-random-python user to have any idea how to use a C interface safely.
11:52 < sipa> i wonder how https://github.com/ofek/coincurve works...
11:52 < sipa> ah, cffi
11:53 < gmaxwell> sipa: we could provide an 'alternative header' for ctypes user, without providing a whole python interface. Essentially it just escapes the needing to parse a c header problem.
11:54 < sipa> right
11:54 < gmaxwell> I'm a lot more confident in our ability to maintain a shadow copy of the typedefs than a full on interface.
11:55 < gmaxwell> though I don't think there is really a replacement for a domain expert that knows both the calling language and C in making a wrapper.
11:55 < dgpv> for ctypes what is essentially needed is the upper bound
11:55 < dgpv> "the struct won't be bigger than that"
11:56 < gmaxwell> well then use a megabyte and call it done.
11:56 < sipa> haha
11:56 < dgpv> yes that is an option, terrible, though
11:56 < gmaxwell> why is it terrible?
11:56 < dgpv> too much
11:57 < gmaxwell> I mean the whole concept of using an 'upper bound' would not be safe... and could still crash.
11:57 < dgpv> not if it is documented and enforced with #error sizeof(struct) <= bound
11:59 < gmaxwell> that is unportable, highly non-ideomatic, and probably language wonk wise invalid C.
12:00 < sipa> i prefer having a ctype-friendly separate API over that
12:00 < dgpv> that is not correct code, just the ide
12:00 < dgpv> idea
12:00 < sipa> sure
12:00 < sipa> but stil
12:01 < sipa> +l
12:01 -!- midnightmagic [~midnightm@unaffiliated/midnightmagic] has quit [Ping timeout: 264 seconds]
12:03 < dgpv> in practice that would be a-la static_assert macro
12:04 < dgpv> but I see that it might somehow 'taint' the code with such tricks
12:06 < gmaxwell> Crapping up the library with unsafe hacks or alternative interfaces is just a really poor substitute for simply shipping a ctypes version of the header (or an actual python interface)
12:07 < dgpv> that is not python-only I believe. parsing C headers for FFI is not universal, heavy, and error-prone
12:08 < dgpv> gmaxwell: what do you mean by 'ctypes version of the header' ? like, python file with constants ?
12:09 < dgpv> and btw, why that would be worse than just exporting the symbol from .so ?
12:10 < sipa> dgpv: a version with only opaque types and create/destroy functions
12:11 < dgpv> but if create/destroy funcs are added, why separate into special header ?
12:12 < dgpv> for ctypes the header does not matter anyway, what matters is the symbols in dynamic lib, calling convention and func prototypes
12:12 < sipa> fair
12:12 < gmaxwell> that isn't what I was suggesting
12:13 < gmaxwell> I'm suggesting shipping a little python file that defines the types using ctypes that gets updated in lockstep with the library, just like would be done if a full python interface using ctypes were shipped with it.
12:13 < gmaxwell> which appears to be the most common way ctypes gets used.
12:14 < sipa> ah
12:14 < dgpv> so the distro package maintainer would add the hook to install that .py file into python path ?
12:15 < gmaxwell> Right, or something equivilent.
12:15 < gmaxwell> sipa: I don't think changing the C interface entirely (to one that C callers won't use or won't use safely) is a good exchange.
12:15 < dgpv> still, would work only for python
12:16 < gmaxwell> dgpv: You cannot call C code in general without access to the headers. It is weird that ctypes doesn't provide access to them, though I understand why it doesn't.
12:17 < sipa> gmaxwell: is this CI testable? (i guess by writing a python program and c program that both compute the sizeof, and comparing them?)
12:18 < gmaxwell> sipa: By doing that... yes, however, it's still fragile but this is apparently a norm in python land-- at least from googling around. (I find it kind of horrifying)
12:19 < gmaxwell> apparently ctypes is believed to handle padding consistently with the local compiler, I have no idea how they could possibly guarentee it, but I'm not turning up any examples of anyone finding it failing.
12:19 < gmaxwell> And it's what a LOT of python modules appear to do internally for their own C code.
12:21 < sipa> i wasn't aware of that approach
12:22 < sipa> as long as that padding computation works, it seems like a very reasonable approacb
12:22 < dgpv> gmaxwell: for dynamic libraries it is actually quite normal to be able to use them without c headers, in my experience. There is usually some method to discover the sizes needed
12:23 < gmaxwell> dgpv: the fact that you're only having this problem for secp256k1_surjectionproof sounds like something awful is already going on in your code.
12:23 < gmaxwell> since the pubkey and signature types are also semi-opqaue structs.
12:23 < dgpv> the sizes are explicitly documented
12:24 < dgpv> like, 'however guaranteed to be 64 bytes in size, and can be safely copied/moved'
12:26 < sipa> right, all of our basic datastructures have guaranteed sizes
12:26 < sipa> except scatch space, but that one is already opaque
12:27 < sipa> though it seems that's not tenable for more complex APIs
12:29 < dgpv> one approach that is used to discover the size is that if you don't know the size, you can call the func twise, first specifying the size zero, and it will return the needed size
12:29 < dgpv> not that it is very elegant, but it works and used often
12:31 < dgpv> that also means one extra parameter for the size, so I think that won't fly :-)
12:53 < dgpv> It turns out you can have both macro with sizeof() and the symbol
12:53 < dgpv> because you can undef the macro before declaring the const int variable with the same name
12:54 < dgpv> #define SECP256K1_SURJECTIONPROOF_RAW_SIZE sizeof(secp256k1_surjectionproof) in the header
12:54 < dgpv> #undef SECP256K1_SURJECTIONPROOF_RAW_SIZE
12:54 < dgpv> SECP256K1_API const int SECP256K1_SURJECTIONPROOF_RAW_SIZE = sizeof(secp256k1_surjectionproof);
12:54 < dgpv> in src/secp256k1.c
12:55 < dgpv> now the same name can be used to allocate on stack in C and on heap when used via FFI
13:17 < dgpv> I updated the PR with this.
13:19 < dgpv> IMO this is the simpliest and least fragile approach
13:44 < dgpv> don't see how this might break in practice.
13:49 < andytoshi> i don't think we want to screw around with macros like this, it's hard to reason about
13:50 < andytoshi> i also don't want to expose the size like this, because that has limited applicability (it only exposes the size, not other invariants that we might want to enforce on these types later on)
13:50 < andytoshi> it would be better to have an alterative to the _init functions that also does the allocation
13:51 -!- dgpv [58631edb@gateway/web/freenode/ip.88.99.30.219] has quit [Ping timeout: 256 seconds]
13:53 < gmaxwell> the undefing and the name aliasing are both violations of MISRA C that I wouldn't feel comfortable writing a justification for.
13:53 < gmaxwell> An allocator/deallocator seems fine for those. It's not the end of the world if they can't be on the stack.
14:35 -!- elichai2 [uid212594@gateway/web/irccloud.com/x-zuqkvybchlaclmvs] has quit [Quit: Connection closed for inactivity]
14:37 -!- dgpv [58631edb@gateway/web/freenode/ip.88.99.30.219] has joined #secp256k1
14:37 < dgpv> gmaxwell: like for scratch space ?
14:40 < gmaxwell> the most common way of handling opaque objects in C is just having the calee allocate them and you just get an opaque pointer.  Sometimes that isn't done because the performance overheads of the additional allocations which could be avoided by constucting the objects on the stack.
14:42 < gmaxwell> from the perspective of a python user its all irrelevant, you don't have a stack and everything is going to be really slow no matter what. :)
14:47 < dgpv> I mean If I make PR that adds allocator/deallocator func specifically for secp256k1_surjectionproof you would be OK with that ?
14:48 < dgpv> despite that now there would be 2 ways to allocate - via sizeof and via allocator
14:49 < andytoshi> the "2 ways" are by asking the library to do it for you, or doing it yourself
14:49 < andytoshi> yes, that's fine
14:49 < andytoshi> i'll accept such a PR for secp-zkp
14:52 < gmaxwell> You wouldn't use sizeof, we's just have a function that gets the size.  And make the struct totally opaque (just a pointer) so the caller can't even see the size.
14:53 < gmaxwell> Which would allow changing the size in the future without breaking the ABI.
14:53 < gmaxwell> So at least that advantage gets gained.
14:53 < gmaxwell> The only loss is that callers lose the ability to construct it on the stack (well, without using var arrs).
14:56 < sipa> i suspect that andytoshi would like to keep the ability to construct on the stack in rust
14:56 < andytoshi> i would like to retain that ability
15:00 < gmaxwell> bleh okay.
15:16 < dgpv> andytoshi: so there are 2 ways to allocate either way - via alloc func and sizeof, or via symbol and sizeof. The only difference is that in the first case you can "other invariants that we might want to enforce on these types later on".
15:16 < dgpv> andytoshi: what invariants ? alignment ?
15:21 < dgpv> andytoshi: would you be able to check these invariants in rust other than manually ?
15:21 < andytoshi> no, i mean things like magic bytes that will detect invalid transmutation of types
15:22 < andytoshi> yes, i would need to ensure in unsafe rust code that the invariants were upheld
15:22 < andytoshi> but that would be hidden behind a safe API
15:26 < dgpv> if your rust code so intimately linked with C code, does it matter if the structure is in public header or in private header ?
15:27 < andytoshi> wtf
15:27 < andytoshi> sorry
15:27 < dgpv> as I understand you are re-creating thw structure in rust anyway
15:27 < andytoshi> the rust code is not "intimately linked" with the C code
15:27 < andytoshi> the rust code obeys the API contract
15:27 < dgpv> magic bytes would be part of the API ?
15:28 < andytoshi> which includes upholding any invariants (e.g. "don't use structures you didn't initialize or get from the alloc function")
15:28 < andytoshi> the magic bytes will be correct as long as users correctly use the API
15:29 < sipa> andytoshi: how does the rust code know how to call C?
15:29 < andytoshi> sipa: it has a rust file with declarations of all the C functions (in rust syntax)
15:29 < andytoshi> the rest is up to the linker
15:29 < sipa> right, so it needs a mirroring of the .h file, which needs to be kept up to date?
15:29 < andytoshi> yep
15:29 < dgpv> who will set these magic bytes ? the _initialize func ?
15:30 < andytoshi> dgpv: yes
15:30 < andytoshi> dgpv: but i think the allocate() function might as well also call _initialize before returning
15:30 < sipa> of course
15:30 < dgpv> so the invariants will not be enforced at allocation time
15:31 < dgpv> so it does not matter if only the size is exported (it is re your " don't want to expose the size like this" message earlier)
15:31 < andytoshi> dgpv: if you're manually allocating then you are required (by the API, not by the compiler) to call _initialize
15:31 < andytoshi> sigh
15:31 < dgpv> yes, but yor only need to know the size, thats all
15:32 < andytoshi> i considered not answering your "what invariant?" question earlier because i thought you'd reply "oh well for that specific invariant you can do what i'm suggesting"
15:32 < andytoshi> i want a sane API which can handle as wide a set of changes as possible without change
15:32 < andytoshi> which means that for users who don't want to manage their own memory, exposing functions that create objects for them, not functions that expose a pile of specific aspects of them
15:33 < dgpv> i am trying to push any particular way. as long as there's the way thats fine
15:33 < sipa> i have no idea what you guys are disagreeing about
15:34 < dgpv> I was actually interested what the invariants would be that would be broken by just exposing the size
15:34 < andytoshi> ah, k, sorry
15:34 < dgpv> but this is only for my curiosity
15:35 < dgpv> * i am <not> trying to push any particular way
15:35 < andytoshi> any such invariants would be enforceable by requiring users to call _initialize after allocating
15:35 < dgpv> I though that is already a requirement
15:35 < andytoshi> but that should only matter to users who are trying to stack-allocate (and who are therefore using the exposed types and need to be careful)
15:35 < andytoshi> yes
15:36 < andytoshi> but if you want an API that does not require you to mirror the types
15:36 < andytoshi> then that API should not require you do all the other manual work
15:36 < dgpv> it makes sense that the allocator would initialize
16:19 -!- dgpv [58631edb@gateway/web/freenode/ip.88.99.30.219] has quit [Ping timeout: 256 seconds]
17:57 -!- gmaxwell_ [gmaxwell@mf4-xiph.osuosl.org] has joined #secp256k1
18:00 -!- jonasschnelli_ [~jonasschn@2a01:4f8:172:10da::2] has joined #secp256k1
18:02 -!- Netsplit *.net <-> *.split quits: jonasschnelli, gmaxwell
19:01 -!- gmaxwell_ [gmaxwell@mf4-xiph.osuosl.org] has quit [Changing host]
19:01 -!- gmaxwell_ [gmaxwell@wikimedia/KatWalsh/x-0001] has joined #secp256k1
19:01 -!- gmaxwell_ is now known as gmaxwell
21:40 -!- belcher [~belcher@unaffiliated/belcher] has quit [Ping timeout: 245 seconds]
23:04 -!- jonasschnelli_ [~jonasschn@2a01:4f8:172:10da::2] has quit [Changing host]
23:04 -!- jonasschnelli_ [~jonasschn@unaffiliated/jonasschnelli] has joined #secp256k1
--- Log closed Thu Apr 18 00:00:48 2019