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

[CL]: Add swaprouter queries to stargate whitelist #3907

Merged
merged 4 commits into from
Jan 6, 2023

Conversation

mattverse
Copy link
Member

Closes: #3882

What is the purpose of the change

This PR adds swaprouter queries(namely Numpools, EstimateSwapExactAmountIn, EstimateSwapExactAmountOut) to stargate query whitelist.

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? yes
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? yes
  • How is the feature or change documented? (not applicable / specification (x/<module>/spec/) / Osmosis docs repo / not documented)

@mattverse mattverse added the V:state/compatible/no_backport State machine compatible PR, depends on prior breaks label Jan 3, 2023
@mattverse mattverse changed the title Add swaprouter queries to stargate whitelist [CL]: Add swaprouter queries to stargate whitelist Jan 3, 2023
@mattverse mattverse requested review from p0mvn and czarcas7ic and removed request for p0mvn January 3, 2023 11:05
Copy link
Member

@p0mvn p0mvn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! LGTM after the gamm queries are brought back.

CHANGELOG.md Outdated Show resolved Hide resolved
Comment on lines 87 to 88
setWhitelistedQuery("/osmosis.gamm.v1beta1.Query/EstimateSwapExactAmountIn", &gammtypes.QuerySwapExactAmountInResponse{})
setWhitelistedQuery("/osmosis.gamm.v1beta1.Query/EstimateSwapExactAmountOut", &gammtypes.QuerySwapExactAmountOutResponse{})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still want to keep these to avoid breaking old contracts

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brought them back for now, do we have further plans to deprecate them for the contracts though? I think it'd be awkward when in long term we support the two same queries but from different modules

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would need to communicate with the integrators, asking whether anyone is using these.

If contracts are already utilizing some of these, removal from the stargate-whitelist wouldn't be easy so we might have to accept keeping these while there are production users.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sad 😭

@p0mvn p0mvn added V:state/breaking State machine breaking PR and removed V:state/compatible/no_backport State machine compatible PR, depends on prior breaks labels Jan 5, 2023
@p0mvn
Copy link
Member

p0mvn commented Jan 5, 2023

I marked this as state-breaking to re-review before v15 since this touches the stargate whitelist.

@mattverse mattverse merged commit d730ec7 into main Jan 6, 2023
@mattverse mattverse deleted the mattverse/swaprouter-stargate- branch January 6, 2023 06:46
@github-actions github-actions bot mentioned this pull request Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
V:state/breaking State machine breaking PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(x/swaprouter): add new queries to stargate whitelist
2 participants