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

Test: Additional test cases for TestCalcJoinPoolShares (multi-coin swap) #1732

Conversation

stackman27
Copy link
Contributor

@stackman27 stackman27 commented Jun 9, 2022

Closes: built on top of #1713

What is the purpose of the change

Add additional test cases to TestCalcJoinPoolShares;

  • Test: Multi-tokens In: swap unequal amounts, equal weights with 0.01 swap fee
  • Test: Multi-tokens In: swap unequal amounts, with unequal weights with 0.03 swap fee
  • Need to add error tolerance maybe on "0.0000001"

Testing and Verifying

This change added tests and can be verified as follows:
Adds unit test that checks shares for multi asset deposits

Documentation and Release Note

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

@github-actions github-actions bot added the C:x/gamm Changes, features and bugs related to the gamm module. label Jun 9, 2022
@stackman27 stackman27 changed the title Sis/test calc join pool shares swap tests Test: Additional test cases for TestCalcJoinPoolShares (multi-coin swap) Jun 9, 2022
@p0mvn
Copy link
Member

p0mvn commented Jun 9, 2022

@stackman27 I added some logic for err tolerance in #1731. You might find it useful here

@alexanderbez alexanderbez changed the base branch from main to bez/TestCalcJoinPoolShares-swap-tests June 9, 2022 19:33
Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

@stackman27 this should be based against bez/TestCalcJoinPoolShares-swap-tests

@stackman27 stackman27 force-pushed the sis/TestCalcJoinPoolShares-swap-tests branch from 9912c8c to 46a038c Compare June 9, 2022 20:58
// A_t = amount of deposited asset = 50,000
// B_t = existing balance of deposited asset in the pool prior to deposit = 1,000,000,000,000
// W_t = normalized weight of deposited asset in pool = 0.5 (equally weighted two-asset pool)
// swapFeeRatio = (1 - (1 - W_t) * swapFee)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we considering swapFeeRatio when depositing with exact ratios (i.e. no swap occurs)?

@stackman27 stackman27 force-pushed the sis/TestCalcJoinPoolShares-swap-tests branch from 9750c13 to 83c4394 Compare June 10, 2022 19:26
@alexanderbez alexanderbez deleted the branch osmosis-labs:bez/TestCalcJoinPoolShares-swap-tests June 10, 2022 19:44
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants