< GitHub166>
[bitcoin] arnuschky opened pull request #6742: Changed logging to make -logtimestamps to work also for -printtoconsole (master...feature-logtimestamps-toconsole) https://github.com/bitcoin/bitcoin/pull/6742
< rusty>
sipa: libsecp questions. How do I check that the signature is in canonical form (small s value)? You've made signature struct "opaque".
< rusty>
Does verify do this?
< sipa>
rusty: no, there is no functionality for doing that
< sipa>
we should add it, though - i'm thinking about changing the signature parsing code anyway
< rusty>
sipa: yeah... that's what I figured. I hand around sigs in lightning as 64 byte raw vals, and I'm having to hack it.
< sipa>
how do you convert them from/to 64 byte format?
< rusty>
struct signature { u8 r[32];u8 s[32]; };
< rusty>
sipa: I stole the DER encode/decode from bitcoin.
< sipa>
got it
< sipa>
well, it would not be hard to add another parse/serialize function
< sipa>
that converts to a well-defined 64-byte format
< rusty>
sipa: yes, your comments even refer to it :)
< sipa>
DER is a needless complication, both for client and library
< sipa>
well there is such a format for the recoverable signatures
< rusty>
"use the secp256k1_ecdsa_signature_serialize_* and secp256k1_ecdsa_signature_serialize_* functions." :)
< sipa>
rusty: someone sent a PR today to fix that
< gmaxwell>
we already produce canonical form in all cases, it's just there is no way to verify it.
< gmaxwell>
If _parse too verification flags, it could be asked to check for that trivally.
< rusty>
sipa: secp256k1_ecdsa_signature_serialize()/_parse() then? I'll have to dig into your slightly weird scalar system to implement it.
< gmaxwell>
though perhaps it would better be done with a seperate sig_has_low_s().
< sipa>
rusty: give me 5 mins, i'll implement it
< rusty>
gmaxwell: yes, I have a couple of "assert(sig_valid(s))" scattered through my code.
< sipa>
i don't like adding dozens of flags
< rusty>
gmaxwell: (which is "return true;" for schnorr, and tests for top s bit for ecdh)
< sipa>
in fact, i prefer none...
< rusty>
gmaxwell: so in practice, a standalone check would fit me better. I guess that's a data point?
< sipa>
rusty: good. decided.
< gmaxwell>
I sort of dread doing the testing if there are flags. :( Also I think parsing for lows but sloppy DER doesn't make logical sense.
< sipa>
i think i have a nearly-complete BER parser
< sipa>
it's only twice as long as the current parse code
< gmaxwell>
sipa: I am ... really not looking forward to writing tests for that. :( Also, how can it be BER, I assume it must length limit the outputs?
< sipa>
gmaxwell: yes, up to a fixed limit
< gmaxwell>
Also, there exist no other nearly complete BER parsers, so I dunno how I can do a differential test against it.
< rusty>
sipa: I really want to be able to insist it's normalized. Actually, I want that for everything. If someone slips something in which causes me to build an invalid tx, I'm hosed.
< sipa>
my design goal is: accept everything that is valid BER or accepted by openssl
< sipa>
rusty: oh, absolutely... i don't want to encourage BER as default
< sipa>
rusty: there needs to be a strict DER parser one (which is much easier)
< sipa>
but for use in Bitcoin we can't just only have a strict DER one
< sipa>
michagogo: it should definitely recompute things (either on the fly, or at startup and update)
< sipa>
that's how the BIP34/66 implementation works as well
< CodeShark>
michagogo: I've been considering two approaches...either save state or recompute. I think it's inevitable we need to save some state at runtime (i.e. as part of the block index)...but we can recompute at startup