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

Desktop UI for Solana signAndSendTransaction, signTransaction, and signAllTransactions provider APIs #13666

Merged
merged 30 commits into from
Jun 21, 2022

Conversation

josheleonard
Copy link
Contributor

@josheleonard josheleonard commented Jun 7, 2022

Resolves brave/brave-browser#23369
Resolves brave/brave-browser#23434

Security review request: https://github.com/brave/security/issues/910

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

  1. Create a new Wallet, select Solana Devnet and then create a Solana Account if needed.
  2. Go to https://solfaucet.com/ , paste your Solana address and click Devnet to get some Devnet Solana Tokens
  3. Go back to the Brave Wallet and make sure you have received your Devnet tokens in your wallet.
  4. Ask @Douglashdaniel for a shared test wallet or message @yrliou to send you some SPL tokens on the Solana Devnet (you will need to add this token as a custom token in the Edit Visible Asset modal.)
  5. Go to https://pwgoom.csb.app/ and click Connect, connect your wallet!

Testing - Sign and Send Transaction

  1. Click Sign and Send Transaction.

Screen Shot 2022-06-16 at 11 08 29 PM

  1. Check and make sure Transaction info is displayed.

Screen Shot 2022-06-16 at 10 37 46 PM

  1. Click Details and make sure transaction Details are displayed.

Screen Shot 2022-06-16 at 10 38 04 PM

  1. Click Confirm and make sure you can see the Transaction Details

Screen Shot 2022-06-16 at 10 41 50 PM

Testing - Sign and Send Transaction (Request)

  1. Click on Sign and Send Transaction (Request)

Screen Shot 2022-06-16 at 11 08 40 PM

  1. Follow the same instructions for Testing - Sign and Send Transaction from 2-4

Testing - Sign and Send SPL Token Transaction

  1. Click Sign and Send SPL Token Transaction.

Screen Shot 2022-06-16 at 11 11 16 PM

  1. Check and make sure Transaction info is displayed.

Screen Shot 2022-06-17 at 9 56 46 AM

  1. Click Details and make sure transaction Details are displayed.

Screen Shot 2022-06-17 at 9 57 00 AM

  1. Click Confirm and make sure you can see the Transaction Details

Screen Shot 2022-06-17 at 9 57 14 AM

Testing - Sign Transaction

  1. Click Sign Transaction.

Screen Shot 2022-06-16 at 11 30 05 PM

  1. Review Sign at your own risk warning and click Continue

Screen Shot 2022-06-16 at 10 55 59 PM

  1. Review Details and click Sign

Screen Shot 2022-06-16 at 10 56 18 PM

Testing - Sign Transaction (Request)

  1. Click on Sign Transaction (Request)

Screen Shot 2022-06-16 at 11 31 00 PM

  1. Follow the same instructions for Sign Transaction from 2-3

Testing - Sign All Transaction (multiple)

  1. Click on Sign All Transaction (multiple)

Screen Shot 2022-06-16 at 11 31 44 PM

  1. Review Sign at your own risk warning and click Continue

Screen Shot 2022-06-16 at 10 55 59 PM

  1. Review Details and make sure that there are multiple transactions, then click Sign

Screen Shot 2022-06-16 at 11 03 15 PM

Testing - Sign All Transaction (multiple) (Request)

  1. Click on Sign All Transaction (multiple) (Request)

Screen Shot 2022-06-16 at 11 32 17 PM

  1. Follow the same instructions for Sign All Transaction (multiple) from 2-3

Testing - Sign All Transaction (single)

  1. Click on Sign All Transaction (single)

Screen Shot 2022-06-16 at 11 32 46 PM

  1. Review Sign at your own risk warning and click Continue

Screen Shot 2022-06-16 at 11 25 33 PM

  1. Review Details and make sure that there is a single transaction, then click Sign

Screen Shot 2022-06-16 at 11 25 45 PM

Testing - Sign All Transaction (single) (Request)

  1. Click on Sign All Transaction (single) (Request)

Screen Shot 2022-06-16 at 11 33 28 PM

  1. Follow the same instructions for Sign All Transaction (single) from 2-3

@josheleonard josheleonard self-assigned this Jun 7, 2022
@josheleonard josheleonard force-pushed the feat-solana-desktop-dapp-support-ui branch 3 times, most recently from e82eb03 to 8ded8af Compare June 9, 2022 01:07
@josheleonard josheleonard marked this pull request as ready for review June 9, 2022 01:31
@josheleonard josheleonard requested a review from a team as a code owner June 9, 2022 01:31
@github-actions github-actions bot added the rebase label Jun 9, 2022
@josheleonard josheleonard requested review from onyb and yrliou June 9, 2022 01:32
@github-actions github-actions bot removed the rebase label Jun 9, 2022
@josheleonard josheleonard force-pushed the feat-solana-desktop-dapp-support-ui branch 5 times, most recently from 0cfe0f1 to 0223458 Compare June 9, 2022 22:15
@josheleonard josheleonard force-pushed the feat-solana-desktop-dapp-support-ui branch 2 times, most recently from ba1c23a to 064e144 Compare June 11, 2022 00:19
Copy link
Contributor

@Douglashdaniel Douglashdaniel left a comment

Choose a reason for hiding this comment

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

Should these panels show Solana Testnet when doing a Testnet transaction?

Screen.Recording.2022-06-12.at.11.00.38.PM.mov

@Douglashdaniel Douglashdaniel force-pushed the feat-solana-desktop-dapp-support-ui branch from 064e144 to c041351 Compare June 14, 2022 00:45
@Douglashdaniel Douglashdaniel force-pushed the feat-solana-desktop-dapp-support-ui branch 2 times, most recently from 1e1b165 to afd5f8c Compare June 14, 2022 20:21
@yrliou
Copy link
Member

yrliou commented Jun 14, 2022

For non-system instructions which we don't support decode yet, currently it looks like this: (it's a SPL token transfer in this case)
Screen Shot 2022-06-14 at 2 59 46 PM

I think it's probably better if we can display more details, like showing raw data arrays and account lists.

@Douglashdaniel Douglashdaniel force-pushed the feat-solana-desktop-dapp-support-ui branch 2 times, most recently from ea24224 to e2caa3f Compare June 15, 2022 06:27
@Douglashdaniel Douglashdaniel force-pushed the feat-solana-desktop-dapp-support-ui branch from 11b8afe to f0e740a Compare June 21, 2022 17:28
Copy link
Contributor Author

@josheleonard josheleonard left a comment

Choose a reason for hiding this comment

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

Approved @Douglashdaniel's changes

@srirambv
Copy link
Contributor

Verification passed on

Brave 1.42.24 Chromium: 103.0.5060.53 (Official Build) nightly (64-bit)
Revision a1711811edd74ff1cf2150f36ffa3b0dae40b17f-refs/branch-heads/5060@{#853}
OS Windows 11 Version 21H2 (Build 22000.739)
13666.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants