search

tendermint / go-amino

Protobuf3 with Interface support - Designed for blockchains (deterministic, upgradeable, fast, and compact)
Other
260 stars 84 forks source link

disallow multidimensional slices and arrays #283

Closed tessr closed 4 years ago

tessr commented 4 years ago

Stop allowing multidimensional slices and arrays, in order to increase proto3 compatibility. Addresses #272.

This is my first PR, and a little bit of a shot in the dark, so please let me know if I need to restructure anything!

tessr commented 4 years ago

Ah, just remembered this needs to permit multidimensional arrays of bytes. Will add

tessr commented 4 years ago

This now permits byte arrays and slices at any depth, and is ready for review!

zmanian commented 4 years ago

Manually checked both Tendermint and the CosmosSdk and [][]byte is the multidimensional struct we are actively serializing.

Actually trying to integrate this branch into tendermint and the sdk got me into a dependency resolution snowball.

tac0turtle commented 4 years ago

oooh yea, you have to update roughly every repo of ours to make it work. We can merge this into master and then update ismail's branch with it or merge it into ismail's branch, which is preferred? @zmanian then I can run through and update all the existing branches I created in iavl, tendermint and cosmos-sdk to test it with the simulator,

tessr commented 4 years ago

Good question, I'm not sure if it would be better to merge this into master or into Ismail's feature branch. Which is more copacetic with our normal git flow? :)

Fwiw, this change could stand on its own, if we're trying to avoid bloating Ismail's PR.

tac0turtle commented 4 years ago

This repo and Iavl don't really follow our normal gitflow. They are outliers in a way, this repo won't receive a release till ismail's pr is complete so it may make more sense to merge to master then back update ismail's. Will do merge this into master then go about testing in the sdk again.