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

feat: show pending and failed transactions for nwc #3292

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

pavanjoshi914
Copy link
Contributor

@pavanjoshi914 pavanjoshi914 commented Dec 17, 2024

show failed and pending transactions for nwc

Fixes #3291
image
image
image
image
image
image
image
image

Copy link

socket-security bot commented Dec 17, 2024

@pavanjoshi914 pavanjoshi914 marked this pull request as ready for review December 17, 2024 18:28
@pavanjoshi914 pavanjoshi914 requested review from reneaaron, rolznz and im-adithya and removed request for reneaaron, rolznz and im-adithya December 18, 2024 05:50
src/common/utils/helpers.ts Outdated Show resolved Hide resolved
@pavanjoshi914 pavanjoshi914 requested a review from rolznz December 24, 2024 11:07
@@ -31,10 +31,12 @@ export interface ConnectorTransaction {
/**
* Settle date in UNIX milliseconds
*/
settleDate: number;
settleDate: number | null;
creationDate?: number;
Copy link
Contributor

Choose a reason for hiding this comment

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

which connectors do not have a creation date? can't this be taken from the bolt11 invoice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

all other connectors only pass settled transactions and we don't even need creationDate for them except for nwc. so i kept it optional and passed this field only for nwc connector. maybe we can be consistent accross and pass it for all now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for commando there is no creation date passed via api. https://docs.corelightning.org/reference/lightning-listinvoices

Copy link
Contributor

Choose a reason for hiding this comment

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

it can be taken from the bolt11 invoice, can't it?

@@ -128,7 +128,7 @@ export default class LaWallet implements Connector {
return {
data: {
transactions: parsedTransactions.sort(
(a, b) => b.settleDate - a.settleDate
(a, b) => (b.settleDate ?? 0) - (a.settleDate ?? 0)
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 this connector should be affected like this in this PR? if lawallet only has settled transactions, there is now extra code that does not even apply.

Can't you sort the transactions before mapping them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that was just to make typescript happy we have common type definitions for all the connectors. now settleDate can be either number | null (cause of nwc pending and failed transactions). any better approach here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can't you sort the transactions before mapping them?

@rolznz rolznz requested a review from bumi January 2, 2025 04:51
@pavanjoshi914 pavanjoshi914 force-pushed the nwc-transaction-states branch from f245ef1 to c44add1 Compare January 2, 2025 13: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.

[Feature] Show pending and failed transactions from the Hub
2 participants