-
Notifications
You must be signed in to change notification settings - Fork 28
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
Initial work to implement Sapphire snap connection #431
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for oasisprotocol-sapphire-paratime canceled.
|
47c82fc
to
9d74f0f
Compare
@@ -43,6 +44,7 @@ export function isLegacyProvider<T extends object>( | |||
|
|||
export interface SapphireWrapOptions { | |||
fetcher: KeyFetcher; |
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.
Options object is optional. When provided, TS requires fetcher, but we should be able just to set enableSapphireSnap
flag. As fetcher always defaults to new KeyFetcher()
should it now be fetcher: KeyFetcher | undefined;
?
@@ -80,10 +82,10 @@ export function isWrappedEthereumProvider<P extends EIP2696_EthereumProvider>( | |||
* @param options (optional) Re-use parameters from other providers | |||
* @returns Sapphire wrapped provider | |||
*/ | |||
export function wrapEthereumProvider<P extends EIP2696_EthereumProvider>( | |||
export async function wrapEthereumProvider<P extends EIP2696_EthereumProvider>( |
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.
Does it affect all integrations? For example I don't see ether-v6 is relying on wrapEthereumProvider
. It has it's own logic. Do we plan to apply the same code in a few places, or share some snap utils across packages?
I guess for ether-v6 we should call notifySapphireSnap
right after https://github.com/oasisprotocol/sapphire-paratime/blob/main/integrations/ethers-v6/src/index.ts#L96
params: { | ||
id: transactionData, | ||
ephemeralSecretKey: hexlify(secretKey), | ||
peerPublicKey: peerPublicKey.key, |
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.
hexlify
fixes #389
This provides the decryption keys to snap.
Warning
If an RPC server pretends to implement the MetaMask snap protocol it could trick users into revealing the transaction encryption key.
For this reason, we have to explicitly enable Snap support in the dApp, by passing the
enableSapphireSnap
option.Usage:
This must only be done if the dApp is sure that the provider it's connecting to is MetaMask.
TODO:
wallet_invokeSnap