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
Passkey support #808
base: development
Are you sure you want to change the base?
Passkey support #808
Conversation
const safeProvider = new SafeProvider({ provider: this.#provider, signer: this.#signer }) | ||
const safeProvider = new SafeProvider({ | ||
provider: this.#provider, | ||
signer: this.#signer as 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.
Can we avoid the cast ?
@@ -29,6 +29,7 @@ import ModuleManager from './managers/moduleManager' | |||
import OwnerManager from './managers/ownerManager' | |||
import { | |||
AddOwnerTxParams, | |||
AddPasskeyOwnerTxParams, |
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.
Why two separate types? Could we use an Union type instead?
packages/protocol-kit/src/Safe.ts
Outdated
@@ -38,6 +39,7 @@ import { | |||
SigningMethod, | |||
SigningMethodType, | |||
SwapOwnerTxParams, | |||
passkeyArgType, |
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.
Uppercase?
const isPasskeySigner = await this.#safeProvider.isPasskeySigner() | ||
if (isPasskeySigner) { | ||
const txHash = await this.getTransactionHash(transaction) | ||
const signedHash = await this.#safeProvider.signMessage(txHash) |
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.
We usually do this.signHash(txHash)
for signing tx hashes and will adjust the V and return the EthSafeSignature object
y: string | ||
} | ||
|
||
export type passkeyArgType = { |
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.
Uppercase?
const chainId = await safeProvider.getChainId() | ||
const customContracts = contractNetworks?.[chainId.toString()] | ||
|
||
const safeWebAuthnSignerFactoryContract = await getSafeWebAuthnSignerFactoryContract({ |
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.
This can be retrieved from the safeProvider right?
@@ -40,7 +41,7 @@ type SafeConfigWithPredictedSafeProps = { | |||
|
|||
export type SafeConfigProps = { | |||
provider: SafeProviderConfig['provider'] | |||
signer?: SafeProviderConfig['signer'] | |||
signer?: string | passkeyArgType |
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.
Why this change? SafeProviderConfig signer should be the correct one right?
constructor( | ||
passkeyRawId: ArrayBuffer, | ||
coordinates: PasskeyCoordinates, | ||
safeWebAuthnSignerFactoryContract: SafeWebAuthnSignerFactoryContractImplementationType, |
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.
What about using the safeProvider as a param and retrieve the contract and external provider from there?
import SafeProvider from '@safe-global/protocol-kit/SafeProvider' | ||
|
||
/** | ||
* SafeWebAuthnSignerFactoryContract_v1_4_1 is the implementation specific to the Safe Proxy Factory contract version 1.4.1. |
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 Safe Proxy Factory
is mentioned in multiple comments here, let's change it to the correct name to avoid confusion
const safeWebAuthnSignerFactoryContract = await safeProvider.getSafeWebAuthnSignerFactoryContract( | ||
{ | ||
safeVersion, | ||
customContractAddress: customContracts?.simulateTxAccessorAddress, |
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.
Should be like this, right?
customContractAddress: customContracts?.simulateTxAccessorAddress, | |
customContractAddress: customContracts?.safeWebAuthnSignerFactoryAddress, | |
customContractAbi: customContracts?.safeWebAuthnSignerFactoryAbi |
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.
I think we need an utility (buildPasskey) to build the object used with navigator.credentials.create
{
pubKeyCredParams: [
{
// ECDSA w/ SHA-256: https://datatracker.ietf.org/doc/html/rfc8152#section-8.1
alg: -7,
type: 'public-key',
},
],
challenge: crypto.getRandomValues(new Uint8Array(32)),
rp: {
name: 'Safe Wallet',
},
user: {
displayName: 'Safe Owner',
id: crypto.getRandomValues(new Uint8Array(32)),
name: 'safe-owner',
},
timeout: 60000,
attestation: 'none',
}
What it solves
Resolves #789 & #793 by adding support for passkeys in the
protocol-kit
.How this PR fixes it
protocol-kit
instantiation using passkeysYou can create an instance of the
protocol-kit
using a passkey as a Signer. The passkey Signer is a contract compliant with EIP-1271 standards.the passkey argument is an object with 2 keys:
rawId
: The raw identifier of the passkey. SeerawId
docspublicKey
: The passkey’s public key, generated via the getPublicKey() method, necessary for generating X & Y coordinates. SeegetPublicKey()
method docsAdd a passkey owner
Now you can use the
createAddOwnerTx
method present in the protocol-kit using a passkey:This function returns the Safe transaction to add a passkey as owner and optionally change the threshold. If the passkey Signer contract has not been deployed, this function creates a batch that includes both the Signer contract deployment and the addOwner transaction.
isOwner
functionThe isOwner function now checks if a specific address or passkey is an owner of the current Safe:
Sign a transaction using passkeys
Added the WebAuthn Signer Factory Contract
This
SafeWebAuthnSignerFactory
contract is used to create a Signer and to get the address of a Signer.