search

ethereum / eth-keys

A common API for Ethereum key operations.
MIT License
159 stars 64 forks source link

Add local asn1 encoding tools #60

Closed carver closed 5 years ago

carver commented 5 years ago

What was wrong?

In order to use coincurve for creating/verifying non-recoverable signatures, we want (or need?) ASN1-encoded signatures. See #58

How was it fixed?

ASN1 in general is a pretty expansive standard. It takes pretty significant libraries to support it in general. We would prefer not to add external dependencies to this very sensitive library. Luckily, we are only using one small corner of ASN1 (a sequence of two integers that are guaranteed positive and can be contained in 32 bytes). So it's relatively straightforward to implement a custom encoded/decoder for this case, which saves us a library.

Out of an abundance of caution, I added hypothesis tests against two different asn1 libraries. (encoding to/from the cross-product of all three implementations)

Cute Animal Picture

Cute animal picture

pipermerriam commented 5 years ago

@carver did you do any initial stress testing like running the tests in a loop for an hour and boosting the total examples up a but for that time?

carver commented 5 years ago

@carver did you do any initial stress testing like running the tests in a loop for an hour and boosting the total examples up a but for that time?

Yup, I ran it for a bit! I only ran it for about a half an hour though, and some of them were testing pyasn1 vs asn1tools. I'll focus on just the ones that test local against reference (that change should actually get merged). I'll do a local run that should take about... 2 hours, and then merge.