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

Add full stack whitelist AMM test #9

Closed
ethanfrey opened this issue Jul 13, 2021 · 4 comments · Fixed by #44
Closed

Add full stack whitelist AMM test #9

ethanfrey opened this issue Jul 13, 2021 · 4 comments · Fixed by #44
Assignees
Milestone

Comments

@ethanfrey
Copy link
Contributor

In tests, create 4 contracts using multitest:

  • tfi-factory generic AMM factory
  • cw4-group as a DSO placeholder (for testing)
  • A cw20-whitelist contract that points to the above cw4-group
  • Use tfi-factory to create tfi-pair that swaps between a native token and this cw20-whitelist token

Actions:

  • Demo that even cw4-group members cannot provide liquidity to the tfi-pair.
  • Add tfi-pair to the cw4-group "whitelist"
  • Allow cw4-group members to add liquidity to tfi-pair and make a swap
  • Show non cw4-group members cannot do a swap in either direction
@hashedone
Copy link
Collaborator

It looks like four separated tests cases too me:

  • Full stack success tests (everyone is in cw4-group, so everyone can swap and provide liquidity)
  • Failure test where if tfi-pair is not in a group, noone can provide liquidity to it or trade (best way would be to initially make it belong to group, so it is possible to provide some initial liquidity to it, so it is clear it is not a reason)
  • Failure test where if token itself is not in a group, noone can interact with it
  • Failure test where addresses from outside group cannot interact

Testing all scenario in single test makes big blob which is difficult to follow, and in case of failure it is slightly more difficult to find a reason. Splitting test to test well defined path is often better (it is also just single responsibility rule mapped to the QA world).

@hashedone hashedone self-assigned this Aug 11, 2021
@ethanfrey
Copy link
Contributor Author

ethanfrey commented Aug 11, 2021

Yes, 4 different tests make sense when you have this nice suite setup.

Full stack success tests (everyone is in cw4-group, so everyone can swap and provide liquidity)

Yes

Failure test where if tfi-pair is not in a group, noone can provide liquidity to it or trade (best way would be to initially make it belong to group, so it is possible to provide some initial liquidity to it, so it is clear it is not a reason)

tfi-pair doesn't have a group. the asset in it is a dso token and in a group. we can have it part of a group that has one member, that can fill liquidity. but all other users are not in the group and no one can do anything with it.

Failure test where if token itself is not in a group, noone can interact with it

I think this is already tested in dso-token code. you can improve those for sure.

Failure test where addresses from outside group cannot interact

Yes

@ethanfrey
Copy link
Contributor Author

Clarifying something about tokens. There are three tokens we want to test.

  • native: this allows anyone to use it (already tested)
  • cw20-base: this allow anyone to use it (already tested)
  • dso-token: this must link to a group and can only be used as group members

For the suite, I would imagine setting something up which is cw20 to cw20. It takes group_a: Option<Addr> and group_b: Option<Addr>. If they are none, instantiate a cw20-base for that side. If it is some, instantiate dso-token for that side.

I would make a different suite setup that is only cw20-cw20 so the interfaces match and just a slightly different setup.

@ethanfrey
Copy link
Contributor Author

The cases for me are:

  • both sides are pemissionless
    • anyone can swap (this is already covered well by your tests)
  • one side is limited to a group
    • group members can swap both ways
    • non-members cannot swap either way
  • both sides are limited to (different groups)
    • members of only group A cannot swap
    • members of only group B cannot swap
    • members of group A and B can swap

"can swap" and "can provide liquidity" can both be tested, they have the same controls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants