Override signMessage for SmartWallet class #1953
Conversation
🦋 Changeset detectedLatest commit: 6295cf7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
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 |
|
/release-pr |
| throw new Error("Failed to sign message"); | ||
| } | ||
|
|
||
| const isValid = await checkContractWalletSignature( |
There was a problem hiding this comment.
I don't think it makes sense to validate the signature here, will also slow down the process quite a bit
There was a problem hiding this comment.
I don't think there's another way to check whether to fallback to legacy method of signing.
So this seems necessary to maintain backward compatibility with existing smart wallets out there.
|
/release-pr |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1953 +/- ##
==========================================
- Coverage 67.94% 67.94% -0.01%
==========================================
Files 291 291
Lines 10928 10927 -1
Branches 1495 1495
==========================================
- Hits 7425 7424 -1
Misses 2882 2882
Partials 621 621 ☔ View full report in Codecov by Sentry. |
* add label view/edit support for admins and access tokens * add labels for backend wallets
Problem solved
Uses EIP-712 signature in
signMessagefor the updated EIP-1271 signature verification in smart wallet contracts (thirdweb-dev/contracts#572)Changes made
SmartWallet.signMessagedirectly uses ethers-jssigner.signMessageto sign the message hash of the data to sign (i.e.signMessage(ethers.utils.hashMessage(data)))SmartWallet.signMessagesignes EIP-712 typed data of the data to sign mixed with the smart wallet's domain separator. If this yields in invalid signature, the function falls back to the legacy method of signing.How to test