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

feat(ledger): add utf-8 message signing support #2583

Merged
merged 4 commits into from Sep 6, 2022

Conversation

kyranjamie
Copy link
Collaborator

@kyranjamie kyranjamie commented Jul 21, 2022

Try out this version of the Hiro Wallet - download extension builds.

This PR adds support for UTF-8 message signing for Ledger.

I've refactored many duplicated components, yet retained an approach where each Ledger interaction is composed of generic components, making use of Route state where action-specific textual changes are needed. The current set up allows each Ledger action to either use the generic component if sufficient, or a bespoke component if necessary.

Todo:

@vercel
Copy link

vercel bot commented Jul 21, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
stacks-wallet-web ✅ Ready (Inspect) Visit Preview Sep 6, 2022 at 11:50AM (UTC)

@kyranjamie kyranjamie requested a review from fbwoolf July 21, 2022 14:48
@vercel vercel bot temporarily deployed to Preview July 21, 2022 14:51 Inactive
@vercel vercel bot temporarily deployed to Preview July 21, 2022 14:58 Inactive
@fbwoolf
Copy link
Contributor

fbwoolf commented Jul 21, 2022

Made a first pass through, looks like some good improvements while adding the message signing flow. 👍 I saw you moved the routes in with the feature which is interesting.

@vercel vercel bot temporarily deployed to Preview July 22, 2022 08:28 Inactive
@vercel vercel bot temporarily deployed to Preview July 22, 2022 08:31 Inactive
@vercel vercel bot temporarily deployed to Preview July 22, 2022 08:43 Inactive
@vercel vercel bot temporarily deployed to Preview July 22, 2022 08:51 Inactive
@vercel vercel bot temporarily deployed to Preview July 22, 2022 09:00 Inactive
@vercel vercel bot temporarily deployed to Preview July 27, 2022 10:24 Inactive
@vercel vercel bot temporarily deployed to Preview July 28, 2022 10:59 Inactive
@vercel vercel bot temporarily deployed to Preview July 29, 2022 09:42 Inactive
@vercel vercel bot temporarily deployed to Preview July 29, 2022 11:30 Inactive
@vercel vercel bot temporarily deployed to Preview August 1, 2022 12:32 Inactive
@fbwoolf
Copy link
Contributor

fbwoolf commented Aug 17, 2022

Maybe it never broke the lines on that screen? It does on the details screen before so was thinking it did.

@kyranjamie
Copy link
Collaborator Author

Thanks for the feedback @fbwoolf—sorry totally forgot that we're also waiting on these changes in Stacks.js hirosystems/stacks.js#1329

@kyranjamie kyranjamie linked an issue Aug 23, 2022 that may be closed by this pull request
@markmhendrickson markmhendrickson linked an issue Aug 23, 2022 that may be closed by this pull request
@kyranjamie kyranjamie force-pushed the feat/sign-msgs-on-ledger branch 2 times, most recently from 9908eb2 to f230c9e Compare August 23, 2022 10:39
@vercel vercel bot temporarily deployed to Preview August 23, 2022 10:40 Inactive
@kyranjamie kyranjamie linked an issue Aug 23, 2022 that may be closed by this pull request
@vercel vercel bot temporarily deployed to Preview August 23, 2022 10:56 Inactive
@vercel vercel bot temporarily deployed to Preview August 23, 2022 11:01 Inactive
@vercel vercel bot temporarily deployed to Preview September 5, 2022 08:39 Inactive
@vercel vercel bot temporarily deployed to Preview September 6, 2022 11:33 Inactive
@vercel vercel bot temporarily deployed to Preview September 6, 2022 11:35 Inactive
@vercel vercel bot temporarily deployed to Preview September 6, 2022 11:50 Inactive
@kyranjamie kyranjamie marked this pull request as ready for review September 6, 2022 12:34
@kyranjamie
Copy link
Collaborator Author

Signature with long message is successful, but doesn't close the popup on success.

Fixed, had it not closing for debugging purposes

@kyranjamie
Copy link
Collaborator Author

This is safe to merge, as it won't change existing behaviour

@kyranjamie kyranjamie merged commit 797ce5a into dev Sep 6, 2022
@kyranjamie kyranjamie deleted the feat/sign-msgs-on-ledger branch September 6, 2022 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants