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

fix: add parseEip712Transaction util function #2948

Merged
merged 2 commits into from
Nov 17, 2024

Conversation

nikola-bozin-txfusion
Copy link
Contributor

@nikola-bozin-txfusion nikola-bozin-txfusion commented Oct 31, 2024

PR-Codex overview

This PR introduces a new utility function, parseEip712Transaction, to the ZKsync extension, enhancing transaction parsing capabilities for EIP712 serialized transactions. It also updates documentation and adds tests to ensure functionality.

Detailed summary

  • Added parseEip712Transaction function in src/zksync/utils/parseEip712Transaction.ts.
  • Exported parseEip712Transaction from src/zksync/index.ts.
  • Created documentation for parseEip712Transaction in site/pages/zksync/utilities/parseEip712Transaction.md.
  • Added tests for parseEip712Transaction in src/zksync/utils/parseEip712Transaction.test.ts.
  • Updated sidebar in site/sidebar.ts to include link to the new utility.

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

Copy link

changeset-bot bot commented Oct 31, 2024

🦋 Changeset detected

Latest commit: 5fd181c

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

This PR includes changesets to release 1 package
Name Type
viem 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

Copy link

vercel bot commented Oct 31, 2024

@nikola-bozin-txfusion is attempting to deploy a commit to the Wevm Team on Vercel.

A member of the Team first needs to authorize it.

export function parseEip712Transaction(
transaction: Hex,
): ZksyncTransactionSerializableEIP712 {
{
Copy link
Member

Choose a reason for hiding this comment

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

Is the scope required here (L14 + L21)?

Copy link
Contributor

Choose a reason for hiding this comment

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

The scope is removed.

src/zksync/utils/parseEip712Transaction.ts Outdated Show resolved Hide resolved
src/zksync/utils/parseEip712Transaction.ts Outdated Show resolved Hide resolved
throw new BaseError('transaction type must be eip712')
}

return constructEip712Transaction(payload)
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 inline the contents of constructEip712Transaction into here?

Copy link
Contributor

Choose a reason for hiding this comment

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

The constructEip712Transaction method is inlined.

Comment on lines 27 to 30
type PaymasterParams = {
paymaster: Hex
paymasterInput: Hex
}
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to explicitly define this type? The handleArrayToPaymaster return type should be inferred.

Copy link
Contributor

Choose a reason for hiding this comment

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

PaymasterParams is not needed and it has been removed.

src/zksync/utils/parseEip712Transaction.ts Outdated Show resolved Hide resolved
src/zksync/utils/parseEip712Transaction.ts Outdated Show resolved Hide resolved
src/zksync/utils/parseEip712Transaction.ts Outdated Show resolved Hide resolved
@jxom
Copy link
Member

jxom commented Nov 3, 2024

Can we please add an export & docs for this? Also needs a changeset.

@danijelTxFusion danijelTxFusion force-pushed the kiriyaga-txfusion-parse-eip-712 branch 2 times, most recently from 09689f7 to 5f8b1d3 Compare November 9, 2024 18:56
@jxom jxom self-requested a review November 13, 2024 04:14
Copy link
Member

@jxom jxom left a comment

Choose a reason for hiding this comment

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

@danijelTxFusion
Copy link
Contributor

Changeset and docs are added.

@jxom jxom merged commit dbd72a4 into wevm:main Nov 17, 2024
2 of 3 checks passed
@github-actions github-actions bot mentioned this pull request Nov 17, 2024
@danijelTxFusion danijelTxFusion deleted the kiriyaga-txfusion-parse-eip-712 branch November 18, 2024 09:46
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.

3 participants