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

x/gamm: Add check for correct input asset denoms to CalcJoinPoolShares #1906

Closed
AlpinYukseloglu opened this issue Jun 30, 2022 · 1 comment · Fixed by #1931
Closed

x/gamm: Add check for correct input asset denoms to CalcJoinPoolShares #1906

AlpinYukseloglu opened this issue Jun 30, 2022 · 1 comment · Fixed by #1931
Assignees
Labels
C:x/gamm Changes, features and bugs related to the gamm module. T:bug 🐛 Something isn't working

Comments

@AlpinYukseloglu
Copy link
Contributor

AlpinYukseloglu commented Jun 30, 2022

Background

Our current implementation of CalcJoinPoolShares checks to make sure the number of denoms sent in for an all-asset join is correct (here), but has no check for whether those assets are actually the correct denoms. This makes it so that entirely invalid inputs (i.e. correct number of input tokens but incorrect denoms) successfully make it through the entire set of all-asset join calculations until they are incidentally caught in the tail-end single-asset join section in a check in updateIntermediaryPoolAssetsLiquidity.

I suggest we add an additional check to the input tokens that makes sure they are of the correct denoms so that the function never assumes invalid inputs are correct for calculating all-asset joins.

Suggested Design

  • Add a check here that makes sure that the denoms are equivalent as well (simplest way is probably just to use DenomSubsetOf to ensure both are denom subsets of each other)

Acceptance Criteria

  • All existing and new tests should pass
@AlpinYukseloglu AlpinYukseloglu added the C:x/gamm Changes, features and bugs related to the gamm module. label Jun 30, 2022
@AlpinYukseloglu AlpinYukseloglu changed the title Add check for correct input asset denoms to CalcJoinPoolShares x/gamm: Add check for correct input asset denoms to CalcJoinPoolShares Jun 30, 2022
@p0mvn
Copy link
Member

p0mvn commented Jun 30, 2022

Good catch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:x/gamm Changes, features and bugs related to the gamm module. T:bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants