Skip to content
This repository has been archived by the owner on Dec 22, 2019. It is now read-only.

Encoding int32, int64 to signed varint? #9

Closed
Thunnini opened this issue Nov 9, 2018 · 3 comments
Closed

Encoding int32, int64 to signed varint? #9

Thunnini opened this issue Nov 9, 2018 · 3 comments

Comments

@Thunnini
Copy link
Contributor

Thunnini commented Nov 9, 2018

package main

import (
	"encoding/hex"
	"fmt"

	amino "github.com/tendermint/go-amino"
)

type TestBin struct {
	A int64 `binary:"fixed64"`
}

type TestBin2 struct {
	A int64
}

func main() {
	cdc := amino.NewCodec()

	bz, err := cdc.MarshalBinary(TestBin{1})
	if err != nil {
		panic(err)
	}
	fmt.Println(hex.EncodeToString(bz))

	bz, err = cdc.MarshalBinary(TestBin2{1})
	if err != nil {
		panic(err)
	}
	fmt.Println(hex.EncodeToString(bz))
}

Result is

09090100000000000000
020802

https://github.com/tendermint/go-amino/blob/master/codec.go#L521
https://github.com/tendermint/go-amino/blob/master/binary-encode.go#L89

In go-amino, it seemed that they encode int32, 64 to fixed length when users define variables with fixed option.
But js-amino encode int32, 64 to fixed length by default.
I think it is better to encode int32, 64 to varint by default, and support options that encode int32, 64 to fixed length.

@TanNgocDo
Copy link
Contributor

Thanks @Thunnini , options is the target I will implement. I'm trying to fix your last issues. Stay tune for more update :)

@Thunnini
Copy link
Contributor Author

Thunnini commented Nov 9, 2018

@TanNgocDo, I really appreciate your efforts. This project is helping me a lot. :)

@TanNgocDo
Copy link
Contributor

Could you please check new commit: f918a9b ?

@Thunnini Thunnini closed this as completed Dec 1, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants