-
Notifications
You must be signed in to change notification settings - Fork 8
Pcv/smart account #214
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
Pcv/smart account #214
Conversation
@pcarranzav is attempting to deploy a commit to the The Graph Foundation team on Vercel, but is not a member of this team. To resolve this issue, you can:
To read more about collaboration on Vercel, click here. |
1553616
to
dfa589b
Compare
f1072bd
to
8f2fa9d
Compare
af2a48f
to
bd4d67f
Compare
await Connect.login({ walletClient, signer, syncServerUri, storage, identityToken, rpcUrl, chain: CHAIN }); | ||
}, | ||
[identityToken], | ||
[identityToken, signMessage], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
signMessage leads to infinite loops sometimes from my experience - would remove it from the dependency array
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe it's fine - just saw I used it below as well, can you test that signup and logging in again works? then we can keep it
{ | ||
headers: { | ||
'privy-id-token': identityToken, | ||
'account-address': accountAddress, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the accountAddress can be access in the backend based on the privy id token. Do we need to send it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's not the same, the address in the privy id token is the owner wallet, which is different from the smart account address. We need to identify the user by the smart account address.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahhh ok
navigate({ to: '/', replace: true }); | ||
} catch (error) { | ||
alert('Failed to login'); | ||
console.log('Failed to login'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we leave it at alert
? otherwise users get no feedback in case it fails
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, sorry, this was during testing, as it was annoying to debug with the alert
// Check that the signature is valid | ||
const valid = await verifyMessage({ | ||
address: accountAddress as Hex, | ||
address: (await signer.getAddress()) as Hex, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this the same address as the accountAddress? I need the account address to query e.g. list of public spaces the current account is a member of
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, as I mentioned above, the signer address is different from the account address. We have the account address in other places (it is deterministically generated from the owner address) so we don't need it in this function afaict.
|
||
export const encryptAppIdentity = async ( | ||
signer: Signer, | ||
accountAddress: string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above, can I still access the accountAddress in the app then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, getSmartAccountWalletClient will give you the smart account client, and then client.account.address is the accountAddress - you can see how we generate and store it in Connect.login
No description provided.