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

Passkey support #808

Open
wants to merge 4 commits into
base: development
Choose a base branch
from
Open

Passkey support #808

wants to merge 4 commits into from

Conversation

DaniSomoza
Copy link
Contributor

@DaniSomoza DaniSomoza commented May 8, 2024

What it solves

Resolves #789 & #793 by adding support for passkeys in the protocol-kit.

How this PR fixes it

protocol-kit instantiation using passkeys

You 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. See rawId docs
  • publicKey: The passkey’s public key, generated via the getPublicKey() method, necessary for generating X & Y coordinates. See getPublicKey() method docs
  const passkeyCredential = await navigator.credentials.create({ ... });

  const rawId = passkeyCredential.rawId;
  const publicKey = passkeyCredential.response.getPublicKey();

  const passkey = {
    rawId,
    publicKey
  };

  const safeWithPasskey = await Safe.create({
    provider: RPC_URL,
    signer: passkey,
    safeAddress
  });

Add a passkey owner

Now you can use the createAddOwnerTx method present in the protocol-kit using a passkey:

  const safeTransaction = await safe.createAddOwnerTx({
    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 function

The isOwner function now checks if a specific address or passkey is an owner of the current Safe:

  const safe = await Safe.create({
    provider: RPC_URL,
    safeAddress,
  });

  await safe.isOwner(passkey); // returns true or false

Sign a transaction using passkeys

  const safeWithPasskey = await Safe.create({
    provider: RPC_URL,
    signer: passkey,
    safeAddress,
  });

  const dumpTransaction = {
    to: safeAddress,
    value: "0",
    data: "0x",
  };

  const safeTransaction = await safeWithPasskey.createTransaction({
    transactions: [dumpTransaction],
  });

  const signedTransaction = await safeWithPasskey.signTransaction(
    safeTransaction
  );


// execution need to be handled via EOA

Added the WebAuthn Signer Factory Contract

This SafeWebAuthnSignerFactory contract is used to create a Signer and to get the address of a Signer.

@DaniSomoza DaniSomoza changed the base branch from main to development May 8, 2024 16:26
@DaniSomoza DaniSomoza changed the base branch from development to Abitype-1_3_0-safe-contract May 8, 2024 16:27
@DaniSomoza DaniSomoza self-assigned this May 9, 2024
@DaniSomoza DaniSomoza marked this pull request as ready for review May 9, 2024 14:19
const safeProvider = new SafeProvider({ provider: this.#provider, signer: this.#signer })
const safeProvider = new SafeProvider({
provider: this.#provider,
signer: this.#signer as string
Copy link
Member

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,
Copy link
Member

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?

@@ -38,6 +39,7 @@ import {
SigningMethod,
SigningMethodType,
SwapOwnerTxParams,
passkeyArgType,
Copy link
Member

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)
Copy link
Member

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 = {
Copy link
Member

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({
Copy link
Member

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
Copy link
Member

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,
Copy link
Member

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.
Copy link
Contributor

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,
Copy link
Contributor

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?

Suggested change
customContractAddress: customContracts?.simulateTxAccessorAddress,
customContractAddress: customContracts?.safeWebAuthnSignerFactoryAddress,
customContractAbi: customContracts?.safeWebAuthnSignerFactoryAbi

Copy link
Member

@yagopv yagopv left a 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',
    }

@dasanra dasanra changed the base branch from Abitype-1_3_0-safe-contract to development May 16, 2024 12:17
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.

[Passkey] Add passkey owner to an already existing Safe [Passkeys] Add compatibility to the protocol-kit
3 participants