I did the best part of an implementation of Crypto-Conditions in Haskell (link) (I work for BigchainDB), and when I came to test my code against the provided test suite (very useful btw), there were a few things that were not as I expected, so I will outline them here as feedback:
ASN
Throughout the spec, the ASN encoding is expressed in terms of ASN field types, however, the encoded binary does not contain the type headers, instead the field headers are "implicit" and manually numbered.
I am using the hs-asn1 libraries which have a data type to describe ASN1 encodable data, rather than using models, but the calls to implicit(0) are apparent in the Javascript code.
Below is an example of how that looks, encoding a payload that contains "hello", then a Sequence of an integer and then "world".
[ Other Context 0 "hello"
, Start (Container Context 1)
, Other Context 0 "\SOH"
, Other Context 1 "world"
, End (Container Context 1)
]
"\128\ENQhello\161\n\128\SOH\SOH\129\ENQworld"
So you can see that the field headers are different but the data is the same. Although the spec specifies SEQUENCE OF, usually I had to enter Start (Container Context n), where n is the index of the sequence in the parent data structure.
The exception to this is in the fingerprints, where the payload that gets hashed is in fact wrapped in a Sequence, at least according to my code. Looking at the Javascript code, this.seq() is always called, it could be that a call to implicit(n) is what ends up setting the header of the container, I don't know.
I'm not sure if the use of numbered "implicit" (I suppose that means the type is implicit) fields is better or worse than the other way, but, it wasn't obvious or explained that way in the spec, as far as I saw.
Test Suite
Overall really good, very reassuring to have actual encoded values to test against :+1:.
I made a pr with one small issue I found. I would also like to have seen a message or a prefix payload in some places where they were empty, just to be thorough. I didn't implement all the tests yet, and I see there have been some changes to the latest HEAD so I can't say for sure that's the only one, but still, the vast marjority of it worked, really good!
The Spec
I didn't read it in its entirity from start to finish, so I may have missed details, but I did refer to it extensively.
At a high level, it feels like it's been re-written a bit, half old half new, I tend to see this effect when documentation has patches applied to it, it no longer feels natural, I could be off the mark though.
There are some sections like the following:
A PREFIX-SHA-256 fulfillment is valid iff:
* The size of M, in bytes, is less than or equal to F.maxMessageLength AND
* F.subfulfillment is valid, where the message used for validation of f is M prefixed by F.prefix AND
* D is equal to C
Which left me wondering what D and C were, and I was just now still unable to locate "D".
I did the best part of an implementation of Crypto-Conditions in Haskell (link) (I work for BigchainDB), and when I came to test my code against the provided test suite (very useful btw), there were a few things that were not as I expected, so I will outline them here as feedback:
Throughout the spec, the ASN encoding is expressed in terms of ASN field types, however, the encoded binary does not contain the type headers, instead the field headers are "implicit" and manually numbered.
I am using the hs-asn1 libraries which have a data type to describe ASN1 encodable data, rather than using models, but the calls to
implicit(0)are apparent in the Javascript code.Below is an example of how that looks, encoding a payload that contains "hello", then a Sequence of an integer and then "world".
I expected:
But instead it's:
So you can see that the field headers are different but the data is the same. Although the spec specifies
SEQUENCE OF, usually I had to enterStart (Container Context n), where n is the index of the sequence in the parent data structure.The exception to this is in the fingerprints, where the payload that gets hashed is in fact wrapped in a
Sequence, at least according to my code. Looking at the Javascript code,this.seq()is always called, it could be that a call toimplicit(n)is what ends up setting the header of the container, I don't know.I'm not sure if the use of numbered "implicit" (I suppose that means the type is implicit) fields is better or worse than the other way, but, it wasn't obvious or explained that way in the spec, as far as I saw.
Overall really good, very reassuring to have actual encoded values to test against :+1:.
I made a pr with one small issue I found. I would also like to have seen a message or a prefix payload in some places where they were empty, just to be thorough. I didn't implement all the tests yet, and I see there have been some changes to the latest HEAD so I can't say for sure that's the only one, but still, the vast marjority of it worked, really good!
I didn't read it in its entirity from start to finish, so I may have missed details, but I did refer to it extensively.
At a high level, it feels like it's been re-written a bit, half old half new, I tend to see this effect when documentation has patches applied to it, it no longer feels natural, I could be off the mark though.
There are some sections like the following:
Which left me wondering what D and C were, and I was just now still unable to locate "D".
Hope this is useful!