Skip to content
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

Proposal: make time varint encoded (instead of fixed length) #208

Closed
liamsi opened this issue Jul 19, 2018 · 1 comment
Closed

Proposal: make time varint encoded (instead of fixed length) #208

liamsi opened this issue Jul 19, 2018 · 1 comment

Comments

@liamsi
Copy link
Contributor

liamsi commented Jul 19, 2018

In protobuf there is https://github.com/golang/protobuf/blob/14aad3d5ea4c323bcd7a2137e735da24a76e814c/ptypes/timestamp/timestamp.pb.go#L99-L108
where seconds and nanos are encoded as varint. See also the corresponding proto file / message:
https://github.com/golang/protobuf/blob/14aad3d5ea4c323bcd7a2137e735da24a76e814c/ptypes/timestamp/timestamp.proto#L121-L133

In amino, we currently encode time as fixed length (signed) ints:

go-amino/encoder.go

Lines 123 to 136 in 2106ca6

err = encodeFieldNumberAndTyp3(w, 1, Typ3_8Byte)
if err != nil {
return
}
err = EncodeInt64(w, s)
if err != nil {
return
}
err = encodeFieldNumberAndTyp3(w, 2, Typ3_4Byte)
if err != nil {
return
}
err = EncodeInt32(w, ns)

I think it could be worthwhile to change this behaviour. Then, people working on amino in other languages, or interefacing via vanilla protobuf could just use proto's encoding / types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant