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/arbitrary message signing structured data/i2387 #2405

Merged

Conversation

beguene
Copy link
Contributor

@beguene beguene commented May 6, 2022

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

Add support for signing structured data (ClarityValue)

sd0

sd1

sd3

The web wallet receives the ClarityValue serialized in hex format which is then deserialized to display in the UI in a human readable format. The serialized format is finallly hashed and signed when the user clicks 'Sign'

cc/ @kyranjamie @fbwoolf @beguene @He1DAr

@vercel
Copy link

vercel bot commented May 6, 2022

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

Name Status Preview Updated
stacks-wallet-web ✅ Ready (Inspect) Visit Preview May 16, 2022 at 4:07PM (UTC)

@beguene beguene force-pushed the feat/arbitrary-message-signing-structured-data/I2387 branch from 90f1dda to 789d612 Compare May 6, 2022 12:29
@vercel vercel bot temporarily deployed to Preview May 6, 2022 12:32 Inactive
@beguene beguene force-pushed the feat/arbitrary-message-signing-structured-data/I2387 branch from 789d612 to 2bd6f77 Compare May 6, 2022 12:33
@vercel vercel bot temporarily deployed to Preview May 6, 2022 12:34 Inactive
@beguene beguene requested review from kyranjamie, fbwoolf and landitus and removed request for landitus May 6, 2022 13:56
@landitus
Copy link

landitus commented May 6, 2022

@beguene Is there a way to see these screens with the test app?

@beguene beguene force-pushed the feat/arbitrary-message-signing-structured-data/I2387 branch from 2bd6f77 to 8191fbc Compare May 9, 2022 11:17
@vercel vercel bot temporarily deployed to Preview May 9, 2022 11:19 Inactive
@beguene
Copy link
Contributor Author

beguene commented May 9, 2022

@landitus Were you able to test with https://stacks-wallet-jznmyg9k2-blockstack.vercel.app/ ?

@beguene beguene linked an issue May 9, 2022 that may be closed by this pull request
@landitus
Copy link

landitus commented May 9, 2022

@landitus Were you able to test with https://stacks-wallet-jznmyg9k2-blockstack.vercel.app/ ?

Yes, thank you. Be sure to check the Figma for the text formatting. It doesn't need to be a <pre> tag as the font should be our normal one. HTML wise it could be better as a list <ul><li> with definition lists <dl><dd> for title and description). In the design, the definition is lighter (gray) while the description is in the regular text color (black).

Also, beware there's an extra space at the top of the text content which I'm not sure where it comes from.

CleanShot 2022-05-09 at 14 19 29@2x

@beguene beguene force-pushed the feat/arbitrary-message-signing/I1051 branch 2 times, most recently from 0da8a1e to 124b2cb Compare May 10, 2022 13:34
@timstackblock
Copy link
Contributor

@beguene I fixed this test on dev if you rebase it you should be ok

@beguene
Copy link
Contributor Author

beguene commented May 11, 2022

Thanks a lot @timstackblock I will check it out.

@beguene beguene force-pushed the feat/arbitrary-message-signing/I1051 branch 2 times, most recently from 7c6aca2 to 250b7cb Compare May 11, 2022 07:37
@beguene beguene force-pushed the feat/arbitrary-message-signing-structured-data/I2387 branch from 8191fbc to 54b2bf6 Compare May 11, 2022 11:04
@vercel vercel bot temporarily deployed to Preview May 11, 2022 11:06 Inactive
@beguene
Copy link
Contributor Author

beguene commented May 11, 2022

@landitus I updated the UI with dl dd markup. Let me know if it's fine.
s1
s2

Base automatically changed from feat/arbitrary-message-signing/I1051 to dev May 11, 2022 11:28
@beguene beguene force-pushed the feat/arbitrary-message-signing-structured-data/I2387 branch from 54b2bf6 to 3a99838 Compare May 11, 2022 11:30
@vercel vercel bot temporarily deployed to Preview May 11, 2022 11:32 Inactive
@landitus
Copy link

landitus commented May 11, 2022

@beguene It's looking better now, but it's still difficult to read when there are nested elements. I just peaked at the code and with flexbox it will be quite hard to align the elements correctly. I tested a hierarchy and styling to be as simple as possible in a CodeSandbox, and maybe it can help you this PR: https://codesandbox.io/s/pensive-bas-utyx9m?file=/index.html

Note that this code is very rough, so measures and color values are approximate to illustrate the path I would go for

CleanShot 2022-05-11 at 15 39 41@2x

Copy link

@landitus landitus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added me comments above ☝️

@beguene
Copy link
Contributor Author

beguene commented May 11, 2022

Thanks a lot @landitus, I will try your solution.

@kyranjamie kyranjamie force-pushed the feat/arbitrary-message-signing-structured-data/I2387 branch 2 times, most recently from 17591a2 to baccd18 Compare May 16, 2022 15:10
@vercel vercel bot temporarily deployed to Preview May 16, 2022 15:14 Inactive
@kyranjamie kyranjamie force-pushed the feat/arbitrary-message-signing-structured-data/I2387 branch from baccd18 to ccbcda0 Compare May 16, 2022 15:45
@vercel vercel bot temporarily deployed to Preview May 16, 2022 15:48 Inactive
@kyranjamie kyranjamie force-pushed the feat/arbitrary-message-signing-structured-data/I2387 branch from ccbcda0 to 3ae2d55 Compare May 16, 2022 15:57
@vercel vercel bot temporarily deployed to Preview May 16, 2022 15:59 Inactive
@kyranjamie kyranjamie force-pushed the feat/arbitrary-message-signing-structured-data/I2387 branch from 3ae2d55 to 3b27d7e Compare May 16, 2022 16:04
@vercel vercel bot temporarily deployed to Preview May 16, 2022 16:07 Inactive
Copy link
Collaborator

@kyranjamie kyranjamie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@landitus's comments yet to be addressed, but merging for now to release feature

@kyranjamie kyranjamie dismissed landitus’s stale review May 16, 2022 16:16

feature needs to be released

@kyranjamie kyranjamie merged commit 23d9d9a into dev May 16, 2022
@kyranjamie kyranjamie deleted the feat/arbitrary-message-signing-structured-data/I2387 branch May 16, 2022 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for Structured Data in Arbitrary Message Signing
4 participants