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

feat: FIP-8 verifications for contract wallets #1449

Merged
merged 6 commits into from
Oct 7, 2023

Conversation

horsefacts
Copy link
Contributor

@horsefacts horsefacts commented Oct 3, 2023

Motivation

Support verifications signed by smart contracts. See FIP-8 for more details.

Change Summary

Update validations to verify smart contract signatures provided in verification messages using publicClient.verifyTypedData. This function makes an RPC call to verify smart contract signatures, trying EIP-6492, EIP-1271, and ECDSA recovery in order.

Create and pass default public clients to validation functions that may need them.

Merge Checklist

Choose all relevant options below by adding an x now or at any time before submitting for review

Additional Context

Related protocol docs PR: farcasterxyz/protocol#129

Hubble

  • Hub: Create an Optimism publicClient at hub creation time.
  • Storage Engine: Add l2PublicClient. Pass public clients through to validation functions.
  • Storage Engine Worker: Pass serialized RPC config to worker thread, create and pass public clients to validation functions.

Core

  • New: Add eth/clients and eth/chains modules, providing default mainnet/testnet public clients.
  • Validations: Pass provided or default public clients through validations to signature verifier. Validations that do not explicitly provide PublicClients will inherit the defaults.
  • Crypto: Add smart contract signature verification using publicClient.verifyTypedData. This function makes an RPC call to verify smart contract signatures, trying EIP-6492, EIP-1271, and ECDSA recovery in order. Note that we don't use this for EOA recovery to avoid making an unnecessary RPC call. Instead we call the "offline" verifyTypedData util (i.e. the existing EOA signature verification behavior).
  • Protobufs: Update VerificationAddEthAddressBody protobuf to add chainId and verificationType.
  • Signers: Add optional chainId argument to signVerificationEthAddressClaim
  • Factories: Add transient contractSignature parameter to verification factory.

PR-Codex overview

This PR focuses on adding FIP-8 contract verifications to the Farcaster codebase.

Detailed summary

  • Added FIP-8 contract verifications to the eth module
  • Updated the signVerificationEthAddressClaim method in various signers to include the chainId parameter
  • Added defaultPublicClients and defaultL1PublicTestClient to the clients module
  • Updated the VerificationAddEthAddressBodyFactory in the factories module to include the contractSignature parameter
  • Updated the Hub and ValidationWorkerData classes in the hubble module to include support for optimism chains
  • Updated the Message and MessageData protobufs in the schemas module to include verification_type and chain_id fields

The following files were skipped due to too many changes: apps/hubble/src/storage/engine/index.test.ts, packages/hub-web/src/generated/message.ts, packages/hub-nodejs/src/generated/message.ts, packages/core/src/protobufs/generated/message.ts, packages/core/src/crypto/eip712.ts, packages/core/src/validations.ts, apps/hubble/src/storage/engine/index.ts, packages/core/src/validations.test.ts, packages/hub-web/src/generated/rpc.ts

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@changeset-bot
Copy link

changeset-bot bot commented Oct 3, 2023

🦋 Changeset detected

Latest commit: 7ef4797

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@farcaster/hub-nodejs Patch
@farcaster/hub-web Patch
@farcaster/core Patch
@farcaster/hubble Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Oct 3, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hub-monorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 6, 2023 11:45pm

@codecov
Copy link

codecov bot commented Oct 3, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Files Coverage Δ
apps/hubble/src/hubble.ts 52.88% <100.00%> (+0.33%) ⬆️
packages/core/src/eth/chains.ts 100.00% <100.00%> (ø)
packages/core/src/eth/clients.ts 100.00% <100.00%> (ø)
packages/core/src/factories.ts 92.34% <100.00%> (+0.03%) ⬆️
packages/core/src/signers/eip712Signer.ts 100.00% <ø> (ø)
packages/core/src/signers/ethersEip712Signer.ts 94.73% <100.00%> (+0.61%) ⬆️
packages/core/src/signers/ethersV5Eip712Signer.ts 90.00% <100.00%> (+1.11%) ⬆️
packages/core/src/signers/viemLocalEip712Signer.ts 90.00% <100.00%> (+1.11%) ⬆️
packages/core/src/validations.ts 87.50% <100.00%> (+0.40%) ⬆️
apps/hubble/src/storage/engine/index.ts 83.68% <95.83%> (+0.74%) ⬆️
... and 1 more

... and 1 file with indirect coverage changes

📢 Thoughts on this report? Let us know!.

Copy link
Contributor

@adityapk00 adityapk00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some type-related comments, but otherwise looks good to me

apps/hubble/src/storage/engine/index.ts Outdated Show resolved Hide resolved
apps/hubble/src/storage/engine/index.ts Outdated Show resolved Hide resolved
apps/hubble/src/storage/engine/validation.worker.ts Outdated Show resolved Hide resolved
@horsefacts horsefacts force-pushed the horsefacts/1271-verifications branch from d264f4c to a54f653 Compare October 6, 2023 02:11
apps/hubble/src/storage/engine/index.ts Outdated Show resolved Hide resolved
packages/core/src/crypto/eip712.ts Outdated Show resolved Hide resolved
packages/core/src/crypto/eip712.ts Outdated Show resolved Hide resolved
@horsefacts horsefacts merged commit 81e6d8e into main Oct 7, 2023
9 checks passed
@horsefacts horsefacts deleted the horsefacts/1271-verifications branch October 7, 2023 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
t-feat Add a new feature or protocol improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants