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

Adapt app for metamask integration extension #5216

Open
wants to merge 40 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
831bd80
adapted webpack and added packages
joelamouche May 5, 2021
2d5f9ee
sync with master
joelamouche May 6, 2021
22a7803
add types to api
joelamouche May 6, 2021
72b0f6d
sync with master
joelamouche May 7, 2021
3da5017
lint and remove logs
joelamouche May 10, 2021
870155f
sync with master
joelamouche May 11, 2021
6ca0b6d
updates
joelamouche May 24, 2021
ec7ec25
sync
joelamouche Jun 21, 2021
731f9d7
sync with maaster
joelamouche Jun 22, 2021
100ce6d
sync with master
joelamouche Jul 5, 2021
49dcb67
sync with master
joelamouche Jul 7, 2021
114b178
handle undefined type
joelamouche Jul 8, 2021
9e991e8
sync with master
joelamouche Jul 23, 2021
24e78c3
fix types
joelamouche Jul 26, 2021
2a5e72a
add type to web3aounts call
joelamouche Aug 23, 2021
f74b901
sync with master and update deps
joelamouche Sep 3, 2021
a6cd8dd
re added dependencies
joelamouche Sep 3, 2021
1ef3ddb
sync with master
joelamouche Sep 6, 2021
62ea529
update moonbeam deps, remove consoles, update type
joelamouche Sep 6, 2021
053b2a1
add @types/[email protected]
joelamouche Sep 6, 2021
a22f56f
sync with master
joelamouche Sep 7, 2021
54aca19
update moonbeam-types-bundle
joelamouche Nov 19, 2021
87f54be
Merge remote-tracking branch 'paritytech/master'
joelamouche Nov 19, 2021
e18f1bd
Merge remote-tracking branch 'paritytech/master'
joelamouche Dec 6, 2021
77e34f9
Merge remote-tracking branch 'paritytech/master'
joelamouche Dec 7, 2021
14285f2
Merge remote-tracking branch 'paritytech/master'
joelamouche Dec 13, 2021
affbf9c
Merge remote-tracking branch 'paritytech/master'
joelamouche Dec 15, 2021
e49f10f
Merge remote-tracking branch 'paritytech/master'
joelamouche Dec 16, 2021
7f811f1
Merge remote-tracking branch 'paritytech/master'
joelamouche Dec 17, 2021
82a9c14
Merge remote-tracking branch 'paritytech/master'
joelamouche Dec 20, 2021
64b9eda
Merge remote-tracking branch 'paritytech/master'
joelamouche Dec 27, 2021
21588be
sync with master
joelamouche Dec 27, 2021
d68cd99
resolve conflicts
joelamouche Dec 27, 2021
98def48
comment type force
joelamouche Dec 27, 2021
5367b72
reset yarn lock
joelamouche Dec 27, 2021
a9d20aa
fix deps
joelamouche Dec 27, 2021
cae1b3d
sync with master
joelamouche Jan 26, 2022
c0745f9
add missing url
joelamouche Jan 26, 2022
ebc9883
update deps
joelamouche Jan 26, 2022
0a0ce3c
sync with master
joelamouche Jan 26, 2022
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
8 changes: 8 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -99,5 +99,13 @@
"stylelint-config-styled-components": "^0.1.1",
"webpack": "^5.37.0",
"webpack-cli": "^4.7.0"
},
"dependencies": {
"@metamask/detect-provider": "^1.2.0",
"assert": "^2.0.0",
"https-browserify": "^1.0.0",
"os-browserify": "^0.3.0",
"stream-http": "^3.2.0",
"web3": "^1.3.5"
Copy link
Member

Choose a reason for hiding this comment

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

These will just need to drop when the extension is included.

}
}
4 changes: 4 additions & 0 deletions packages/apps/webpack.base.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,11 @@ function createWebpack (context, mode = 'production') {
},
extensions: ['.js', '.jsx', '.mjs', '.ts', '.tsx'],
fallback: {
assert: require.resolve('assert'),
crypto: require.resolve('crypto-browserify'),
http: require.resolve('stream-http'),
https: require.resolve('https-browserify'),
os: require.resolve('os-browserify/browser'),
path: require.resolve('path-browserify'),
stream: require.resolve('stream-browserify')
}
Expand Down
7 changes: 5 additions & 2 deletions packages/react-api/src/Api.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type BN from 'bn.js';
import type { InjectedExtension } from '@polkadot/extension-inject/types';
import type { ChainProperties, ChainType } from '@polkadot/types/interfaces';
import type { KeyringStore } from '@polkadot/ui-keyring/types';
import type { KeypairType } from '@polkadot/util-crypto/types';
import type { ApiProps, ApiState } from './types';

import React, { useContext, useEffect, useMemo, useState } from 'react';
Expand Down Expand Up @@ -40,6 +41,7 @@ interface InjectedAccountExt {
source: string;
whenCreated: number;
};
type: KeypairType
}

interface ChainData {
Expand Down Expand Up @@ -82,13 +84,14 @@ async function getInjectedAccounts (injectedPromise: Promise<InjectedExtension[]

const accounts = await web3Accounts();

return accounts.map(({ address, meta }, whenCreated): InjectedAccountExt => ({
return accounts.map(({ address, meta, type }, whenCreated): InjectedAccountExt => ({
Copy link
Member

Choose a reason for hiding this comment

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

On line 85 above it should explicitly ask for the ethereum types, when the flag is passed. (Comments in extension on this - I'm guessing this is based on the isEthereum flag)

Copy link
Member

Choose a reason for hiding this comment

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

EDIT: Since we do filter on the type in the keyring anyway, it is most probably not needed, since we do have the same effect. (Probably just more descriptive always being explicit, i.e. no need to even pass these through unless needed)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So what do you think I should do? Leave as is?

Copy link
Member

Choose a reason for hiding this comment

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

Ahhhh.... ok this links through to this comment as well polkadot-js/extension#566 (comment)

In the keyring itself, with the injected load, we don't actually check the type at all, e.g. https://github.com/polkadot-js/ui/blob/master/packages/ui-keyring/src/Keyring.ts#L249-L260 - we just inject what is there. The best place to add a solution is either -

  • in the extension, i.e. add an extra flag (may not be the prettiest, as described in the linked comment it is kinda ugly)
  • on the keyring itself, i.e. filtering by type there (just need to follow there again, you did do some type filtering already there, maybe I'm missing something in the link)

Would not add a solution inside the apps UI since there are a lot of extension-dapp users, and don't want them to have to make all the same changes.

Anyway, we can test this once all ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the filter in the extension dapp (in web3accounts), so I will pass down the type flag here

address,
meta: {
...meta,
name: `${meta.name || 'unknown'} (${meta.source === 'polkadot-js' ? 'extension' : meta.source})`,
whenCreated
}
},
type
}));
} catch (error) {
console.error('web3Enable', error);
Expand Down
Loading