Conversation
Summary of ChangesHello @GioLogist, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request lays the groundwork for integrating external wallet authentication into the B3 sign-in process. It enhances the existing Turnkey authentication modal by introducing a new step where users can select their preferred authentication method: either the current email/OTP flow or a new external wallet connection option. While the user interface for wallet selection and initial detection of wallets is implemented, the complete backend authentication logic for external wallets is noted as a work in progress. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces initial support for external wallet authentication within the TurnkeyAuthModal by adding a new dependency (@turnkey/react-wallet-kit), modifying the modal's flow to include method selection and wallet connection steps, and exposing an enableWalletAuth prop. The reviewer noted that the wallet authentication feature is not yet fully functional, recommending either disabling it or improving communication until it's complete. Additionally, the reviewer suggested using thirdweb's existing wallet connectors instead of direct window.ethereum access for better robustness and making the hardcoded 'ethereum' chain dynamic to support multi-chain capabilities.
| // For now, we'll need to implement the actual Turnkey wallet auth | ||
| // This would involve signing a message with the wallet and sending it to the backend | ||
| // TODO: Implement full wallet authentication flow with Turnkey | ||
| setWalletError("Wallet authentication coming soon. Please use email authentication for now."); |
There was a problem hiding this comment.
The TODO comment and the user-facing message "Wallet authentication coming soon. Please use email authentication for now." indicate that the wallet authentication feature is not yet functional. Presenting a non-functional option to users can lead to a poor experience. Consider either:
- Disabling the option: If the feature is not ready, it might be better to disable the "External Wallet" button or hide it entirely until it's fully implemented.
- Clearer communication: If it must be shown, ensure the message is very prominent and explains why it's not available (e.g., "This feature is under development and will be available soon.").
Given this is a [WIP] PR, this might be acceptable for now, but it's a high-priority item before merging to main.
| if (typeof window !== "undefined" && (window as any).ethereum) { | ||
| const ethereum = (window as any).ethereum; | ||
| const accounts = await ethereum.request({ method: "eth_requestAccounts" }); |
There was a problem hiding this comment.
Directly accessing window.ethereum can be less robust than using thirdweb's wallet connectors, which are already integrated into the SDK. Leveraging thirdweb's utilities for wallet detection and connection would provide a more consistent and resilient experience, handling various wallet injection scenarios and potential edge cases more gracefully.
| { | ||
| name: ethereum.isMetaMask ? "MetaMask" : "Ethereum Wallet", | ||
| address: accounts[0], | ||
| chain: "ethereum", |
There was a problem hiding this comment.
The chain property is hardcoded to "ethereum" here. If the SDK is intended to support multiple EVM chains, this should be made dynamic to reflect the currently active chain or allow for selection of other supported chains. Hardcoding it limits the flexibility and potential multi-chain capabilities of the wallet authentication feature.
1 participant
[LINEAR_ISSUE_ID_HERE]
Description
Write a description.
Test Plan
Screenshots
For BE, include snippets, response payloads and/or curl commands to test endpoints
[FE] Before
[FE] After
[BE] Snippets/Response/Curl
automerge=false