-
Notifications
You must be signed in to change notification settings - Fork 981
REF: lightning wallet minor refactor #8006
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
Conversation
Co-authored-by: overtorment <[email protected]>
Co-authored-by: overtorment <[email protected]>
limpbrains
left a comment
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.
What about Transactions that are alreay in the real store? What existing user will see ?
|
@limpbrains very good findings, thanks! |
| const currentDate = new Date(); | ||
| const now = (currentDate.getTime() / 1000) | 0; // eslint-disable-line no-bitwise | ||
| const invoiceExpiration = updatedUserInvoice.timestamp + updatedUserInvoice.expire_time; | ||
| const invoiceExpiration = (updatedUserInvoice.timestamp ?? 0) + (updatedUserInvoice.expire_time ?? 0); |
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.
Bug: Invoice Polling Fails on String Prop
The invoice status polling useEffect has two issues: it incorrectly starts when the invoice prop is a string (not yet decoded), as !(invoice as LightningTransaction).ispaid evaluates to true for strings. Furthermore, if an invoice is already paid when the component loads, the polling interval is not cleared and isFetchingInvoices remains true, leading to an incorrect UI state.
|
Wake the fuck up samurai, we have PRs to merge [all PRs for @limpbrains] https://github.com/BlueWallet/BlueWallet/pulls/review-requested/limpbrains |
limpbrains
left a comment
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.
My pleasure
|
Unbelievable. You, [subject name here], must be the pride of [subject hometown here]! |

I carved out all refactorings from #7961 to deliver separately
additionally, refactoreed
Transactiontype, got rid of.receivein favour of.timestamp. we used to have both, but now we have only one and its definately an integer representing seconds, way less mess. @marcosrdz i also changed all occurances in native code without uderstanding whats going on, so you might want to review those changes extra carefully.i also tried to refactor Transaction & LightningTransaction types and make everything correctly, but this led to a lot of ts complaints all over the codebase so Ill leave it for later