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

Disallow multidimensinal arrays #272

Closed
liamsi opened this issue May 21, 2019 · 4 comments
Closed

Disallow multidimensinal arrays #272

liamsi opened this issue May 21, 2019 · 4 comments

Comments

@liamsi
Copy link
Contributor

liamsi commented May 21, 2019

Amino allows encoding of [][]T directly or as fields (where T can be different from []byte). This does not have a matching representation in protobuf, meaning that sth. like
message MsgName { repeated repeated T = 1;} is not allowed in protobuf (for some
message T { /* ...*/ }.

Not sure how much this is used in the SDK. Tendermint only uses [][]byte as far as I can see.

(note that [][]byte fields are still ok as it can be represented as repeated bytes is valid in protobuf, too)

@alexanderbez
Copy link

I don't think this is a problem for the SDK. We don't use [][]T in amino encodings from what I can see.

@liamsi
Copy link
Contributor Author

liamsi commented May 21, 2019

I was under the same impression. But I was not 100% sure as one can't easy spot type aliases where this still might be used implicitly, e.g.

type SomeType struct {
  FooBars []Foo
}

// this can be in a different package:
type Foo []Bar

Amino certainly allows this as one can see in the tests, e.g.:

go-amino/binary_test.go

Lines 16 to 20 in 716e340

type TestStruct struct {
A []byte
B []int
C [][]byte
D [][]int

My suggestion here would be to throw an error in case amino detects this. This would ensure that users won't use this in the future, too.

@alexanderbez
Copy link

Ahhh yes! Good point. I'll have to double check that we're not doing this, but I doubt we are. And if we are, I'm sure we can resolve it.

@liamsi
Copy link
Contributor Author

liamsi commented May 21, 2019

Thanks @alexanderbez!

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

3 participants