account.signTypedData() for @celo/viem-account-ledger (#505)

[nicolasbrugneaux]

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.

---

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}

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]

---

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}

[changeset-bot]

⚠️ No Changeset found

Latest commit: 32f514f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[codecov]

Codecov Report

Attention: Patch coverage is 88.57143% with 24 lines in your changes missing coverage. Please review.

Project coverage is 76.58%. Comparing base (be62336) to head (32f514f).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #513      +/-   ##
==========================================
+ Coverage   76.45%   76.58%   +0.13%     
==========================================
  Files         247      248       +1     
  Lines       10448    10639     +191     
  Branches     1649     1699      +50     
==========================================
+ Hits         7988     8148     +160     
- Misses       2369     2393      +24     
- Partials       91       98       +7     

Components	Coverage Δ
celocli	86.77% <ø> (ø)
sdk	68.96% <ø> (ø)
wallets	74.03% <84.84%> (+0.72%)	⬆️
viem-sdks	93.54% <94.87%> (-1.54%)	⬇️

[nicolasbrugneaux]

Hi @jbrower95 thanks again for your work!

After some investigation, I came to the conclusion that unfortunately celo-app still cannot sign typed data

Explanation:

In node_modules/@celo/hw-app-eth/lib/modules/EIP712/index.js line 455

return transport.send(0xe0, 0x0c, 0x00, 0x00, buffer)

the second argument is the hex code that the app will receive and decide what to do with it, for signing it's 0x04, signPersonal it's 0x08 and for signTypedData it's 0x0c.

Unfortunately you can find in https://github.com/LedgerHQ/app-celo-spender/blob/develop/src/main.c#L600-L625 that the celo-app does not implement a signEIP712HashedMessage or signEIP712Message handler and thus returns invalid data. (In our case a constant <Buffer 02 90 00>

---

TLDR: it's not implemented in the C application

So it arises the following question, as you seem to need it for your work, how did you make it work on your ledger device?

I see that running the tests on the Ethereum app instead of Celo results in a successful signed hash, but I don't think we officially support that at the moment. I'm adding some more defensive coding to handle celo vs ethereum app better and more transparently

[nicolasbrugneaux]

I added support for ethereum app for eip1559 txs and signTypedData, I still kept the error throwing for signTypedData on the celo app though.