-
Notifications
You must be signed in to change notification settings - Fork 315
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
[LIVE-3146][LIVE-3626] Feature - EIP712 filtering #1211
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: edb32aa 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 |
943ab74
to
9b0aed3
Compare
Screenshots: ✅
There are no changes in the screenshots for this PR. If this is expected, you are good to go. |
9b0aed3
to
8a6635e
Compare
8a6635e
to
ba8163b
Compare
ba8163b
to
d60c34c
Compare
d60c34c
to
8ef29bd
Compare
8ef29bd
to
364d0cb
Compare
364d0cb
to
3d9fa25
Compare
3d9fa25
to
f07cf30
Compare
Codecov ReportBase: 70.84% // Head: 44.81% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #1211 +/- ##
============================================
- Coverage 70.84% 44.81% -26.04%
============================================
Files 46 637 +591
Lines 3070 26873 +23803
Branches 612 7315 +6703
============================================
+ Hits 2175 12042 +9867
- Misses 892 13682 +12790
- Partials 3 1149 +1146
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
8a10075
to
316873b
Compare
Co-authored-by: Landry Monga <[email protected]>
61d8e66
to
6079fbc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just some suggestion but nothing mandatory.
TBH there's a lot of technical stuff I'm not sure about what it does mean tho but it's more la specific to the concept of EIP712
apps/ledger-live-desktop/src/renderer/modals/SignMessage/steps/StepSummary.jsx
Outdated
Show resolved
Hide resolved
* Add LLM UI parsing to walletconnect msg signing * Add Dynamic CAL EIP712 in hw-app-eth * Update LLD UI for EIP712 dynamic CAL * Update LLM UI for EIP712 dynamic CAL * Move ERC20 dynamic CAL tests into their own folder * Update EIP712 tests for dynamic CAL * changeset * Add typing to dynamic CAL axios call * Fix flashing on EIP721 signing with dynamic CAL
@lambertkevin quel est l'intérêt du "show all message" dans le cas EIP191 ? |
📝 Description
This PR adds the filtering to the EIP712 clear signing. It requires the nano app 1.9.20-EIP712 Ethereum and clones apps to be tested.
❓ Context
ledgerjs/hw-app-eth
,ledgerjs/live-common
,ledger-live-desktop
,ledger-live-mobile
✅ Checklist
📸 Demo
EIP712 with CAL example
Screen.Recording.2022-09-20.at.16.52.18.mov
RPReplay_Final1663771621.mov
EIP712 without CAL example
Screen.Recording.2022-09-20.at.16.56.19.mov
RPReplay_Final1663771523.MP4
EIP191
Screen.Recording.2022-09-20.at.16.57.33.mov
RPReplay_Final1663771707.MP4
🚀 Expectations to reach
Please make sure you follow these Important Steps.
Pull Requests must pass the CI and be internally validated in order to be merged.