account.signTypedData() for @celo/viem-account-ledger
#505
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
account.signTypedData() for @celo/viem-account-ledger
|
thanks for the contribution. will look at it closely soon! |
3495cad
into
celo-org:nicolasbrugneaux/sign-typed-data
3 checks passed
|
can you comment / post which version on npm this goes out in? Would love to delete my local code and use this instead. Thanks |
nicolasbrugneaux
added a commit
that referenced
this pull request
Jan 24, 2025
> [!NOTE] > Originally opened by @jbrower95 --- ### Description - Needed this for my own work. `viem-account-ledger` was the nicest wrapper for Viem I could find, and everything else is Ethers or lower-level. - Ledger requires the domain seperator to be passed in explicitly, so you need to hash things independently (i.e you cannot use Viem's `hashTypedData()`). ### Tested - Tested this onchain and got valid signatures back. I'm using it in my own work to sign a Gnosis transaction hash, and submit onchain to approve the transaction. I successfully approved a transaction hash using this. - If you guys want to copy this branch and add unit tests that's totally fine. Just wanted to make this easier for ya'll to integrate. <!-- start pr-codex --> --- ## PR-Codex overview This PR primarily focuses on enhancing the `LedgerSigner` and `LedgerWallet` classes by adding an `appName` property to their configurations and implementing logic to handle different ledger applications (Celo and Ethereum) effectively. ### Detailed summary - Added `appName` property to `appConfiguration` in `LedgerSigner`. - Updated constructor to initialize `appName` with a default value. - Implemented validation logic in `signTypedData` and `signTransaction` methods based on `appName`. - Created `readAppName` function to fetch the app name from the ledger device. - Introduced tests for `readAppName` and various transaction signing scenarios. - Updated documentation to include new `test-utils` module and its contents. > The following files were skipped due to too many changes: `packages/viem-account-ledger/src/ledger-to-account.test.ts` > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}` <!-- end pr-codex --> ### Description _Tell us why these changes are being made_ #### Other changes _Describe any minor or "drive-by" changes here. If none delete this section_ ### Tested _An explanation of how the changes were tested or an explanation as to why they don't need to be._ ### How to QA _List of steps required to QA the change if applicable_ ### Related issues - Fixes #[issue number here] <!-- start pr-codex --> --- ## PR-Codex overview This PR focuses on enhancing the `ledgerToAccount` functionality by implementing the `signTypedData` method, which allows for signing EIP-712 typed data using the Ledger device. It also adds new utility functions for handling EIP-712 domains and structures. ### Detailed summary - Added imports for `getTypesForEIP712Domain`, `hashDomain`, `hashStruct`, and `HashTypedDataParameters` from `viem`. - Introduced `MessageTypeProperty` interface for type safety. - Implemented the `signTypedData` method to sign EIP-712 typed data. - Utilized `hashDomain` and `hashStruct` for message hashing. - Updated the return statement to serialize the signature. > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}` --------- Co-authored-by: Jay <jbrower95@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
viem-account-ledgerwas the nicest wrapper for Viem I could find, and everything else is Ethers or lower-level.hashTypedData()).Tested
PR-Codex overview
This PR enhances the
ledgerToAccountfunctionality by implementing thesignTypedDatamethod for EIP-712 typed data signing, incorporating new utility functions fromviemand defining an interface for message properties.Detailed summary
getTypesForEIP712Domain,hashDomain,hashStruct,HashTypedDataParametersfromviem.MessageTypeProperty.signTypedDatamethod to handle EIP-712 signing.hashDomainandhashStructto compute hashes for the signing process.serializeSignatureto format the signature before returning.