credo-ts
credo-ts copied to clipboard
openwallet-foundation
Encoded indy attributes drop the leading 0's for numeric strings
PR #632 fixed an interop issue with ACA-Py. ACA-Py would remove leading 0's from numeric strings when encodign indy values, while AFJ would not do that. This would results in errors as we couldn't verify the encoded values matched the raw values.
After doing some interop testing between AFJ 0.1.0 and 0.2.0 it not seems this broke interop with AFJ 0.1.0. This issue is to discuss whether we should drop the leading 0's. As per @blu3beri's comment in #632, we should maybe treat number strings with leading 0's as strings. Probably if you're issuing a credential with a leading 0, that value is there for a reason:
Don't you think we should change this in aca-py? Keeping a number as a string might be done to keep the leading zeros for some, no clue which, use cases. The RFC does not specify to remove leading zeros if provided in a number string.
Maybe I miss something, but it seems more to me that removing characters from a string is unexpected.
Also see my comment on #632:
@JamesKEbert I just encountered an issue with this when testing for interop between AFJ 0.1.0 and 0.2.0. Now that I understand the issue better I'm not sure if we actually need to drop the 0 in this case.
E.g. in our demo we issue a "credit card" credential where the security code has a value 063. AFJ 0.1.0 will encode that as 063, while AFJ 0.2.0 and ACA-Py will encode it as 63. After thinking about it more, aren't both ACA-Py and AFJ 0.1.0 + AFJ 0.2.0 wrong? Shouldn't we take the 'this is not a number' path and create a hash of the value and convert that to a number?
In this case we should really threat the value as a string. This is of course horrible for interop with ACA-Py and older AFJ versions, but curious to hear what you think. Anyway by fixing this we now have broken interop with older versions of AFJ which is of course not great (although only if you issue credentials with number strings that have leading 0's)
Thats a difficult one. I think that numbers with leading zeros should be treated as strings. Your example of the security code is a valid use case where you would like to keep this. Any non-leading-zero int32 can just be kept as a number. I do believe however that this does remove range proofs on the value, but I might be mistaken and a string -> number conversion is done before the range proof.
If we decide to go this route, I strongly suggest we update the RFC with the leading zeroes note to avoid further issues.
Yes this will make range proofs impossible on values with leading zeroes. So if we e.g. want to check if your security code is >= 100 we wouldn't be able to do that.
@swcurran do you have any thoughts on this issue?
Sorry for the long delay in responding to this.
The key to the answer is found here in RFC0592 Indy Attachments - Encoding Claims sections (misnamed - should "AnonCreds Attachments" -- but that's for another day). And in that, we have a lovely contradiction.
- On the one hand, we have a rule that says only "32-bit integers are left as is", and everything else is stringified (if need be) and hashed. "Everything else" would include, I would think, strings of digits, even if they look like integers.
- A related question might be, in processing JSON, does a string of digits (e.g. "12345") imply an integer or a string, or is it ambiguous. If not ambiguous, that would make this question easy. I'm betting it's ambiguous.
- On the other hand, we have example code (from ACA-Py) and a gist of test values that includes examples of converting a string of digits (such as the example claim "zip") to be encoded as an integer, not a hash.
My proposal is that we stick to the RFC as documented in the example code and (especially) the test set, perhaps with a clarification in the text of the RFC. I suggestion that because that is what other frameworks (should) have done. It would be good to confirm that with an AATH test. For AFJ, we leave the change that James made and accept that this is a conflict between AFJ 0.1.0 and AFJ 0.2.0.
As noted on the merge that created this issue, as long as the frameworks handle the encoding consistently, all is good regardless of which approach is used.
I'd agree with Stephen's thoughts here--I think that as long as the frameworks handle the encoding consistently then we're okay. I'd also agree with the need/desire to update the RFC with a clarification. (also sorry for the late reply)
