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.

Sign up for GitHub

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

Jump to bottom

[LW-9482] Fix/cip30 sign data #807

Merged
merged 4 commits into from Jan 4, 2024
Merged

[LW-9482] Fix/cip30 sign data #807

merged 4 commits into from Jan 4, 2024

Conversation

mchappell
Copy link
Collaborator

@mchappell mchappell commented Jan 2, 2024 •
edited by rhyslbw

Checklist

  • JIRA - <link>
  • Proper tests implemented
  • Screenshots added.

Proposed solution

signBlob method from keyAgent is now also wrapped, so that the auto-redirect to sign success state can be removed, this was prematurely removing password from memory which caused the blob not to sign

Testing

Execute at example.com in console

const api = await window.cardano.lace.enable();
const ownAddress = await api.getUsedAddresses();
const signedData = await api.signData(
        ownAddress[0],
        '66697865642074686520627567'
      );

Screenshots

image
image

Fixes #803

@mchappell mchappell requested a review from a team as a code owner January 2, 2024 17:53
@github-actions github-actions bot added the browser Changes to the browser application. label Jan 2, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 2, 2024 •
edited

Allure report

allure-report-publisher generated test report!

smokeTests: ✅ test report for 72bf4d60

passed failed skipped flaky total result
Total 31 0 0 0 31 ✅

@coveralls
Copy link

coveralls commented Jan 2, 2024 •
edited

Coverage Status

coverage: 53.169% (-0.001%) from 53.17%
when pulling 72bf4d6 on fix/cip30-sign-data
into 737635e on main.

pczeglik-iohk
pczeglik-iohk approved these changes Jan 3, 2024
View reviewed changes
Copy link
Contributor

@pczeglik-iohk pczeglik-iohk left a comment

Choose a reason for hiding this comment

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

I spot one small think, but it's not a deal breaker :)

@mchappell mchappell requested a review from a team as a code owner January 3, 2024 16:06
@github-actions github-actions bot added the e2e Changes to the e2e testing instrumentation. label Jan 3, 2024
@ljagiela ljagiela changed the title Fix/cip30 sign data [LW-9482] Fix/cip30 sign data Jan 3, 2024
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Jan 4, 2024

Quality Gate Failed Quality Gate failed

Failed conditions

C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

idea Catch issues before they fail your Quality Gate with our IDE extension SonarLint SonarLint

@mchappell mchappell merged commit 31f542c into main Jan 4, 2024
7 of 9 checks passed
@mchappell mchappell deleted the fix/cip30-sign-data branch January 4, 2024 14:19
mchappell added a commit that referenced this pull request Jan 5, 2024
@mchappell @ljagiela
fix: [LW-9482] cip30 sign data (#807)
53cb503 
* fix(extension): add wrapped callback for signBlob

this allows a callback to be executed on success after signing data

* fix: use correct method for hardware wallet data signing

* refactor: rename callback method

* test(extension): add e2e test / add support for console logs collection

---------

Co-authored-by: Lukasz Jagiela <Lukasz.Jagiela@iohk.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pczeglik-iohk pczeglik-iohk pczeglik-iohk approved these changes

@danielmain danielmain danielmain approved these changes

@lucas-barros lucas-barros lucas-barros approved these changes

@ljagiela ljagiela ljagiela approved these changes

@wklos-iohk wklos-iohk wklos-iohk approved these changes

Assignees
No one assigned
Labels
browser Changes to the browser application. e2e Changes to the e2e testing instrumentation.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cannot sign message to get signature
7 participants
@mchappell @coveralls @danielmain @lucas-barros @ljagiela @pczeglik-iohk @wklos-iohk