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: trades from native assets & utxos #6260

Merged
merged 1 commit into from
Feb 20, 2024
Merged

Conversation

0xApotheosis
Copy link
Member

Description

Fixes an issue in release which prevents trading from native assets or UTXOs.

Regression from #6217

Problem summary:

  • useIsApprovalNeeded was returning undefined if selectAllowanceCryptoBaseUnit evaluated to undefined (which happens in the case of native fee assets or UTXOs
  • This caused isApprovalNeeded to be undefined for these assets, causing the isLoading condition of isApprovalNeeded === undefined to be met

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)

N/A - current release blocker.

See #6257 (comment).

Risk

High Risk PRs Require 2 approvals

Medium. A small change, but does affect the approval checks of transactions.

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

Any transaction that requires an approval check (trades and LP deposits).

Testing

  • Trades from native assets should now work
  • Trades from UTXOs should now work
  • Trades from ERC tokens on an EVM should still work

Engineering

☝️

Operations

☝️

Screenshots (if applicable)

Infinite spinner state that this fixes:

Screenshot 2024-02-20 at 5 28 05 pm

@0xApotheosis 0xApotheosis requested a review from a team as a code owner February 20, 2024 06:29
@0xApotheosis 0xApotheosis merged commit a7ca22c into release Feb 20, 2024
7 checks passed
@0xApotheosis 0xApotheosis deleted the fix-native-allowance branch February 20, 2024 06:51
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