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

Sign In With Solana #783

Merged
merged 26 commits into from
Aug 7, 2023
Merged

Sign In With Solana #783

merged 26 commits into from
Aug 7, 2023

Conversation

jordaaash
Copy link
Collaborator

@jordaaash jordaaash commented Jun 2, 2023

This PR adds a signIn method to adapters.

This method uses the Wallet Standard solana:signIn feature from anza-xyz/wallet-standard#12

Some tasks:

  • Use published alpha of @solana/wallet-standard with SIWS
  • Add signIn method, types, and abstract adapter to base package
  • Expose signIn method to React context
  • Add signIn to Unsafe Burner adapter for testing
  • Modify the example app to support signIn
  • Test the example with Unsafe Burner adapter
  • Add verifySignMessage and verifySignIn functions
  • Replace tweetnacl with @noble/crypto packages
  • Fix other build issues
  • Publish alpha of @solana/wallet-adapter with SIWS
  • Update @solana/wallet-standard-wallet-adapter-base to use published alpha of @solana/wallet-adapter
  • Modify StandardWalletAdapter to support signIn
  • Publish alpha of @solana/wallet-standard-wallet-adapter-base
  • Use published alpha of @solana/wallet-standard-wallet-adapter-base
  • Test the example with Phantom
  • Enable signIn to be used instead of connect in connect and autoConnect behaviors
  • Test the example with Unsafe Burner adapter and Phantom again
  • Publish production version of @solana/wallet-standard with SIWS
  • Update @solana/wallet-adapter to use production version of @solana/wallet-standard with SIWS
  • Publish production version of @solana/wallet-adapter with SIWS
  • Update @solana/wallet-standard to use production version of @solana/wallet-adapter with SIWS
  • Publish patch version of @solana/wallet-standard

@socket-security
Copy link

socket-security bot commented Jun 14, 2023

No top level dependency changes detected. Learn more about Socket for GitHub ↗︎

@jordaaash jordaaash changed the title [WIP] Sign In With Solana Sign In With Solana Aug 7, 2023
@jordaaash jordaaash marked this pull request as ready for review August 7, 2023 04:26
@jordaaash jordaaash merged commit e9be008 into master Aug 7, 2023
3 checks passed
@jordaaash jordaaash deleted the sign-in branch August 7, 2023 04:44
@kolpav
Copy link

kolpav commented Aug 11, 2023

@jordansexton Hello, can you please confirm that the signIn is really in the latest non-alpha version? I think there must be a mistake somewhere I am running latest non alpha version of wallet-adapter/packages/starter/example and the signIn is always disabled because the method is undefined on the wallet from useWallet.

If I were to guess where the problem might have occurred from commit history here I can see you thoroughly tested everything but in 9cbc406 switched versions and the signIn became unusable for some reason because technically its not the version where everything was tested. Unfortunately the branch is deleted by now so I cannot verify.

I did two tests one with cloned master of this repo and also installing the necessary packages into my nextjs application with everything up to date including chrome and all extensions. I also did the same for most of the examples here, I think I am doing everything right but the signIn is always undefined. Last thing I have tried was alpha branch but I am unable to run the examples due to module not found error. I have latest pnpm and LTS node but I am not really familiar with pnpm and its usage in monorepos like this one.

Below is screenshot where you can see disabled sign in button. Even if I make the button clickable it fails.

CleanShot 2023-08-11 at 04 14 13@2x

Could you please help me I have spend few hours on this without any progress while trying to make this feature work in application already running in production where we started encountering sporadic problems with auth. State in wallet after nextjs transitions sometimes doesn't make sense or useWallet on same page in different components having different results. It might be totally unrelated to problem I am describing here. I think I have ran out of options to try or I am overlooking something obvious.

@jordaaash
Copy link
Collaborator Author

@kolpav it's not in the production build of Phantom yet. You can test it in the demo by selecting the Unsafe Burner wallet. I'll follow up here when it's released in Phantom with an actual integration guide.

@jordaaash
Copy link
Collaborator Author

To further explain this, the signIn method will only be defined if the selected wallet implements this method. Because the version of Phantom you have doesn't, the button is disabled. Take a look at the code for the Unsafe Burner wallet adapter to understand what's going on.

@kolpav
Copy link

kolpav commented Aug 11, 2023

@jordansexton Than you for getting back, I think now I understand how the release process works and why the method is available but not supported yet. Is there rough estimate when its going to land in phantom? I'll test my current dev implementation with Unsafe Burner in the meantime and wait till its going to be ready, really like where this is going UX/DX wise.

@jordaaash
Copy link
Collaborator Author

@bfriel or @0xproflupin may know about the release timeline.

@bfriel
Copy link
Contributor

bfriel commented Aug 11, 2023

@bfriel or @0xproflupin may know about the release timeline.

This is going out in Phantom's next extension release! No set ETA but shooting for within the next ~2 weeks. Will make an announcement when we are closer to launch.

@alex-fung
Copy link

hey @jordansexton, thanks for doing all this – really awesome stuff. Just wanted to clarify – seems like whenever dApps want to trigger the SIWS experience, they should be calling signIn on the adapter based on this PR. However, in web3auth's docs, it's stated to use a connect + signMessage approach on a SIWS-constructed message: https://siws.web3auth.io/implementfrontend. Are both approaches correct? If the dApp attempts to trigger the sign-in flow via the approach in web3auth's docs, is signMessage responsible for detecting that it's a SIWS message so that it shows the sign-in dialog instead?

@0xproflupin
Copy link
Contributor

hey @alex-fung, web3auth's docs do not use the new SIWS spec. You can use it still, but it won't make use of the new feature which clubs the connect + signMessage flows.
Here's the spec + integration steps you can follow: https://github.com/phantom/sign-in-with-solana

@alex-fung
Copy link

Thank you @0xproflupin!

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.

None yet

6 participants