Skip to content

Commit

Permalink
More explicit when skipping validation
Browse files Browse the repository at this point in the history
  • Loading branch information
ScreamingHawk committed Dec 5, 2024
1 parent e8cbccd commit 260bacb
Show file tree
Hide file tree
Showing 7 changed files with 38 additions and 24 deletions.
5 changes: 2 additions & 3 deletions packages/account/src/account.ts
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ export class Account {
// Apply ERC-6492 to undeployed children
const signature = await this.signDigest(digest, 0, false, 'ignore', {chainId, referenceChainId, cantValidateBehavior: "eip6492"})
const decoded = this.coders.signature.decode(signature)
const recovered = await this.coders.signature.recover(decoded, { digest, chainId, address: this.address })
const recovered = await this.coders.signature.recover(decoded, { digest, chainId, address: this.address }, undefined, 'ignore')
const signatures = this.coders.signature.signaturesOf(recovered.config)
const signaturesWithReferenceChainId = signatures.map(s => ({...s, referenceChainId}))
return this.tracker.saveWitnesses({ wallet: this.address, digest, chainId, signatures: signaturesWithReferenceChainId })
Expand Down Expand Up @@ -669,8 +669,7 @@ export class Account {
await this.tracker.savePresignedConfiguration({
wallet: this.address,
nextConfig: config,
signature,
validateBehavior: 'ignore'
signature
})

// safety check, tracker should have a reverse lookup for the imageHash
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/commons/signature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export interface SignatureCoder<

trim: (data: string) => Promise<string>

recover: (data: Z, payload: SignedPayload, provider?: ethers.Provider) => Promise<T>
recover: (data: Z, payload: SignedPayload, provider?: ethers.Provider, validateBehavior?: 'ignore' | 'throw') => Promise<T>

supportsNoChainId: boolean

Expand Down
5 changes: 1 addition & 4 deletions packages/core/src/commons/signer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export function isValidSignature(
address: string,
digest: ethers.BytesLike,
signature: ethers.BytesLike,
provider?: ethers.Provider
provider: ethers.Provider
) {
const bytes = ethers.getBytes(signature)

Expand All @@ -55,9 +55,6 @@ export function isValidSignature(
}

if (type === SigType.WALLET_BYTES32) {
if (!provider) {
throw new Error('Provider is required to validate EIP1271 signatures')
}
return isValidEIP1271Signature(address, ethers.hexlify(digest), bytes.slice(0, -1), provider)
}

Expand Down
23 changes: 18 additions & 5 deletions packages/core/src/v1/signature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,8 @@ export function encodeSignature(signature: Signature | UnrecoveredSignature | et
export async function recoverSignature(
data: UnrecoveredSignature,
payload: base.SignedPayload,
provider?: ethers.Provider
provider?: ethers.Provider,
validateBehavior: 'ignore' | 'throw' = 'throw'
): Promise<Signature> {
const subdigest = base.subdigestOf(payload)
const signers = await Promise.all(
Expand All @@ -136,8 +137,15 @@ export async function recoverSignature(

if (s.isDynamic) {
if (!s.address) throw new Error('Dynamic signature part must have address')
if (!isValidSignature(s.address, subdigest, s.signature, provider)) {
throw new Error(`Invalid dynamic signature part ${s.address}`)

if (validateBehavior !== 'ignore') {
if (!provider) {
throw new Error('Provider is required to validate EIP1271 signatures')
}

if (!isValidSignature(s.address, subdigest, s.signature, provider)) {
throw new Error(`Invalid dynamic signature part ${s.address}`)
}
}

return { address: s.address, weight: s.weight, signature: s.signature }
Expand Down Expand Up @@ -216,8 +224,13 @@ export const SignatureCoder: base.SignatureCoder<WalletConfig, Signature, Unreco

supportsNoChainId: true,

recover: (data: UnrecoveredSignature, payload: base.SignedPayload, provider: ethers.Provider): Promise<Signature> => {
return recoverSignature(data, payload, provider)
recover: (
data: UnrecoveredSignature,
payload: base.SignedPayload,
provider?: ethers.Provider,
validateBehavior: 'ignore' | 'throw' = 'throw'
): Promise<Signature> => {
return recoverSignature(data, payload, provider, validateBehavior)
},

encodeSigners: (
Expand Down
22 changes: 14 additions & 8 deletions packages/core/src/v2/signature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,11 +223,11 @@ export class InvalidSignatureLeafError extends Error {
}
}

// Signature validity is only checked if provider is provided
export async function recoverTopology(
unrecovered: UnrecoveredTopology,
subdigest: string,
provider?: ethers.Provider
provider?: ethers.Provider,
validateBehavior: 'ignore' | 'throw' = 'throw'
): Promise<Topology> {
if (isUnrecoveredNode(unrecovered)) {
const [left, right] = await Promise.all([
Expand All @@ -252,7 +252,11 @@ export async function recoverTopology(
throw new Error('Dynamic signature leaf without address')
}

if (provider) {
if (validateBehavior !== 'ignore') {
if (!provider) {
throw new Error('Provider is required to validate EIP1271 signatures')
}

const isValid = await isValidSignature(unrecovered.address, subdigest, unrecovered.signature, provider)
if (!isValid) {
throw new InvalidSignatureLeafError(unrecovered)
Expand Down Expand Up @@ -601,7 +605,8 @@ export function setImageHashStruct(imageHash: string) {
export async function recoverSignature(
signature: UnrecoveredSignature | UnrecoveredChainedSignature,
payload: base.SignedPayload | { subdigest: string },
provider?: ethers.Provider
provider?: ethers.Provider,
validateBehavior: 'ignore' | 'throw' = 'throw'
): Promise<Signature | ChainedSignature> {
const signedPayload = (payload as { subdigest: string }).subdigest === undefined ? (payload as base.SignedPayload) : undefined

Expand All @@ -613,7 +618,7 @@ export async function recoverSignature(
const subdigest = signedPayload ? base.subdigestOf(signedPayload) : (payload as { subdigest: string }).subdigest

if (!isUnrecoveredChainedSignature(signature)) {
const tree = await recoverTopology(signature.decoded.tree, subdigest, provider)
const tree = await recoverTopology(signature.decoded.tree, subdigest, provider, validateBehavior)
return { version: 2, type: signature.type, subdigest, config: { version: 2, ...signature.decoded, tree } }
}

Expand All @@ -628,7 +633,7 @@ export async function recoverSignature(
// NOTICE: Remove the suffix from the "first" siganture
// otherwise we recurse infinitely
for (const sig of [{ ...signature, suffix: undefined }, ...signature.suffix]) {
const recovered = await recoverSignature(sig, mutatedPayload, provider)
const recovered = await recoverSignature(sig, mutatedPayload, provider, validateBehavior)
result.unshift(recovered)

const nextMessage = setImageHashStruct(imageHash(deepestConfigOfSignature(recovered)))
Expand Down Expand Up @@ -930,9 +935,10 @@ export const SignatureCoder: base.SignatureCoder<WalletConfig, Signature, Unreco
recover: (
data: UnrecoveredSignature | UnrecoveredChainedSignature,
payload: base.SignedPayload,
provider?: ethers.Provider
provider?: ethers.Provider,
validateBehavior: 'ignore' | 'throw' = 'throw'
): Promise<Signature> => {
return recoverSignature(data, payload, provider)
return recoverSignature(data, payload, provider, validateBehavior)
},

encodeSigners: (
Expand Down
1 change: 0 additions & 1 deletion packages/sessions/src/tracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ export type PresignedConfig = {
nextConfig: commons.config.Config
signature: string
referenceChainId?: string
validateBehavior?: 'ignore' | 'throw'
}

export type PresignedConfigLink = Omit<PresignedConfig, 'nextConfig'> & { nextImageHash: string }
Expand Down
4 changes: 2 additions & 2 deletions packages/sessions/src/trackers/local.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,11 @@ export class LocalConfigTracker implements ConfigTracker, migrator.PresignedMigr
const savePayload = this.savePayload({ payload })
const saveNextConfig = this.saveWalletConfig({ config: args.nextConfig })

const validateBehavior = args.validateBehavior ?? 'throw'
const recovered = await v2.signature.SignatureCoder.recover(
decoded,
payload,
validateBehavior === 'ignore' ? undefined : this.provider // Only validate if we are not ignoring
this.provider,
'ignore'
)

// Save the recovered configuration and all signature parts
Expand Down

0 comments on commit 260bacb

Please sign in to comment.