Add local asn1 encoding tools #60
Conversation
|
@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? |
eth_keys/utils/asn1.py
Outdated
| # We locally implement serialization and deserialization for this specific spec | ||
| # with constrained inputs. | ||
| # This is done locally to avoid importing a 3rd-party library, in this very sensitive project. | ||
| # asn1tools and pyasn1 were used as reference APIs, see how in tests/core/test_utils_asn1.py |
There was a problem hiding this comment.
Maybe add a comment here that this module is for internal use only as it doesn't fully validate inputs (which is fine for us as we only use it to bridge to coincurve, but if someone uses it with unvalidated external inputs they might end up accepting invalid signatures).
eth_keys/utils/asn1.py
Outdated
| @@ -0,0 +1,112 @@ | |||
| # Non-recoverable signatures are encoded using an asn1 sequence of two integers | |||
There was a problem hiding this comment.
Nitpick regarding the module name: ASN1 seems to be just the language to describe the schema, the encoding is called DER.
There was a problem hiding this comment.
Ah, thanks for pointing that out. Will rename.
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. |
Exclude huge and unnecessary venv*/ from dist
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