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 utxo unknown fees #8189

Merged
merged 7 commits into from
Nov 27, 2024
Merged

fix: thorchain utxo unknown fees #8189

merged 7 commits into from
Nov 27, 2024

Conversation

gomesalexandre
Copy link
Contributor

@gomesalexandre gomesalexandre commented Nov 25, 2024

Description

This PR ensures we get fees estimation back at input time for UTXOs, by leveraging xpub when available. This is after #8079 went in, which made UTXO fees unknown for UTXOs regardless of wallet connected state, and only got the fees on final quote getting before sign and broadcast.

While this was a feature, ops rightly pointed out that it effectively looks and feels like a regression, and if it does so, it probably is.
Fixed by leveraging xpub if handy (i.e if wallet connected) to actually estimate things.

TODO: 

- [x] Test me with and without a wallet connected 
  - [x] ensure this doesn't rug quotes with UTXO sells without a wallet connected 
  - [x] ensure this doesn't rug execution with a wallet connected
- [x] cleanup in a nice enough way so that us deviating from TradeRateInput types with what's effectively a quoteish input isn't too messy

Issue (if applicable)

closes #

Risk

High Risk PRs Require 2 approvals

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

Testing

Engineering

Operations

  • 🏁 My feature is behind a flag and doesn't require operations testing (yet)

Screenshots (if applicable)

  • No wallet connected
image
  • Wallet connected

https://jam.dev/c/873fc60a-82fe-429f-a274-b99f05553e65

@gomesalexandre gomesalexandre changed the title feat: thorchain utxo unknown fees fix: thorchain utxo unknown fees Nov 25, 2024
@gomesalexandre gomesalexandre marked this pull request as ready for review November 26, 2024 09:13
@gomesalexandre gomesalexandre requested a review from a team as a code owner November 26, 2024 09:13
Copy link
Contributor

@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.

✅ Gas fee shows for UTXOs when wallet is connected
✅ Gas fee still unknown for UTXOs without wallet connected
✅ Connecting a wallet after getting a UTXO quote correctly fetches the gas fees

@0xApotheosis 0xApotheosis merged commit ef3dbcc into develop Nov 27, 2024
3 checks passed
@0xApotheosis 0xApotheosis deleted the fix_thorchain_fees branch November 27, 2024 00:18
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