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

Parity Signer #1349

Merged
merged 20 commits into from Apr 6, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 26 additions & 0 deletions common/actions/wallet/actionCreators.ts
@@ -1,3 +1,4 @@
import EthTx from 'ethereumjs-tx';
import { Wei, TokenValue } from 'libs/units';
import { IWallet, WalletConfig } from 'libs/wallet';
import * as types from './actionTypes';
Expand Down Expand Up @@ -166,3 +167,28 @@ export function setAccountBalance(): types.SetAccountBalanceAction {
type: TypeKeys.WALLET_SET_ACCOUNT_BALANCE
};
}

export type TSetWalletQrTransaction = typeof setWalletQrTransaction;
export function setWalletQrTransaction(
tx: EthTx,
from: string,
onSignature: (signature: string) => void,
onCancel: () => void
): types.SetWalletQrTransactionAction {
return {
type: TypeKeys.WALLET_SET_QR_TRANSACTION,
payload: {
tx,
from,
onSignature,
onCancel
}
};
}

export type TFinalizeQrTransaction = typeof finalizeQrTransaction;
export function finalizeQrTransaction(): types.FinalizeWalletQrTransactionAction {
return {
type: TypeKeys.WALLET_FINALIZE_QR_TX
};
}
14 changes: 13 additions & 1 deletion common/actions/wallet/actionTypes.ts
@@ -1,5 +1,6 @@
import { Wei, TokenValue } from 'libs/units';
import { IWallet, WalletConfig } from 'libs/wallet';
import { QrSignatureState } from 'reducers/wallet';
import { TypeKeys } from './constants';

/*** Unlock Private Key ***/
Expand Down Expand Up @@ -129,6 +130,15 @@ export interface SetAccountBalanceAction {
type: TypeKeys.WALLET_SET_ACCOUNT_BALANCE;
}

export interface SetWalletQrTransactionAction {
type: TypeKeys.WALLET_SET_QR_TRANSACTION;
payload: QrSignatureState;
}

export interface FinalizeWalletQrTransactionAction {
type: TypeKeys.WALLET_FINALIZE_QR_TX;
}

/*** Union Type ***/
export type WalletAction =
| UnlockPrivateKeyAction
Expand All @@ -148,4 +158,6 @@ export type WalletAction =
| SetWalletTokensAction
| SetWalletConfigAction
| SetPasswordPendingAction
| SetAccountBalanceAction;
| SetAccountBalanceAction
| SetWalletQrTransactionAction
| FinalizeWalletQrTransactionAction;
4 changes: 3 additions & 1 deletion common/actions/wallet/constants.ts
Expand Up @@ -20,5 +20,7 @@ export enum TypeKeys {
WALLET_SET_CONFIG = 'WALLET_SET_CONFIG',
WALLET_RESET = 'WALLET_RESET',
WALLET_SET_PASSWORD_PENDING = 'WALLET_SET_PASSWORD_PENDING',
WALLET_SET_ACCOUNT_BALANCE = 'WALLET_SET_ACCOUNT_BALANCE'
WALLET_SET_ACCOUNT_BALANCE = 'WALLET_SET_ACCOUNT_BALANCE',
WALLET_SET_QR_TRANSACTION = 'WALLET_SET_QR_TRANSACTION',
WALLET_FINALIZE_QR_TX = 'WALLET_FINALIZE_QR_TX'
}
Binary file added common/assets/images/mobile/app-store-badge.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added common/assets/images/mobile/google-play-badge.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions common/assets/images/wallets/paritysigner.svg
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 2 additions & 0 deletions common/components/Footer/index.tsx
Expand Up @@ -12,6 +12,7 @@ import React from 'react';
import PreFooter from './PreFooter';
import DisclaimerModal from './DisclaimerModal';
import { NewTabLink } from 'components/ui';
import QrSignerModal from 'containers/QrSignerModal';
import OnboardModal from 'containers/OnboardModal';
import './index.scss';
import { translateRaw } from 'translations';
Expand Down Expand Up @@ -219,6 +220,7 @@ export default class Footer extends React.PureComponent<Props, State> {
</div>
</footer>

<QrSignerModal />
Copy link
Contributor

Choose a reason for hiding this comment

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

Would prefer this component only be rendered where it contextually makes sense, somewhere in the send form.

<OnboardModal />
<DisclaimerModal isOpen={this.state.isDisclaimerOpen} handleClose={this.toggleModal} />
</div>
Expand Down
11 changes: 11 additions & 0 deletions common/components/WalletDecrypt/WalletDecrypt.tsx
Expand Up @@ -31,6 +31,7 @@ import {
WalletButton,
InsecureWalletWarning
} from './components';
import ParitySignerDecrypt from 'containers/ParitySignerDecrypt';
import { AppState } from 'reducers';
import { showNotification, TShowNotification } from 'actions/notifications';
import { getDisabledWallets } from 'selectors/wallet';
Expand All @@ -49,6 +50,7 @@ import LedgerIcon from 'assets/images/wallets/ledger.svg';
import MetamaskIcon from 'assets/images/wallets/metamask.svg';
import MistIcon from 'assets/images/wallets/mist.svg';
import TrezorIcon from 'assets/images/wallets/trezor.svg';
import ParitySignerIcon from 'assets/images/wallets/paritysigner.svg';
import './WalletDecrypt.scss';
import { withRouter, RouteComponentProps } from 'react-router';

Expand Down Expand Up @@ -168,6 +170,15 @@ const WalletDecrypt = withRouter<Props>(
unlock: this.props.setWallet,
helpLink: 'https://doc.satoshilabs.com/trezor-apps/mew.html'
},
[SecureWalletName.PARITY_SIGNER]: {
lid: 'X_PARITYSIGNER',
icon: ParitySignerIcon,
description: 'ADD_PARITY_DESC',
component: ParitySignerDecrypt,
initialParams: {},
unlock: this.props.setWallet,
helpLink: 'https://github.com/paritytech/parity-signer/blob/master/README.md'
},
[InsecureWalletName.KEYSTORE_FILE]: {
lid: 'X_KEYSTORE2',
example: 'UTC--2017-12-15T17-35-22.547Z--6be6e49e82425a5aa56396db03512f2cc10e95e8',
Expand Down
3 changes: 2 additions & 1 deletion common/components/WalletDecrypt/disables.ts
Expand Up @@ -22,9 +22,10 @@ export const DISABLE_WALLETS: { [key in WalletMode]: DisabledWallets } = {
}
},
[WalletMode.UNABLE_TO_SIGN]: {
wallets: [SecureWalletName.TREZOR, MiscWalletName.VIEW_ONLY],
wallets: [SecureWalletName.TREZOR, SecureWalletName.PARITY_SIGNER, MiscWalletName.VIEW_ONLY],
reasons: {
[SecureWalletName.TREZOR]: 'This wallet can’t sign messages',
[SecureWalletName.PARITY_SIGNER]: 'This wallet can’t sign messages',
Copy link
Contributor

Choose a reason for hiding this comment

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

Not your responsibility, but just a reminder that we should be translating this.

[MiscWalletName.VIEW_ONLY]: 'This wallet can’t sign messages'
}
}
Expand Down
4 changes: 3 additions & 1 deletion common/components/ui/Modal/ModalBody.tsx
Expand Up @@ -28,8 +28,10 @@ export default class ModalBody extends React.Component<Props> {
this.modal.querySelectorAll(focusableElementsString)
);

const first = focusableElements[0];

// Convert NodeList to Array
this.firstTabStop = focusableElements[0];
this.firstTabStop = first;
this.lastTabStop = focusableElements[focusableElements.length - 1];

this.modal.addEventListener('keydown', this.keyDownListener);
Expand Down
3 changes: 2 additions & 1 deletion common/config/data.tsx
Expand Up @@ -62,7 +62,8 @@ export const ethercardReferralURL =
export enum SecureWalletName {
WEB3 = 'web3',
LEDGER_NANO_S = 'ledgerNanoS',
TREZOR = 'trezor'
TREZOR = 'trezor',
PARITY_SIGNER = 'paritySigner'
}

export enum HardwareWalletName {
Expand Down
14 changes: 14 additions & 0 deletions common/containers/ParitySignerDecrypt/index.scss
@@ -0,0 +1,14 @@
.ParitySignerUnlock {
a img {
display: inline-block;
height: 3em;
margin: 0 0.4em;
}

.qr-bounds {
Copy link
Contributor

Choose a reason for hiding this comment

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

We use SuitCSS naming convention to avoid class name conflicts. Likewise, the a img above would be better suited by class names.

width: 300px;
height: 300px;
display: inline-block;
margin-bottom: 1.5em;
}
}
67 changes: 67 additions & 0 deletions common/containers/ParitySignerDecrypt/index.tsx
@@ -0,0 +1,67 @@
import React, { PureComponent } from 'react';
import { connect } from 'react-redux';
import translate from 'translations';
import QrSigner from '@parity/qr-signer';
import { isValidETHAddress } from 'libs/validators';
import { ParitySignerWallet } from 'libs/wallet';
import {
setWalletQrTransaction,
TSetWalletQrTransaction,
finalizeQrTransaction,
TFinalizeQrTransaction
} from 'actions/wallet';
import './index.scss';
import AppStoreBadge from 'assets/images/mobile/app-store-badge.png';
import GooglePlayBadge from 'assets/images/mobile/google-play-badge.png';

interface Props {
setWalletQrTransaction: TSetWalletQrTransaction;
finalizeQrTransaction: TFinalizeQrTransaction;
onUnlock(param: any): void;
}

class ParitySignerDecrypt extends PureComponent<Props> {
public render() {
return (
<div className="ParitySignerUnlock">
<div className="qr-bounds">
<QrSigner
size={300}
scan={true}
onScan={this.unlockAddress}
styles={{ display: 'inline-block' }}
/>
</div>
<p>{translate('ADD_PARITY_2')}</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

At some point we should put together a help article that explains this process, or have some additional descriptive copy here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are working on a proper help page that describes how to use the signer, I'll add a link to it here once it's done (ideally before publicity but can be done after the PR is merged).

<p>
<a href="https://itunes.apple.com/us/app/parity-signer/id1218174838" rel="nofollow">
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the NewTabLink component here.

<img src={AppStoreBadge} alt="App Store" />
</a>
<a href="https://play.google.com/store/apps/details?id=com.nativesigner" rel="nofollow">
<img src={GooglePlayBadge} alt="Google Play" />
</a>
</p>
</div>
);
}

private unlockAddress = (address: string) => {
if (!isValidETHAddress(address)) {
console.error('Invalid address!');
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the user experience for when this happens? Does the signer keep watching for a potentially valid address?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, it will keep on scanning until a QR code with a valid address is presented.


this.props.onUnlock(
new ParitySignerWallet(
address,
this.props.setWalletQrTransaction,
this.props.finalizeQrTransaction
)
);
};
}

export default connect(() => ({}), {
setWalletQrTransaction,
finalizeQrTransaction
})(ParitySignerDecrypt);
8 changes: 8 additions & 0 deletions common/containers/QrSignerModal/index.scss
@@ -0,0 +1,8 @@
.QrSignerModal {
.qr-bounds {
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer SuitCSS naming convention.

width: 300px;
height: 300px;
display: inline-block;
margin: 0.3em 0;
}
}
120 changes: 120 additions & 0 deletions common/containers/QrSignerModal/index.tsx
@@ -0,0 +1,120 @@
import React from 'react';
import { connect } from 'react-redux';
import translate, { translateRaw } from 'translations';
import EthTx from 'ethereumjs-tx';
import QrSigner from '@parity/qr-signer';
import { AppState } from 'reducers';
import Modal, { IButton } from 'components/ui/Modal';
import './index.scss';

interface State {
scan: boolean;
}

interface PropsClosed {
isOpen: false;
}

interface PropsOpen {
isOpen: true;
from: string;
tx: EthTx;
onSignature(signature: string): void;
onCancel(): void;
}

type Props = PropsClosed | PropsOpen;

class QrSignerModal extends React.Component<Props, State> {
constructor(props: Props) {
super(props);
this.state = {
scan: false
};
}

public render() {
if (!this.props.isOpen) {
return null;
}

const { scan } = this.state;
const { tx, from } = this.props;

// Poor man's serialize without signature.
// All those values are later overriden by actual signature
// values in: wallets/non-deterministic/parity.ts
tx.v = Buffer.from([tx._chainId]);
tx.r = Buffer.from([0]);
tx.s = Buffer.from([0]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like you're mutating the tx props object here. If you want to alter something passed in as props, you'll need to copy it and save it in state. Something like...

public componentDidMount() {
  if (this.props.tx) {
    this.makeStateTx(this.props);
  }
}

public componentWillReceiveProps(nextProps: Props) {
  if (nextProps.tx && this.props.tx !== nextProps.tx) {
    this.makeStateTx(nextProps);
  }
}

private makeStateTx(props: Props) {
  const tx = { ...props.tx };
  tx.v = Buffer.from([tx._chainId]);
  tx.r = Buffer.from([0]);
  tx.s = Buffer.from([0]);
  this.setState({ tx });
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The values aren't read anywhere down the line, so it shouldn't really matter. Ideally there would be a way to serialize a transaction to RLP in ethereumjs without including the signature (tx.serialize(false) to match tx.hash(false)), but that's not the option. I can make a helper function for it here, and make a PR to ethereumjs.

Copy link
Contributor

Choose a reason for hiding this comment

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

The problem is it makes for non-deterministic redux state, makes it so there's no way to determine if the tx object has changed (without checking deep equality) and in general is just advised against in React.

If you want to avoid the state boilerplate, it could at least do

const tx = { ...this.props.tx };

in render.


const rlp = '0x' + tx.serialize().toString('hex');
const buttons: IButton[] = [
{
disabled: false,
text: translate(scan ? 'ACTION_4' : 'ADD_PARITY_3'),
type: 'primary',
onClick: () => this.setState({ scan: !scan })
},
{
disabled: false,
text: translate('ACTION_2'),
type: 'default',
onClick: this.onClose
}
];

return (
<div className="QrSignerModal">
<Modal
title={translateRaw('DEP_SIGNTX')}
isOpen={true}
buttons={buttons}
handleClose={this.onClose}
>
<div className="qr-bounds">
<QrSigner size={300} scan={scan} account={from} rlp={rlp} onScan={this.onScan} />
</div>
</Modal>
</div>
);
}

private onClose = () => {
if (!this.props.isOpen) {
return;
}

this.props.onCancel();
this.setState({ scan: false });
};

private onScan = (signature: string) => {
if (!this.props.isOpen) {
return;
}

this.props.onSignature(signature);
this.setState({ scan: false });
};
}

function mapStateToProps(state: AppState) {
const { signViaQr } = state.wallet;

if (!signViaQr) {
return { isOpen: false };
}

const { tx, from, onSignature, onCancel } = signViaQr;

return {
isOpen: true,
tx,
from,
onSignature,
onCancel
};
}

export default connect(mapStateToProps, {})(QrSignerModal);