-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Define supported mappings (proto->JSON or JSON->proto) #2
Comments
How is this true? What changes in the content? The proto3 spec does define JSON decoding: https://developers.google.com/protocol-buffers/docs/proto3#json. But I think it is out of the scope of this document because decoding wouldn't be used in the signature verification process. cosmos/cosmos-sdk#6031 implies a JSON API via https://github.com/grpc-ecosystem/grpc-gateway, but that doesn't relate to signing. Or am I missing something? |
Oh, damn, I misunderstood
I confused this with unicode normalization. Sorry for the noise! But I'll keep searching for incompatibilities. This proto-JSON bridge was not designed for crypto.
If decoding is in-scope or not really depends on the question if and to which degree JSON->proto must be supported and which guarantees are needed. If we need
Not only a JSON API (that could consume basse64 encoded protobuf documents). But that JSON encoded transaction need to be decoded back to proto without access to the original protobuf document. |
No, decoding the JSON is irrelevant for this scope.
Yeah, I think that's out of the scope of this particular canonicalization document which is more about signing and state encoding than RPC/REST interfaces. |
The current document speaks about encoding proto->JSON. I think we should get clarity if this is the only supported mapping or not. There are other places that imply a usage of JSON->proto (let's call this "(JSON) decoding" in this context).
For signing and signature verification purposes, only encoding is needed.
m = hash(serialize(proto))
andprivkey
into ECDSA secp256k1 signm = hash(serialize(proto))
,pubkey
andsignature
into secp256k1 verifyWhat cosmos/cosmos-sdk#6031 suggests is that there is a reverse mapping (decode) as well. However, such a decoding would be lossy in the current implementation (i.e.
decode(encode(original)) != original
), e.g. because JSON Canonical Form changes strings to a normalized form, i.e. manipulates content, not only structure.I think it would be important to know the goals of those two mappings in order to verify if the spec achives them. Obviously, things get much easier by explicitely disallowing the decoding.
The text was updated successfully, but these errors were encountered: