-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat: migrate bitcoin generate txs, closes LEA-1735 #668
Conversation
7f3c7d2
to
77c5ff7
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #668 +/- ##
==========================================
+ Coverage 29.41% 30.25% +0.83%
==========================================
Files 175 177 +2
Lines 6836 6924 +88
Branches 457 471 +14
==========================================
+ Hits 2011 2095 +84
- Misses 4825 4829 +4
|
77c5ff7
to
1bbdff0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @fbwoolf, like the component generic 👍🏼
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing work!! 🚀
values: BitcoinTransactionValues; | ||
} | ||
|
||
export function useGenerateBtcUnsignedTransactionNativeSegwit( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: i think it's better to keep this function as a pure function and then wrap it in useCallback in the component when we need it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 this doesn't need to be a hook as it's not using any state. useCallback isn't needed if it's defined outside of react
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, thanks, will change.
b8bec60
to
a08eb22
Compare
a08eb22
to
e085851
Compare
This PR migrates code to generate the unsigned psbt for btc sends.
I did some refactoring here using generic types so we can extend our form context to accommodate diff form data in each form flow. I needed to do this to handle loading
feeRates
andutxos
for bitcoin specifically. I spent a lot of time trying to rethink this and hope it is a good solution. I've created a few more loaders to get rid of some undefined returns we had in the code I ported over from the extension.I've also spent some time making sure to write tests that were missing, and refactoring bitcoin fees code as I migrated it over.
As mentioned in Slack, I'd like to standardize our
Fees
model type after this work.