Skip to content

Commit

Permalink
Shared Signer Mapping
Browse files Browse the repository at this point in the history
This PR changes the behaviour of the shared signer to store its
configuration as if it were behind a mapping where the key is the shared
signer address. This allows multiple shared signer instances to be
configured for a single account (in case you ever need to deploy a 2/2
Safe owned by passkeys with 4337).

I created an "accessor" contract to prove that the storage location is
indeed the equivalent to a `mapping(address self => Signer)`.
  • Loading branch information
nlordell committed Jun 5, 2024
1 parent 4b7b71a commit d01c20c
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ contract SafeWebAuthnSharedSigner is SignatureValidator {
/**
* @notice Data associated with a WebAuthn signer. It represents the X and Y coordinates of the
* signer's public key as well as the P256 verifiers to use. This is stored in account storage
* starting at the storage slot {_SIGNER_SLOT}.
* starting at the storage slot {SIGNER_SLOT}.
*/
struct Signer {
uint256 x;
Expand All @@ -23,30 +23,38 @@ contract SafeWebAuthnSharedSigner is SignatureValidator {
}

/**
* @notice The starting storage slot containing the signer data.
* @custom:computed-as keccak256("SafeWebAuthnSharedSigner.signer") - 3
* @dev This value is intentionally computed to be a hash -3 as a precaution to avoid any
* @notice The storage slot of the mapping from shared WebAuthn signer address to signer data.
* @custom:computed-as keccak256("SafeWebAuthnSharedSigner.signer") - 1
* @dev This value is intentionally computed to be a hash -1 as a precaution to avoid any
* potential issues from unintended hash collisions, and have enough space for all the signer
* fields.
* fields. Also, this is the slot of a `mapping(address self => Signer)` to ensure that multiple
* {SafeWebAuthnSharedSigner} instances can coexist with the same account.
*/
uint256 private constant _SIGNER_SLOT = 0x2e0aed53485dc2290ceb5ce14725558ad3e3a09d38c69042410ad15c2b4ea4e6;
uint256 private constant _SIGNER_MAPPING_SLOT = 0x2e0aed53485dc2290ceb5ce14725558ad3e3a09d38c69042410ad15c2b4ea4e8;

/**
* @notice An error indicating a `CALL` to a function that should only be `DELEGATECALL`-ed.
*/
error NotDelegateCalled();

/**
* @dev Address of the launchpad contract itself. it is used for determining whether or not the
* contract is being `DELEGATECALL`-ed when setting signer data.
* @notice Address of the shared signer contract itself.
* @dev This is used for determining whether or not the contract is being `DELEGATECALL`-ed when
* setting signer data.
*/
address private immutable _SELF;

/**
* @notice The starting storage slot on the account containing the signer data.
*/
uint256 public immutable SIGNER_SLOT;

/**
* @notice Create a new shared WebAuthn signer instance.
*/
constructor() {
_SELF = address(this);
SIGNER_SLOT = uint256(keccak256(abi.encode(address(this), _SIGNER_MAPPING_SLOT)));
}

/**
Expand All @@ -66,7 +74,7 @@ contract SafeWebAuthnSharedSigner is SignatureValidator {
* @param account The account to request signer data for.
*/
function getConfiguration(address account) public view returns (Signer memory signer) {
bytes memory getStorageAtData = abi.encodeCall(ISafe(account).getStorageAt, (_SIGNER_SLOT, 3));
bytes memory getStorageAtData = abi.encodeCall(ISafe(account).getStorageAt, (SIGNER_SLOT, 3));

// Call the {StorageAccessible.getStorageAt} with assembly. This allows us to return a
// zeroed out signer configuration instead of reverting for `account`s that are not Safes.
Expand Down Expand Up @@ -114,16 +122,17 @@ contract SafeWebAuthnSharedSigner is SignatureValidator {
* @param signer The new signer data to set for the calling account.
*/
function configure(Signer memory signer) external onlyDelegateCall {
Signer storage signerSlot;
uint256 signerSlot = SIGNER_SLOT;
Signer storage signerStorage;

// solhint-disable-next-line no-inline-assembly
assembly ("memory-safe") {
signerSlot.slot := _SIGNER_SLOT
signerStorage.slot := signerSlot
}

signerSlot.x = signer.x;
signerSlot.y = signer.y;
signerSlot.verifiers = signer.verifiers;
signerStorage.x = signer.x;
signerStorage.y = signer.y;
signerStorage.verifiers = signer.verifiers;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion modules/passkey/contracts/test/TestDependencies.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ pragma solidity >=0.8.0;
import "@account-abstraction/contracts/interfaces/IEntryPoint.sol";
import "@account-abstraction/contracts/samples/VerifyingPaymaster.sol";
import "@safe-global/mock-contract/contracts/MockContract.sol";
import "@safe-global/safe-contracts/contracts/proxies/SafeProxyFactory.sol";
import "@safe-global/safe-contracts/contracts/libraries/MultiSend.sol";
import "@safe-global/safe-contracts/contracts/proxies/SafeProxyFactory.sol";
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// SPDX-License-Identifier: LGPL-3.0-only
pragma solidity ^0.8.0;

import {SafeWebAuthnSharedSigner} from "../4337/experimental/SafeWebAuthnSharedSigner.sol";

contract TestSharedWebAuthnSignerAccessor {
function getSignerConfiguration(address sharedSigner) external view returns (SafeWebAuthnSharedSigner.Signer memory signer) {
signer = _getSigners()[sharedSigner];
}

function _getSigners() private pure returns (mapping(address => SafeWebAuthnSharedSigner.Signer) storage signers) {
uint256 signersSlot = uint256(keccak256("SafeWebAuthnSharedSigner.signer")) - 1;

// solhint-disable-next-line no-inline-assembly
assembly ("memory-safe") {
signers.slot := signersSlot
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,40 +5,49 @@ import * as ERC1271 from '../../utils/erc1271'
import { DUMMY_AUTHENTICATOR_DATA, base64UrlEncode, getSignatureBytes } from '../../../src/utils/webauthn'
import { encodeWebAuthnSigningMessage } from '../../utils/webauthnShim'

const SIGNER_STORAGE_SLOT = ethers.toBeHex(BigInt(ethers.id('SafeWebAuthnSharedSigner.signer')) - 3n, 32)
const SIGNER_MAPPING_SLOT = BigInt(ethers.id('SafeWebAuthnSharedSigner.signer')) - 1n

describe('SafeWebAuthnSharedSigner', () => {
const setupTests = deployments.createFixture(async () => {
const { SafeL2, SafeProxyFactory, SafeWebAuthnSharedSigner } = await deployments.run()
const { SafeL2, SafeProxyFactory, CompatibilityFallbackHandler, SafeWebAuthnSharedSigner } = await deployments.run()

const safeSingleton = await ethers.getContractAt('ISafe', SafeL2.address)
const fallbackHandler = await ethers.getContractAt(CompatibilityFallbackHandler.abi, CompatibilityFallbackHandler.address)
const proxyFactory = await ethers.getContractAt('SafeProxyFactory', SafeProxyFactory.address)
const sharedSigner = await ethers.getContractAt('SafeWebAuthnSharedSigner', SafeWebAuthnSharedSigner.address)

const signerSlot = ethers.solidityPackedKeccak256(['uint256', 'uint256'], [sharedSigner.target, SIGNER_MAPPING_SLOT])

const MockContract = await ethers.getContractFactory('MockContract')
const mockAccount = await MockContract.deploy()
const mockVerifier = await MockContract.deploy()

const TestSharedWebAuthnSignerAccessor = await ethers.getContractFactory('TestSharedWebAuthnSignerAccessor')
const sharedSignerAccessor = await TestSharedWebAuthnSignerAccessor.deploy()

return {
safeSingleton,
fallbackHandler,
proxyFactory,
sharedSigner,
signerSlot,
mockAccount,
mockVerifier,
sharedSignerAccessor,
}
})

describe('getConfiguration', function () {
it('Should return a configuration for an account', async () => {
const { safeSingleton, sharedSigner, mockAccount, mockVerifier } = await setupTests()
const { safeSingleton, sharedSigner, signerSlot, mockAccount, mockVerifier } = await setupTests()

const publicKey = {
x: BigInt(ethers.id('publicKey.x')),
y: BigInt(ethers.id('publicKey.y')),
}

await mockAccount.givenCalldataReturn(
safeSingleton.interface.encodeFunctionData('getStorageAt', [SIGNER_STORAGE_SLOT, 3]),
safeSingleton.interface.encodeFunctionData('getStorageAt', [signerSlot, 3]),
ethers.AbiCoder.defaultAbiCoder().encode(
['bytes'],
[ethers.AbiCoder.defaultAbiCoder().encode(['uint256', 'uint256', 'uint176'], [publicKey.x, publicKey.y, mockVerifier.target])],
Expand Down Expand Up @@ -94,11 +103,47 @@ describe('SafeWebAuthnSharedSigner', () => {
expect(await sharedSigner.getConfiguration(mockAccount)).to.deep.equal([0n, 0n, 0n])
}
})

it('Should store the signer configuration in a mapping with the shared signer address as a key', async () => {
const { safeSingleton, fallbackHandler, proxyFactory, sharedSigner, mockVerifier, sharedSignerAccessor } = await setupTests()

const config = {
x: ethers.id('publicKey.x'),
y: ethers.id('publicKey.y'),
verifiers: ethers.toBeHex(await mockVerifier.getAddress(), 32),
}

const initializer = safeSingleton.interface.encodeFunctionData('setup', [
[sharedSigner.target],
1,
sharedSigner.target,
sharedSigner.interface.encodeFunctionData('configure', [config]),
fallbackHandler.target,
ethers.ZeroAddress,
0,
ethers.ZeroAddress,
])

const safeProxy = fallbackHandler.attach(
await proxyFactory.createProxyWithNonce.staticCall(safeSingleton, initializer, 0),
) as typeof fallbackHandler
await proxyFactory.createProxyWithNonce(safeSingleton, initializer, 0)

expect(
ethers.AbiCoder.defaultAbiCoder().decode(
['uint256', 'uint256', 'uint176'],
await safeProxy.simulate.staticCall(
sharedSignerAccessor.target,
sharedSignerAccessor.interface.encodeFunctionData('getSignerConfiguration', [sharedSigner.target]),
),
),
).to.deep.equal([config.x, config.y, BigInt(config.verifiers)])
})
})

describe('configure', function () {
it('Should configure an account', async () => {
const { safeSingleton, proxyFactory, sharedSigner, mockVerifier } = await setupTests()
const { safeSingleton, proxyFactory, sharedSigner, signerSlot, mockVerifier } = await setupTests()

const config = {
x: ethers.id('publicKey.x'),
Expand All @@ -120,10 +165,9 @@ describe('SafeWebAuthnSharedSigner', () => {
const account = await proxyFactory.createProxyWithNonce.staticCall(safeSingleton, initializer, 0)
await proxyFactory.createProxyWithNonce(safeSingleton, initializer, 0)

expect(await ethers.provider.getStorage(account, SIGNER_STORAGE_SLOT)).to.equal(config.x)
expect(await ethers.provider.getStorage(account, BigInt(SIGNER_STORAGE_SLOT) + 1n)).to.equal(config.y)
expect(await ethers.provider.getStorage(account, BigInt(SIGNER_STORAGE_SLOT) + 2n)).to.equal(config.verifiers)
expect(await ethers.provider.getStorage(account, ethers.id('SafeWebAuthnSharedSigner.signer'))).to.equal(ethers.ZeroHash)
expect(await ethers.provider.getStorage(account, signerSlot)).to.equal(config.x)
expect(await ethers.provider.getStorage(account, BigInt(signerSlot) + 1n)).to.equal(config.y)
expect(await ethers.provider.getStorage(account, BigInt(signerSlot) + 2n)).to.equal(config.verifiers)
})

it('Should revert if not DELEGATECALL-ed', async () => {
Expand All @@ -135,7 +179,7 @@ describe('SafeWebAuthnSharedSigner', () => {

describe('isValidSignature', function () {
it('Should return a magic value for an account', async () => {
const { safeSingleton, sharedSigner, mockAccount, mockVerifier } = await setupTests()
const { safeSingleton, sharedSigner, signerSlot, mockAccount, mockVerifier } = await setupTests()

const data = ethers.toUtf8Bytes('some data to sign')
const dataHash = ethers.keccak256(data)
Expand All @@ -159,7 +203,7 @@ describe('SafeWebAuthnSharedSigner', () => {
})

await mockAccount.givenCalldataReturn(
safeSingleton.interface.encodeFunctionData('getStorageAt', [SIGNER_STORAGE_SLOT, 3]),
safeSingleton.interface.encodeFunctionData('getStorageAt', [signerSlot, 3]),
ethers.AbiCoder.defaultAbiCoder().encode(
['bytes'],
[ethers.AbiCoder.defaultAbiCoder().encode(['uint256', 'uint256', 'uint176'], [x, y, mockVerifier.target])],
Expand Down

0 comments on commit d01c20c

Please sign in to comment.