[x/gamm][Misc improvements] Msg defined twice in different proto files #1185
Assignees
Comments
|
for more context, here is the command I'm running in the repo w all protobufs: https://github.com/pyramation/osmosis-protobufs/blob/master/bin/run-proto.sh |
daniel-farina
added
T:task ⚙️
17 tasks
4 tasks
|
I saw the progress on the PR and tested out the protoc gen — the current PR works 🙌🏻 |
|
Matt had a good point: #1208 (comment) We should change the txs to be defined under the correct package, and just leave the balancer pool defined under the wrong proto package (which won't cause conflicts) |
|
great, I like the idea of |
p0mvn
changed the title
p0mvn
changed the title
mergify bot
pushed a commit
that referenced
this issue
May 16, 2022
Closes: [#1185](#1185) ## What is the purpose of the change With proto Msg service defined twice(both in Balancer proto and gamm package proto), this is currently causing problems in typescript proto generation. As @ValarDragon mentioned in #1208, > They existed at the same import path, because due to interface serialization depending on the amino route (which depended on the proto import path), changing the import path requires a state migration thats a bit cumbersome to do. This PR solves this by making txs under the correct package, but leaving balacner pool proto definition under the legacy wrong one, `package osmosis.gamm.v1beta1`. Solving the problem this way allows us to solve without having to need state breaking changes or additional migration for existing pools. In addition to solving the exisitng problem of duplicated Msg in one package, this PR also changes stableswap pool proto package to the correct package. ## Brief Changelog - Change proto package for balancer msg ## Documentation and Release Note - Does this pull request introduce a new feature or user-facing behavior changes? maybe - Is a relevant changelog entry added to the `Unreleased` section in `CHANGELOG.md`? yes - How is the feature or change documented? not applicable
xla
pushed a commit
to meka-dev/osmosis
that referenced
this issue
May 17, 2022
Closes: [osmosis-labs#1185](osmosis-labs#1185) With proto Msg service defined twice(both in Balancer proto and gamm package proto), this is currently causing problems in typescript proto generation. As @ValarDragon mentioned in osmosis-labs#1208, > They existed at the same import path, because due to interface serialization depending on the amino route (which depended on the proto import path), changing the import path requires a state migration thats a bit cumbersome to do. This PR solves this by making txs under the correct package, but leaving balacner pool proto definition under the legacy wrong one, `package osmosis.gamm.v1beta1`. Solving the problem this way allows us to solve without having to need state breaking changes or additional migration for existing pools. In addition to solving the exisitng problem of duplicated Msg in one package, this PR also changes stableswap pool proto package to the correct package. - Change proto package for balancer msg - Does this pull request introduce a new feature or user-facing behavior changes? maybe - Is a relevant changelog entry added to the `Unreleased` section in `CHANGELOG.md`? yes - How is the feature or change documented? not applicable
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I'm working on a workflow to produce TS definitions and using
protocto generate them. In this process, I'm getting an errorhttps://github.com/osmosis-labs/osmosis/blob/main/proto/osmosis/gamm/pool-models/balancer/tx.proto#L9
https://github.com/osmosis-labs/osmosis/blob/main/proto/osmosis/gamm/v1beta1/tx.proto#L9
Seems that the
Msgidentifier is being used twice. Maybe not an issue in go, but in generating viats-protoit seems to not be happy.The text was updated successfully, but these errors were encountered: