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

Getting Wallet Address #17

Merged
merged 10 commits into from
Mar 5, 2024
Merged

Getting Wallet Address #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6009faa
feat: initial working setup for retrieving wallet address
rabi-siddique Mar 5, 2024
5590696
chore:code cleanup
rabi-siddique Mar 5, 2024
9567506
feat: interaction to switch wallet
frazarshad Mar 5, 2024
682e71d
chore: simplifying switching screens in import wallet flow
rabi-siddique Mar 5, 2024
9ab2db4
chore format code with prettier
rabi-siddique Mar 5, 2024
9ff28d3
chore: moving get wallet address test case in the main context
rabi-siddique Mar 5, 2024
1638199
chore: fixes for await async
frazarshad Mar 5, 2024
7c1c89e
merge switch wallet PR and resolve conflicts
rabi-siddique Mar 5, 2024
8715041
Merge branch 'dev' into rs-interaction-for-getting-wallet-address
rabi-siddique Mar 5, 2024
157131c
chore: address PR comments
rabi-siddique Mar 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions commands/keplr.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@ const { onboardingElements } = require('../pages/keplr/first-time-flow-page');
const {
notificationPageElements,
} = require('../pages/keplr/notification-page');
const clipboardy = require('clipboardy');


let extensionId;
let extensionVersion;
let registrationUrl;
let permissionsUrl;
let popupUrl;
let walletsPageUrl;
let switchBackToCypressWindow;
let walletAddress;

const keplr = {
async resetState() {
Expand All @@ -19,15 +23,22 @@ const keplr = {
extensionVersion = undefined;
registrationUrl = undefined;
permissionsUrl = undefined;
popupUrl = undefined;
walletAddress = undefined;
walletsPageUrl = undefined;
},
walletAddress: () => {
return walletAddress;

},
extensionId: () => {
return extensionId;
},
extensionUrls: () => {
return {
registrationUrl,
permissionsUrl,
popupUrl,
};
},
async goTo(url) {
Expand All @@ -43,6 +54,9 @@ const keplr = {
async goToPermissions() {
await module.exports.goTo(permissionsUrl);
},
async goToHome() {
await module.exports.goTo(popupUrl);
},
async goToWalletsPage() {
await module.exports.goTo(walletsPageUrl);
},
Expand All @@ -60,13 +74,15 @@ const keplr = {
extensionVersion = keplrExtensionData.version;
registrationUrl = `chrome-extension://${extensionId}/register.html`;
permissionsUrl = `chrome-extension://${extensionId}/popup.html#/setting/security/permission`;
popupUrl = `chrome-extension://${extensionId}/popup.html`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

rename to 'dashbaordUrl' or 'homeUrl'. it more closely resemble the intended purpose of the page

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right. These names are more intention revealing. But I'm noticing that popup.html has dynamic content. For example, it's the same URL when we encounter a notification. I feel we should stick with popupUrl as a generic

walletsPageUrl = `chrome-extension://${extensionId}/popup.html#/wallet/select`;

return {
extensionId,
extensionVersion,
registrationUrl,
permissionsUrl,
popupUrl,
walletsPageUrl,
};
},
Expand Down Expand Up @@ -153,7 +169,6 @@ const keplr = {
await playwright.keplrWindow(),
);

await playwright.switchToCypressWindow();
return true;
},
async importWalletWithPhrase(secretWords) {
Expand Down Expand Up @@ -224,6 +239,17 @@ const keplr = {
return true;
},

async getWalletAddress() {
await playwright.switchToKeplrWindow();
await module.exports.goToHome();
const page = playwright.keplrWindow();
await playwright.waitAndClickByText(notificationPageElements.copyAddress);
await page.click(notificationPageElements.copyWalletAddressSelector);
Copy link
Collaborator

Choose a reason for hiding this comment

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

waitAndClick should be used instead of calling click directly

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a temp. I'll update it according to the appropriate helper method after adding implementation for selecting chain in our import wallet flow. Btw, you tried it with waitAndClick?

walletAddress = clipboardy.readSync();
await playwright.switchToCypressWindow();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've initially tried to get value from the clipboard by executing a script through execute in Playwright. When I tried to get value from the clipboard in the browser, it asked me for permission to access it. It was showing a popup. I tried these things which didn't work:

  • I tried changing permissions in the browser config but they didn't apply. With the Playwright, it involved setting up a whole new context. With Cypress, the options were not according to our needs.
  • I also tried using the Tab key. I tried to press the Tab key 3 times using Playwright, to focus on the permission popup and then press the Enter key to get the permission but encountered issues related to focus there.

I tried a few more things which mainly involved executing a script through execute. But didn't work. So I explored some libraries and found clipboardy. I feel it is an intuitive way of catching the value from the clipboard without adding any complexity. And it is working fine.

return walletAddress;
},

async initialSetup(
playwrightInstance,
{ secretWordsOrPrivateKey, password, newAccount, walletName },
Expand All @@ -235,17 +261,15 @@ const keplr = {
}

await playwright.assignWindows();
if (!playwright.isKeplrWindowActive()) {
await playwright.switchToKeplrWindow();
}
playwright.assignActiveTabName('keplr');
await playwright.switchToKeplrWindow();
await module.exports.getExtensionDetails();
await module.exports.importWallet(
secretWordsOrPrivateKey,
password,
newAccount,
walletName,
);
await playwright.switchToCypressWindow();
},

async switchWallet({ walletName }) {
Expand Down
3 changes: 3 additions & 0 deletions commands/playwright-keplr.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ module.exports = {
isCypressWindowActive() {
return activeTabName === 'cypress';
},
activeTabName() {
return activeTabName;
},
async switchToKeplrWindow() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not being used but I feel it's still handy and can be used in the future. It was helpful when I was debugging an issue in keplr.js file.

await keplrWindow.bringToFront();
module.exports.assignActiveTabName('keplr');
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
"test:e2e:ci:cypress-action": "CYPRESS_USE_ANVIL=true pnpm synpress:run",
"synpress:run:keplr": "EXTENSION=keplr SKIP_EXTENSION_SETUP=true SYNPRESS_LOCAL_TEST=true node synpress.js run --configFile=synpress.config.js",
"test:e2e:keplr": "start-server-and-test 'pnpm start:ui' http-get://localhost:3000 'pnpm start:json-server' http-get://localhost:3004 'pnpm synpress:run:keplr'"

},
"dependencies": {
"@cypress/code-coverage": "^3.11.0",
Expand All @@ -73,6 +72,7 @@
"babel-plugin-transform-react-qa-classes": "^1.6.0",
"babel-plugin-transform-react-styled-components-qa": "^2.1.0",
"bytes32": "^0.0.3",
"clipboardy": "^2.3.0",
"commander": "^11.0.0",
"cypress": "12.17.3",
"cypress-wait-until": "^2.0.1",
Expand Down
4 changes: 4 additions & 0 deletions pages/keplr/notification-page.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
const approveButton = `button`;
const copyAddress = 'Copy Address';
const copyWalletAddressSelector = 'div.sc-dkzDqf div.sc-hKMtZM.sc-kDDrLX.cyoEAq.dkJSBQ'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At the moment, using a selector to copy cosmos address. This is temporary as I have to add an implementation for selecting agoric chain in our importWallet flow. Then I'll update the selector based on that.

module.exports.notificationPageElements = {
approveButton,
copyAddress,
copyWalletAddressSelector
};
1 change: 1 addition & 0 deletions plugins/keplr-plugin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ module.exports = (on, config) => {
importWallet: keplr.importWallet,
acceptAccess: keplr.acceptAccess,
rejectAccess: keplr.rejectAccess,
getWalletAddress: keplr.getWalletAddress,
confirmTransaction: keplr.confirmTransaction,
rejectTransaction: keplr.rejectTransaction,
disconnectWalletFromDapp: keplr.disconnectWalletFromDapp,
Expand Down
76 changes: 76 additions & 0 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions support/commands.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,11 @@ Cypress.Commands.add('disconnectWalletFromDapp', () => {
return cy.task('disconnectWalletFromDapp');
});

Cypress.Commands.add('getWalletAddress', () => {
cy.task('getWalletAddress').then(address => {
return address;
});
});
Cypress.Commands.add('switchWallet', walletName => {
return cy.task('switchWallet', { walletName });
});
7 changes: 7 additions & 0 deletions tests/e2e/specs/keplr/keplr-spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-disable ui-testing/no-disabled-tests */

describe('Keplr', () => {
context('Test commands', () => {
it(`should complete Keplr setup by importing an existing wallet using 24 word phrase`, () => {
Expand Down Expand Up @@ -85,5 +86,11 @@ describe('Keplr', () => {
expect(taskCompleted).to.be.true;
});
});

it(`should get wallet address`, () => {
cy.getWalletAddress().then(walletAddress => {
expect(walletAddress.length).to.be.equal(45);
});
});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At the moment, validating it based on the length. Because cosmos address seems to change in subsequent iterations of running tests. But once I add select chain implementation in our importWallet flow, we will check the exact value over here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

why not get the agoriclocal address instead? that is always constant

Copy link
Collaborator Author

@rabi-siddique rabi-siddique Mar 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, that chain still has to be selected for its address to be visible when collecting wallet addresses. For the implementation I'm referring to, I am planning to select agoric as well as agoric-local in it.

});
});
Loading