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: thorchain swapper #6259

Merged
merged 1 commit into from
Feb 20, 2024
Merged

fix: thorchain swapper #6259

merged 1 commit into from
Feb 20, 2024

Conversation

0xApotheosis
Copy link
Contributor

@0xApotheosis 0xApotheosis commented Feb 20, 2024

Description

Fixes a production issue that prevents users from trading UTXOs.

Regression in #6177.

Summary of the issue:

  • An isAddress(address) check was added to the isSmartContractAddress query enabled logic
  • For UTXOs, this will return false, and so the isSmartContractAddress query will not fire
  • This means the data returned from the useIsSmartContractAddress hook is undefined
  • Because we fail closed (intentional, for safety) in downstream logic consuming smart contract checks (i.e. if we can't confirm it isn't a smart contract, we assume it is), we thus fail on undefined (UTXO "addresses")

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)

Closes #6258

Risk

High Risk PRs Require 2 approvals

Medium - getting the wrong prevents users from trading, though ideally this PR makes things right und unbreaks thorchain.

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

  • Namely: any swapper/LP/lending transaction that deals with UTXOs
  • But also, any swapper/LP/lending transaction

Testing

  • Trades to and from UTXOs should work (EVMs should still work, too!)
  • Deposits into LPs should work
  • Deposits/withdraws into/from THORChain lending should work

Engineering

☝️

Operations

☝️

Screenshots (if applicable)

Screenshot 2024-02-20 at 1 52 03 pm

@0xApotheosis 0xApotheosis requested a review from a team as a code owner February 20, 2024 03:06
@0xApotheosis 0xApotheosis merged commit 8365014 into main Feb 20, 2024
7 checks passed
@0xApotheosis 0xApotheosis deleted the fix-thorchain-swapper branch February 20, 2024 03:52
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