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

fix: allow quote sell amounts to be lower than user input sell amount #6265

Merged
merged 2 commits into from
Feb 20, 2024

Conversation

woodenfurniture
Copy link
Member

Description

Fixes issue where quotes with a sell amount LOWER than user input are marked as invalid (namely cowswap quotes). We need to allow this case, and only mark quotes selling an amount HIGHER than the user inputted value as invalid.

Pull Request Type

  • 🐛 Bug fix (Non-breaking Change: Fixes an issue)
  • 🛠️ Chore (Non-breaking Change: Doc updates, pkg upgrades, typos, etc..)
  • 💅 New Feature (Breaking/Non-breaking Change)

Issue (if applicable)

NA

Risk

High Risk PRs Require 2 approvals

Low risk of broken trade qutoes.

What protocols, transaction types or contract interactions might be affected by this PR?

Testing

Check quotes are working generally
Check CowSwap quotes are working (e.g FOX -> ETH $30)

Engineering

Operations

Screenshots (if applicable)

Before fix
image

After fix
image

@woodenfurniture woodenfurniture requested a review from a team as a code owner February 20, 2024 22:06
Copy link
Member

@0xApotheosis 0xApotheosis left a comment

Choose a reason for hiding this comment

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

This diff seems sane, though is it expected the only CoW swap quotes are working?

Screenshot 2024-02-21 at 9 32 38 am

Copy link
Member

@0xApotheosis 0xApotheosis left a comment

Choose a reason for hiding this comment

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

I also can't get a THORChain quote:

This branch

Screenshot 2024-02-21 at 9 37 19 am

Develop

Screenshot 2024-02-21 at 9 37 41 am

Copy link
Member

@0xApotheosis 0xApotheosis left a comment

Choose a reason for hiding this comment

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

CoW and THORSwap quotes are great again.

@woodenfurniture woodenfurniture merged commit d4e6b47 into release Feb 20, 2024
3 checks passed
@woodenfurniture woodenfurniture deleted the fix-sell-amount-threshold-check branch February 20, 2024 23:14
0xApotheosis added a commit that referenced this pull request Feb 21, 2024
* fix: trades from native assets & utxos (#6260)

fix: native allowance

* fix: use correct chain id for matching buy account

* fix: allow quote sell amounts to be lower than user input sell amount (#6265)

* fix: allow quote sell amounts to be lower than user input sell amount

* fix: fix logic, add a teeny tiny thrshold so cowswap works

* fix: consolidate matching buy account id selection logic (#6268)

* chore: fix merge

---------

Co-authored-by: kaladinlight <[email protected]>
Co-authored-by: woody <[email protected]>
gomesalexandre pushed a commit that referenced this pull request Feb 22, 2024
* fix: trades from native assets & utxos (#6260)

fix: native allowance

* fix: use correct chain id for matching buy account

* fix: allow quote sell amounts to be lower than user input sell amount (#6265)

* fix: allow quote sell amounts to be lower than user input sell amount

* fix: fix logic, add a teeny tiny thrshold so cowswap works

* fix: consolidate matching buy account id selection logic (#6268)

* chore: fix merge

---------

Co-authored-by: kaladinlight <[email protected]>
Co-authored-by: woody <[email protected]>
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 this pull request may close these issues.

2 participants