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

feat(x/cosmwasmpool): Sending token_in_max_amount to the contract before running contract msg #5855

Merged
merged 5 commits into from
Jul 28, 2023

Conversation

iboss-ptk
Copy link
Contributor

@iboss-ptk iboss-ptk commented Jul 17, 2023

What is the purpose of the change

In order for transmuter to treat LP shares token as swappable asset, on SwapExactAmountOut it needs to burn the expected token in, but previous implementation sends token to the contract after running sudo msg, which means at the time, contract does not have enough token to burn.

This change fix the aforementioned issue by:

  • sending token_in_denom with token_in_max_amount to the contract
  • run sudo msg
  • send the remaining token back to sender

(E.g.: This pull request improves documentation of area A by adding ....

Testing and Verifying

The existing test is updated to cover the changes

Documentation and Release Note

  • Changelog entry added to Unreleased section of CHANGELOG.md?

Where is the change documented?

  • Specification (x/cosmwasmpool/README.md)

@iboss-ptk iboss-ptk changed the title feat(x/cosmwasmpool): Sending token_in_max_amount to the contract before running contract msg feat(x/cosmwasmpool): Sending token_in_max_amount to the contract before running contract msg Jul 17, 2023
@github-actions github-actions bot added the C:docs Improvements or additions to documentation label Jul 17, 2023
@iboss-ptk iboss-ptk marked this pull request as ready for review July 17, 2023 15:19
@iboss-ptk iboss-ptk added V:state/compatible/backport State machine compatible PR, should be backported V:state/compatible/no_backport State machine compatible PR, depends on prior breaks and removed V:state/compatible/backport State machine compatible PR, should be backported labels Jul 17, 2023
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.

LGTM

x/cosmwasmpool/README.md Show resolved Hide resolved
@iboss-ptk iboss-ptk removed the request for review from ValarDragon July 24, 2023 14:54
@ValarDragon ValarDragon added V:state/breaking State machine breaking PR and removed V:state/compatible/no_backport State machine compatible PR, depends on prior breaks labels Jul 28, 2023
@mergify mergify bot merged commit 18dd3f2 into main Jul 28, 2023
3 checks passed
@mergify mergify bot deleted the boss/send-token-in-max-to-contract branch July 28, 2023 19:15
VitalyV1337 pushed a commit to VitalyV1337/osmosis-1 that referenced this pull request Jul 31, 2023
…efore running contract msg (osmosis-labs#5855)

## What is the purpose of the change

In order for [transmuter to treat LP shares token as swappable asset](osmosis-labs/transmuter#6), on `SwapExactAmountOut` it needs to burn the expected token in, but previous implementation sends token to the contract after running sudo msg, which means at the time, contract does not have enough token to burn.

This change fix the aforementioned issue by:
- sending `token_in_denom` with `token_in_max_amount` to the contract
- run sudo msg
- send the remaining token back to sender

*(E.g.: This pull request improves documentation of area A by adding ....*

## Testing and Verifying

The existing test is updated to cover the changes

## Documentation and Release Note

  - [x] Changelog entry added to `Unreleased` section of `CHANGELOG.md`?

Where is the change documented? 
  - [x] Specification (`x/cosmwasmpool/README.md`)
@github-actions github-actions bot mentioned this pull request Feb 15, 2024
@github-actions github-actions bot mentioned this pull request Apr 15, 2024
@github-actions github-actions bot mentioned this pull request May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:automerge C:docs Improvements or additions to documentation V:state/breaking State machine breaking PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants