Saqif280 commented 2 years ago

Problem Overview

This branch addresses requests in https://github.com/solana-labs/solana-pay/issues/100 to update the Solana Pay protocol specification to support wallet disambiguation in iOS clients and deeplinking to address some security vulnerabilities.

Solution & Decision Log

The open issue proposes several changes; this PR is the first step towards implementing those changes from Solana Pay's end. In addition to this, there will need to be followup PRs to fully close the issue.

Handling the Spec Changes

The issue poster suggested to allow for a url like https://phantom.app/code? instead of the current url scheme solana:. However, the suggested URL has a path, which should be used for the recipient according to the current protocol spec. There are a few options of how we might want to handle this:

We can make a breaking change to the protocol spec and require recipient as a query parameter. I'm avoiding this since breaking changes are costly.
We can continue to keep the recipient in the path, and just parse it out of the end of the wallet URL (in the cases where we use a wallet URL). This is a bit of a rigid solution since it relies on wallet applications registering a wildcard route to handle recipient in the path. The parsing also feels a little hacky to me, so I’m not a fan of this one.
We can allow for a recipient query parameter. This will be encoded in the URL when a wallet is provided, otherwise it will default to putting recipient in the path. This solution is backwards compatible and flexible so I decided to implement this.

Supporting Wallet Disambiguation

This solution creates a TypeScript enum for tracking supported wallets called SupportedWallet. Wallets can create PRs to add their wallet and their registered url to this enum. This branch leaves the enum empty for now, which means we can't build much around this until enum entries are added.

Testing

Documentation changes were manually verified in local docusaurus
Code changes were tested via unit tests
- Note: encodeURL.test.ts will need additional tests once supported wallets have been added to the SupportedWallet enum.

Next Steps

System diagrams in documentation need to be updated to reflect complete call paths (given the new request query param usage). This could be included in this PR but I don't have access to the files for these diagrams.
Documentation needs to be added on how to set up authentication servers for merchants and how to interact with authentication servers as wallets. This could also be included in this PR, but would be confusing without the updated system diagrams.
The Point of Sale package needs to be updated to provide UIs for wallet selection.

vercel[bot] commented 2 years ago

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.