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

Cw 853 mweb utxos are not displayed in coin control screen #1873

Merged

Conversation

Serhii-Borodenko
Copy link
Contributor

@Serhii-Borodenko Serhii-Borodenko commented Dec 12, 2024

Issue Number (if Applicable): Fixes #

Description

Please include a summary of the changes and which issue is fixed / feature is added.

Pull Request - Checklist

  • Initial Manual Tests Passed
  • Double check modified code and verify it with the feature/task requirements
  • Format code
  • Look for code duplication
  • Clear naming for variables and methods


return <String, dynamic>{};
}).toList();
if (result == null) throw Exception('Failed to get listunspent');
Copy link
Contributor

Choose a reason for hiding this comment

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

this will have side effects
ex:

  1. it's called in a Future.wait, so, throwing an exception would cause the whole Future.wait to fail
  2. the second place it's called, has a try and catch, but it will make it not continue the flow
  3. and also would make it report that error to the user through the send error report popup, thus giving the support more unnecessary tickets for connection issues

Comment on lines 1453 to 1455
} catch (_) {
rethrow;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should rethrow while it's not handled correctly, and will just show the send error report popup to the user

@OmarHatem28 OmarHatem28 merged commit d55e635 into main Dec 14, 2024
1 of 3 checks passed
@OmarHatem28 OmarHatem28 deleted the CW-853-MWEB-utxos-are-not-displayed-in-coin-control-screen branch December 14, 2024 00:37
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