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

Add Custom Chain Serializers #691

Merged
merged 12 commits into from
Jun 20, 2023
Merged

Conversation

aaronmgdr
Copy link
Contributor

@aaronmgdr aaronmgdr commented Jun 9, 2023

TLDR

Allow account.signTransaction to take an optional transactionSerializer argument.

##Why?

Through custom chain formatters viem supports alternative standards. for formatting transactions but not yet for signing them

How

  1. add to signTransaction an optional argument for a serializer which

  2. add an optional serializer property to chain.

  3. if the current chain has a serializer pass as argument to signTransaction function.

It is recommended that custom serializers check if the transaction is the type they support and if not call the default viem serializeTransaction function.

Example

// create a customer Serializer ie

import { SerializeTransactionFn, serializeTransaction } from 'viem/utils'
import { TransactionSerializable } from 'viem/types'

type CustomTX = TransactionSerializable {
  type: 'pip112' // a string representing the tx type this should serialize to a EIP2718 compliant TransactionType https://eips.ethereum.org/EIPS/eip-2718#transactiontype-only-goes-up-to-0x7f
  extraField: // some field the spec for this tx type supports
}

export const serializer: SerializeTransactionFn<CustomTX> = (transaction) => {
  if (specialCase(transaction)) {
    // serialization code here
  } else {
    return serializeTransaction(transaction)
  }
}

// Add it to the chain

import {serializer } from './serializer'
import {celo} from 'wagmi/chains'

const chainWithSerializer = defineChain({...celo, serializer})

// use chain when creating client
const client = createWalletClient({
  account,
  chain: chainWithSerializer,
  transport: http()
})

other ways to achieve this

technically its possible to add an custom serializer by creating an account using the toAccount function.

This has 2 limitations.

  1. you must recreated the privateKeyToAccount, signTransaction just to modify the serializeTransaction function. Since mneumonicToAccount and HDKeyToAccount use privateKeyToAccount they must also be recreated.

  2. it doesnt give an elegent way for there to be multiple serializers such as different chains both having custom serializers.

related PRS

supersedes #666


PR-Codex overview

This PR adds support for custom chain serializers and EIP-2930 access lists. It also makes changes to several files, including utils, types, accounts, actions, and transaction.

Detailed summary

  • Added custom chain serializers via chain.serializers
  • Added support for EIP-2930 access lists
  • Removed serializeTransaction from some utils files
  • Modified signTransaction in accounts/utils/signTransaction.ts
  • Modified serializeTransaction in utils/transaction/serializeTransaction.ts
  • Modified Account type in accounts/types.ts
  • Modified Chain type in types/chain.ts
  • Added tests for serializeAccessList and signTransaction

The following files were skipped due to too many changes: src/accounts/utils/signTransaction.test.ts, src/utils/transaction/serializeTransaction.ts, src/actions/wallet/sendTransaction.test.ts, site/docs/accounts/custom.md

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

@changeset-bot
Copy link

changeset-bot bot commented Jun 9, 2023

🦋 Changeset detected

Latest commit: fb23b42

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

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 Jun 9, 2023

Someone is attempting to deploy a commit to the wagmi Team on Vercel.

A member of the Team first needs to authorize it.

@aaronmgdr aaronmgdr force-pushed the aaron/custom-chain-serializers branch from a272748 to a2359ae Compare June 13, 2023 18:18
@aaronmgdr aaronmgdr changed the title PoC - custom chain serializers Custom Chain Serializers Jun 13, 2023
@aaronmgdr
Copy link
Contributor Author

aaronmgdr commented Jun 13, 2023

EDIT: fixed by #709


One issue Im running into is that the TS types for the return values of functions are very specific only allowing for specific strings.

for instance

 type TransactionSerialized<TType extends TransactionType = 'legacy'> =
  | (TType extends 'eip1559' ? TransactionSerializedEIP1559 : never)
  | (TType extends 'eip2930' ? TransactionSerializedEIP2930 : never)
  | (TType extends 'legacy' ? TransactionSerializedLegacy : never)

and

export type GetTransactionType<
  TTransactionSerializable extends TransactionSerializable = TransactionSerializable,
> =
  | (TTransactionSerializable['maxFeePerGas'] extends bigint
      ? 'eip1559'
      : never)
  | (TTransactionSerializable['maxPriorityFeePerGas'] extends bigint
      ? 'eip1559'
      : never)
  | (TTransactionSerializable['gasPrice'] extends bigint
      ? TTransactionSerializable['accessList'] extends AccessList
        ? 'eip2930'
        : 'legacy'
      : never)
  | (TTransactionSerializable['type'] extends string
      ? TTransactionSerializable['type']
      : never)
      

example

Captura de pantalla 2023-06-13 a la(s) 3 39 57 p m

It seems like the solution will be for TransactionType to be something other than "eip1559" | "eip2930" | "legacy" however i don't have a nice way to so so in mind yet. maybe its just adding 'eip2718' to mean tx is a kind of eip2718 tx but that doesnt seems right.

@aaronmgdr aaronmgdr force-pushed the aaron/custom-chain-serializers branch from a2359ae to 58529aa Compare June 14, 2023 18:03
@aaronmgdr aaronmgdr marked this pull request as ready for review June 15, 2023 19:48
@aaronmgdr
Copy link
Contributor Author

ive writen up what I hope to be a doc on how to use this but Im not sure where in the docs it should go


Using custom transaction serializer

For Local Accounts account.signTransaction can take an optional second argument to serialize the transaction object before signing. If you are using walletClient.sendTransaction with a chain that has a serializer defined it will automatically be used to sign the transaction.

::: code-group

import { account, walletClient } from './config'
import { serializer } from './serializer'

const signedTransaction = await account.signTransaction(transaction, serializer)
import { SerializeTransactionFn, serializeTransaction } from 'viem/utils'
import { TransactionSerializable } from 'viem/types'

type CustomTX = TransactionSerializable {
  type: 'pip112' // a string representing the tx type this should serialize to a EIP2718 compliant TransactionType https://eips.ethereum.org/EIPS/eip-2718#transactiontype-only-goes-up-to-0x7f
  extraField: // some field the spec for this tx type supports
}

export const serializer: SerializeTransactionFn<CustomTX> = (transaction) => {
  if (specialCase(transaction)) {
    // serialization code here
  } else {
    return serializeTransaction(transaction)
  }
}

@aaronmgdr aaronmgdr changed the title Custom Chain Serializers Add Custom Chain Serializers Jun 16, 2023
@pr-codex
Copy link

pr-codex bot commented Jun 16, 2023

show me changes made by this pr

@aaronmgdr I'm sorry, I cannot determine the PR number or link from the code diff provided. Please provide the PR number or link so that I can show you the changes made by the PR.

@aaronmgdr
Copy link
Contributor Author

/codex summarize PR #691

@pr-codex
Copy link

pr-codex bot commented Jun 16, 2023

summarize PR #691

@aaronmgdr PR #691 adds support for EIP-2930 access lists to the serializeTransaction utility function in the utils/transaction module. It also adds the ability for chains to specify their own transaction serializer and exports serializeAccessList and type SerializeTransactionFn. The signTransaction function in accounts/utils/signTransaction is modified to accept a serializer argument. Tests are added for the new functionality.

@vercel
Copy link

vercel bot commented Jun 16, 2023

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

Name Status Preview Updated (UTC)
viem ✅ Ready (Inspect) Visit Preview Jun 16, 2023 4:38pm

@codecov
Copy link

codecov bot commented Jun 19, 2023

Codecov Report

Merging #691 (c7fb925) into main (b510233) will decrease coverage by 4.79%.
The diff coverage is 100.00%.

❗ Current head c7fb925 differs from pull request most recent head fb23b42. Consider uploading reports for the commit fb23b42 to get more accurate results

@@            Coverage Diff             @@
##             main     #691      +/-   ##
==========================================
- Coverage   99.80%   95.02%   -4.79%     
==========================================
  Files         250      247       -3     
  Lines       23841    23660     -181     
  Branches     2055     1748     -307     
==========================================
- Hits        23795    22482    -1313     
- Misses         43     1167    +1124     
- Partials        3       11       +8     
Impacted Files Coverage Δ
src/accounts/privateKeyToAccount.ts 100.00% <100.00%> (ø)
src/accounts/utils/signTransaction.ts 100.00% <100.00%> (ø)
src/actions/wallet/sendTransaction.ts 100.00% <100.00%> (ø)
src/index.ts 100.00% <100.00%> (ø)
src/utils/index.ts 100.00% <100.00%> (ø)
src/utils/transaction/serializeAccessList.ts 100.00% <100.00%> (ø)
src/utils/transaction/serializeTransaction.ts 100.00% <100.00%> (ø)

... and 51 files with indirect coverage changes

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.

2 participants