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

Reject signing message larger than hw device size limit #5288

Conversation

oskarleonard
Copy link
Contributor

@oskarleonard oskarleonard commented Sep 1, 2023

What was the problem?

This PR resolves #5218

How was it solved?

Added error state when signing a message with hw wallet (to large message, disconnect hw, etc..)

How was it tested?

  1. git pull, yarn build, yarn dev, LISK_DESKTOP_URL="http://localhost:8080" DEBUG=true yarn run start
  2. Connect HW and open Lisk App
  3. Go to this page and copy all the text (sdfsdf...). This is too large for the hw wallet so we should see an error if when we try to sign with it.
  4. Click Sign message and paste all the text, wait 5 sec.
  5. Click continue
  6. Expected: An error view should be shown telling you the message was to large (Transaction rejected)
  7. Click back arrow in top left
  8. Expected: You should be navigated back to where you can type in a new message
  9. Make sure HW is connected and Lisk App is open
  10. Paste this as message: "A short test"
  11. Click Continue and sign with hw wallet
  12. Expected: All should be fine

Sign wallet connect transaction on ledger device

  1. Modify these values in our wallet connect dapp (to be the values of your current account in lisk desktop)
image
  1. Connect HW and open Lisk App

  2. Initiate a sign_transaction from wallet connect

  3. Continue the action in lisk desktop

  4. Expected: Should be able to send the transaction

@ManuGowda ManuGowda changed the base branch from development to release/3.0.0 September 1, 2023 15:01
@ManuGowda ManuGowda requested review from ikem-legend and ManuGowda and removed request for ManuGowda and ikem-legend September 4, 2023 10:17
Copy link
Contributor

@ManuGowda ManuGowda left a comment

Choose a reason for hiding this comment

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

Currently the error handling is only done for sign message? we also need to handle this for

  • Sign transaction using wallet connect? when an external application tries to sign a transaction/message then we will have memory issue
  • Transaction signing also needs to handle this error, i.e transaction signature collector i guess?

@ManuGowda ManuGowda removed the request for review from eniolam1000752 September 6, 2023 07:49
@ManuGowda ManuGowda merged commit b41a19c into release/3.0.0 Sep 6, 2023
6 checks passed
@ManuGowda ManuGowda deleted the 5218-reject-signing-message-larger-than-hw-device-size-limit branch September 6, 2023 07:49
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.

Reject signing messages larger than HW device size limit
2 participants