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

solana ledger sign in #358

Merged
merged 5 commits into from
Nov 13, 2024
Merged

solana ledger sign in #358

merged 5 commits into from
Nov 13, 2024

Conversation

shan-57blocks
Copy link
Collaborator

@shan-57blocks shan-57blocks commented Nov 12, 2024

1, Solana Ledger sign in by signing transaction
2, Separate solana and evm sign in

@shan-57blocks shan-57blocks requested review from ljiatu and mliu November 12, 2024 07:48
import { ErrorType } from '.'

const WALLETS_WITH_SERIALIZED_TX: WalletName<string>[] = [
LedgerWalletName,
Copy link
Contributor

Choose a reason for hiding this comment

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

Some users use their ledger wallet through Phantom/Solflare. We need to test whether we can get the correct wallet name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I just searched, cannot get the hard wallet name if connecting through Phantom/Solflare. Need to find a way to handle this

const buildAuthTx = async (nonce: string): Promise<Transaction> => {
const tx = new Transaction()

const PROGRAM_ID = new PublicKey(
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be a file-level const?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

return message.prepareMessage()
}

const buildAuthTx = async (nonce: string): Promise<Transaction> => {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should call this message instead of nonce.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

tx.feePayer = new PublicKey(address)
tx.recentBlockhash = (await connection.getLatestBlockhash()).blockhash
const signedTx = await signTransaction(tx)
const serializedTx = Array.from(signedTx.serialize())
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to base64 encode the serialized tx instead of passing it as an array of numbers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

const { account, chainId, provider } = useWeb3React()

useEffect(() => {
if (chainType === CHAIN_TYPE.EVM) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we combine lines 61 and 62?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

const account = publicKey?.toString() ?? ''

useEffect(() => {
if (chainType === CHAIN_TYPE.SOLANA) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we combine lines 127 and 128?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

onVerificationComplete()
}

export const useAuthErrorHandingEvm = (
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo. Missing l in Handling. Same for the file name.

}
}

export const useAuthErrorHandingSolana = (
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo. Missing l in Handling. Same for the file name.

].includes(error.response?.data?.detail?.type)

const isWalletNotCreatedError = error === 'WalletNotCreatedException'
const isWalletNotSignInError = error === 'WalletNotSignInException'
Copy link
Contributor

Choose a reason for hiding this comment

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

Where are these errors coming from? Doesn't seem like they are from the backend. From a grammar perspective, the second error should be called WalletNotSignedInException.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These 2 are frontend defined errors. WalletNotCreatedException is thrown from earn page, WalletNotSignInException is thrown in points earned modal after supply.

ljiatu
ljiatu previously approved these changes Nov 13, 2024
@shan-57blocks shan-57blocks merged commit fcee289 into develop Nov 13, 2024
2 checks passed
@shan-57blocks shan-57blocks deleted the ledger-sign-in branch November 13, 2024 08:52
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.

3 participants