Skip to content

[496] Features: Push notification, Incoming Transaction, Websockets#1739

Draft
ifaouibadi wants to merge 9 commits intodevelopfrom
features/push-notification
Draft

[496] Features: Push notification, Incoming Transaction, Websockets#1739
ifaouibadi wants to merge 9 commits intodevelopfrom
features/push-notification

Conversation

@ifaouibadi
Copy link
Copy Markdown
Member

No description provided.

@github-actions
Copy link
Copy Markdown

@ifaouibadi ifaouibadi marked this pull request as draft December 14, 2022 10:40
@ifaouibadi ifaouibadi force-pushed the features/push-notification branch 2 times, most recently from 59d59af to 1da6eac Compare January 9, 2023 12:03
@ifaouibadi ifaouibadi force-pushed the features/push-notification branch 4 times, most recently from 25a4750 to 935e243 Compare January 11, 2023 11:19
@ifaouibadi ifaouibadi marked this pull request as ready for review January 11, 2023 11:42
@ifaouibadi ifaouibadi changed the title [496] Features/push notification [496] Features: Push notification, Incoming Transaction, Websockets Jan 11, 2023
@ifaouibadi ifaouibadi requested review from CedrikNikita, konradodo, peronczyk, pstachel and subhod-i and removed request for peronczyk and pstachel January 11, 2023 11:54
@ifaouibadi ifaouibadi force-pushed the features/push-notification branch from 88e16c6 to 3ae242a Compare January 12, 2023 10:39
@ifaouibadi ifaouibadi force-pushed the features/push-notification branch from 3ae242a to 80dc992 Compare January 13, 2023 12:30
@ifaouibadi ifaouibadi requested a review from konradodo January 13, 2023 12:36
@subhod-i
Copy link
Copy Markdown
Collaborator

subhod-i commented May 4, 2023

I remember @thepiwo once said we will introduce a web socket for multisig backend too to detect new multisig vaults.
@ifaouibadi Please take this into consideration we should be able to use WebsocketClient.ts file even for other web socket connections.

@thepiwo
Copy link
Copy Markdown

thepiwo commented May 4, 2023

I started work on websockets for the ga-multisig backend, but we never followed up with an actual plan on integration, so I abandoned the approach for the time aeternity/ga-multisig-backend@515b598

let's have a meeting to pick it up again

@subhod-i
Copy link
Copy Markdown
Collaborator

subhod-i commented May 4, 2023

I started work on websockets for the ga-multisig backend, but we never followed up with an actual plan on integration, so I abandoned the approach for the time aeternity/ga-multisig-backend@515b598

let's have a meeting to pick it up again

This is important to optimize the performance of the SH wallet, especially to eliminate polling to the multisig backend. Let's talk soon ✌️

@ifaouibadi
Copy link
Copy Markdown
Member Author

ifaouibadi commented May 8, 2023

  1. Although the WebSocket methods are static it looks like there are many parallel connections that keep on getting added as we switch networks. If you observe the notifications are still received from the first connection made. Here is the screenshot

Improved network unsubscribe from previous when it's been changed

  1. Notifications are received only from the active network. I believe notifications should be network agnostic with each notification indicating the network.

it's not a good idea to listen for all networks been added in the app as it can cause notifications duplicate or receiving unexpected notification from non active network

@subhod-i

@subhod-i
Copy link
Copy Markdown
Collaborator

subhod-i commented May 9, 2023

it's not a good idea to listen for all networks been added in the app as it can cause notifications duplicate or receiving unexpected notification from non active network

Fine.

@CedrikNikita
Copy link
Copy Markdown
Collaborator

CedrikNikita commented Jun 2, 2023

I suggest to add a redirect from transaction notification to open details of this transaction .

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is actually not an channel name. Also Object is already used in the IMiddlewareWebSocketSubscriptionMessage payload prop.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it's a channel name :D https://github.com/aeternity/ae_mdw#publishing-message-format

used to subscribe for specific address

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes done in this file are not necessary as non of the files are using them.

Copy link
Copy Markdown
Collaborator

@subhod-i subhod-i left a comment

Choose a reason for hiding this comment

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

We will have a re-review. Multiple updates

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.

8 participants