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

Move pushData to pushServer (v0.8.x) #61

Closed
pedrouid opened this Issue Dec 3, 2018 · 4 comments

Comments

Projects
2 participants
@pedrouid
Copy link
Member

pedrouid commented Dec 3, 2018

In order to keep the dappName (Issue #60), it needs to detached both from the Dapp itself and the Bridge server to control the way push notifications are displayed.

The first proposal for the issue #60 was to completely remove it. However following the discussion on the WalletConnect forum thread (https://discuss.walletconnect.org/t/push-server-metadata/123), the consensus was that this was a critical change for the WalletConnect UX and it should be reversed.

In order to reverse it, it will include a significant tech spec change on how the current architecture is handled. The second proposal is to make the Bridge server "dumber" by removing all logic related to the push notification (pushType and pushToken) and simply trigger a notification using the provided pushWebhook and the already known sessionID. To achieve this, the session creation flow will complete the session request between the Dapp and the Wallet and be followed by an HTTP request to the Push server to store the push notification data plus some other metadata.

Push server will now be in charge to handle this data and format the push notification display using the following:

Data

  • sessionID (for indexing)
  • pushType (for choosing push notification service provider)
  • pushToken (for targeting the push notification destination)

Metadata

  • dappName (for formatting the push notification message)
  • language (for translating the push notification message)

The language parameter should follow the ISO 639-1 language codes standard. This additional parameter will provide localization based on the user's device.

Additionally, the user can change to not share the metadata with the push server in order to preserve privacy.

@pedrouid pedrouid added this to In Progress in WalletConnect v1.0 Roadmap Dec 3, 2018

@pedrouid

This comment has been minimized.

Copy link
Member Author

pedrouid commented Dec 3, 2018

Furthermore this improves the flexibility of the Protocol to support different levels of privacy both for Wallet providers and users themselves.

  • No Push Notifications (complete independence from the mobile platform)
  • Push Notifications (improved user experience but privacy preserving)
  • Push Notifications + Metadata (best UX with custom notifications but compromised privacy)
@ethluz

This comment has been minimized.

Copy link

ethluz commented Jan 24, 2019

push Preferably not mandatory. I like pushdata is optional,not require。

@pedrouid

This comment has been minimized.

Copy link
Member Author

pedrouid commented Jan 24, 2019

Yes, it's completely optional and it has been changed on the currently in development v1.0.x

@pedrouid

This comment has been minimized.

Copy link
Member Author

pedrouid commented Jan 25, 2019

Fixed on #69

@pedrouid pedrouid closed this Jan 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment