Skip to content
This repository has been archived by the owner on May 24, 2022. It is now read-only.

Commit

Permalink
feat: Relates to #360. Only allow import from Parity Signer chain acc…
Browse files Browse the repository at this point in the history
…ount matching current chain. ETC support (#483)

* feat: Relates to #360. Only allow import from Parity Signer chain account matching current chain. ETC support

* review-fix: Refer to non-Parity chain names in the UI. Add console.error

* review-fix: Do not need to chcek health status before calling chainId RPC of light.js on pages accessed through navigation

* review-fix: Rename function name that matches current chain id with imported chain id of address

* review-fix: Remove unnecessary function

* review-fix: Rename function to accountAlreadyExists

* review-fix: Remove FIXME. See #483 (comment)

* review-fix: Refactor to use util functions isEtcChainId, chainIdToString, isNotErc20TokenAddress

* fix: Fix typo in comment

* review-fix: Change wording of parity phrase comment

* review-fix: Do not clear isImport as not account related

* fix: Clear error so error when recover from seed phrase not still shown if then click to recover from QR code

* fix: Rename so signerChainId correctly destructured and not undefined

* review-fix: Remove async/await from clear

* fix: Avoid mapping signer chain id to chain name since too much maintenance with Parity Ethereum

* review-fix: Remove await from createAccountStore

* tests: Add colour to fether-react tests

* refactor: No need to parseInt on the signerChainId

* refactor: Use isNotErc20TokenAddress

* refactor: Use isNotErc20TokenAddress again

* refactor: Add isErc20TokenAddress util so more readable

* fix: Replace valueOf with .eq. Fix so obtain BN from props

* refactor: Combine into single if statement when checking if valid Eth/Etc address

* refactor: Update utils without unnecessary return block
  • Loading branch information
ltfschoen authored and Tbaut committed Apr 15, 2019
1 parent 9ab36c4 commit 95452ee
Show file tree
Hide file tree
Showing 21 changed files with 553 additions and 85 deletions.
2 changes: 1 addition & 1 deletion packages/fether-react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"start": "npm-run-all -p start-*",
"start-css": "npm run build-css -- --watch --recursive",
"start-js": "cross-env SKIP_PREFLIGHT_CHECK=true BROWSER=none craco start",
"test": "cross-env SKIP_PREFLIGHT_CHECK=true craco test"
"test": "cross-env SKIP_PREFLIGHT_CHECK=true craco test --color"
},
"dependencies": {
"@craco/craco": "^4.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,32 @@
// SPDX-License-Identifier: BSD-3-Clause

import React, { Component } from 'react';
import { addressShort, Card, Form as FetherForm } from 'fether-ui';
import { chainId$, chainName$ } from '@parity/light.js';
import light from '@parity/light.js-react';
import { inject, observer } from 'mobx-react';
import { addressShort, Card, Form as FetherForm } from 'fether-ui';

import RequireHealthOverlay from '../../../RequireHealthOverlay';
import Scanner from '../../../Scanner';
import withAccountsInfo from '../../../utils/withAccountsInfo';
import withHealth from '../../../utils/withHealth';
import i18n, { packageNS } from '../../../i18n';

@withAccountsInfo
@withHealth
@inject('createAccountStore')
@light({
chainId: () => chainId$(),
/**
* It is not necessary to check the health status here before
* calling chainId RPC using light.js like we do in Health.js since
* the AccountImportOptions.js page may only be accessed through
* navigation inside the API, after the API is set.
*
* Reference: https://github.com/paritytech/fether/pull/483#discussion_r271303462
*/
chainName: () => chainName$()
})
@observer
class AccountImportOptions extends Component {
state = {
Expand Down Expand Up @@ -48,7 +64,7 @@ class AccountImportOptions extends Component {
try {
await setPhrase(phrase);

if (this.hasExistingAddressForImport(createAccountStore.address)) {
if (this.accountAlreadyExists(createAccountStore.address)) {
return;
}

Expand All @@ -70,12 +86,15 @@ class AccountImportOptions extends Component {
createAccountStore: { setJsonString }
} = this.props;

this.setState({ isLoading: true });
this.setState({
error: '',
isLoading: true
});

try {
await setJsonString(jsonString);

if (this.hasExistingAddressForImport(createAccountStore.address)) {
if (this.accountAlreadyExists(createAccountStore.address)) {
return;
}

Expand All @@ -89,12 +108,21 @@ class AccountImportOptions extends Component {
}
};

handleSignerImported = async ({ address, chainId: chainIdString }) => {
/**
* The `chainId$` and `chainName$` from light.js corresponds to `chainID` in the
* Genesis configs contained in: paritytech/parity-ethereum/ethcore/res/ethereum
* and was introduced in EIP-155 https://github.com/ethereum/EIPs/blob/master/EIPS/eip-155.md
* to prevent replay attacks between `foundation` and `classic` chains, which both have
* `networkID` of `1`.
*/
handleSignerImported = async ({ address, chainId: signerChainId }) => {
const {
chainId: currentChainIdBN,
chainName,
createAccountStore: { importFromSigner }
} = this.props;

if (!address || !chainIdString) {
if (!address || !signerChainId) {
this.setState({
error: i18n.t(
`${packageNS}:account.import.signer.error_msg_signer_imported`
Expand All @@ -103,31 +131,45 @@ class AccountImportOptions extends Component {
return;
}

const chainId = parseInt(chainIdString);
if (!currentChainIdBN.eq(signerChainId)) {
console.error(
`Parity Signer account chainId ${signerChainId} must match current chainId ${currentChainIdBN.valueOf()} (${chainName}).`
);

this.setState({
error: i18n.t(
`${packageNS}:account.import.signer.error_msg_signer_imported_network_mismatch`,
{ chain_name: chainName }
)
});

return;
}

if (this.hasExistingAddressForImport(address, chainId)) {
if (this.accountAlreadyExists(address, signerChainId)) {
return;
}

await importFromSigner({ address, chainId });
await importFromSigner({ address, signerChainId });

this.handleNextStep();
};

handleSignerImport = () => {
this.setState({
error: '',
importingFromSigner: true
});
};

hasExistingAddressForImport = (addressForImport, chainId) => {
accountAlreadyExists = (addressForImport, signerChainId) => {
const { accountsInfo } = this.props;
const isExistingAddress = Object.keys(accountsInfo).some(
key =>
key.toLowerCase() === addressForImport.toLowerCase() &&
(!accountsInfo[key].chainId ||
!chainId ||
accountsInfo[key].chainId === chainId)
!signerChainId ||
accountsInfo[key].chainId === signerChainId)
);

if (isExistingAddress) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import loading from '../../../assets/img/icons/loading.svg';
class AccountName extends Component {
componentDidMount () {
const { createAccountStore } = this.props;

// Generate a new public address if there's none yet
if (!createAccountStore.address) {
createAccountStore.generateNewAccount();
Expand Down
8 changes: 7 additions & 1 deletion packages/fether-react/src/Health/Health.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@ import i18n, { packageNS } from '../i18n';
status: { good, syncing }
}
}) => good || syncing,
// Only call light.js chainName$ if we're syncing or good
/**
* Only call light.js chainName$ if we're syncing or good
* to avoid making an RPC call before the API is set
* (since Health.js is always rendered).
*
* Reference: https://github.com/paritytech/fether/pull/483#discussion_r271303462
*/
light({
chainName: () => chainName$()
})
Expand Down
12 changes: 6 additions & 6 deletions packages/fether-react/src/Onboarding/termsAndConditions.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
**Disclaimer of Liability and Warranties**

- The user expressly acknowledges and agrees that Parity Technologies Limited makes the Fether client available to the user at the user's sole risk.
- The user represents that the user has an adequate understanding of the risks, usage and intricacies of cryptographic tokens and blockchain-based open source software, the Ethereum platform and ETH.
- The user represents that the user has an adequate understanding of the risks, usage and intricacies of cryptographic tokens and blockchain-based open source software, the Ethereum platform, and both ETH and ETC.
- The user acknowledges and agrees that, to the fullest extent permitted by any applicable law, the disclaimers of liability contained herein apply to any and all damages or injury whatsoever caused by or related to risks of, use of, or inability to use, the Fether client under any cause or action whatsoever of any kind in any jurisdiction, including, without limitation, actions for breach of warranty, breach of contract or tort (including negligence) and that Parity Technologies Limited shall not be liable for any indirect, incidental, special, exemplary or consequential damages, including for loss of profits, goodwill or data.
- Some jurisdictions do not allow the exclusion of certain warranties or the limitation or exclusion of liability for certain types of damages. Therefore, some of the above limitations in this section may not apply to a user. In particular, nothing in these terms shall affect the statutory rights of any user or limit or exclude liability for death or physical injury arising from the negligence or wilful misconduct of Parity Technologies Limited or for fraud or fraudulent misrepresentation.
- All rights reserved by Parity Technologies Limited. Licensed to the public under the BSD3.0 License (also known as "BSD-3-Clause"): [https://opensource.org/licenses/BSD-3-Clause](https://opensource.org/licenses/BSD-3-Clause)
Expand All @@ -20,23 +20,23 @@ The User acknowledges the following serious risks to any use of Fether and expre

**Risk of Security Weaknesses in the Parity Core Infrastructure Software**

Fether uses open-source libraries and components developed by third parties. While Parity Technologies Limited generally aims to use only widely adopted open-source technology and develop it in line with industry standards, such open-source technology may contain bugs and errors and may not function correctly in all circumstances. As a result, there is a risk that Parity Technologies or the Parity Technologies Team may have introduced unintentional weaknesses or bugs into the core infrastructural elements of Fether causing the system to lose Ethereum tokens ("ETH") stored in one or more User accounts or other accounts or lose sums of other valued tokens.
Fether uses open-source libraries and components developed by third parties. While Parity Technologies Limited generally aims to use only widely adopted open-source technology and develop it in line with industry standards, such open-source technology may contain bugs and errors and may not function correctly in all circumstances. As a result, there is a risk that Parity Technologies or the Parity Technologies Team may have introduced unintentional weaknesses or bugs into the core infrastructural elements of Fether causing the system to lose Ethereum tokens ("ETH") and Ethereum Classic tokens ("ETC") stored in one or more User accounts or other accounts or lose sums of other valued tokens.

**Risk of Weaknesses or Exploitable Breakthroughs in the Field of Cryptography**

Cryptography is an art, not a science, and the state of the art can advance over time. Advances in code cracking, or technical advances such as the development of quantum computers, could present risks to cryptocurrencies and Fether, which could result in the theft or loss of ETH. To the extent possible, Parity Technologies intends to update the protocol underlying Fether to account for any advances in cryptography and to incorporate additional security measures, but it cannot predict the future of cryptography or guarantee that any security updates will be made, timely or successful.
Cryptography is an art, not a science, and the state of the art can advance over time. Advances in code cracking, or technical advances such as the development of quantum computers, could present risks to cryptocurrencies and Fether, which could result in the theft or loss of ETH and ETC. To the extent possible, Parity Technologies intends to update the protocol underlying Fether to account for any advances in cryptography and to incorporate additional security measures, but it cannot predict the future of cryptography or guarantee that any security updates will be made, timely or successful.

**Risk of Ether Mining Attacks**

As with other cryptocurrencies, the blockchain accessed by Fether is susceptible to mining attacks, including but not limited to double-spend attacks, majority mining power attacks, "selfish-mining" attacks, and race condition attacks. Any successful attacks present a risk to the Ethereum ecosystem, expected proper execution and sequencing of ETH transactions, and expected proper execution and sequencing of contract computations. Despite the efforts of Parity Technologies and the Parity Technologies Team, known or novel mining attacks may be successful.
As with other cryptocurrencies, the blockchain accessed by Fether is susceptible to mining attacks, including but not limited to double-spend attacks, majority mining power attacks, "selfish-mining" attacks, and race condition attacks. Any successful attacks present a risk to the Ethereum ecosystem, expected proper execution and sequencing of ETH and ETC transactions, and expected proper execution and sequencing of contract computations. Despite the efforts of Parity Technologies and the Parity Technologies Team, known or novel mining attacks may be successful.

**Risk of Rapid Adoption and Insufficiency of Computational Application Processing Power on the Ethereum Network**

If Ethereum is rapidly adopted, the demand for transaction processing and distributed application computations could rise dramatically and at a pace that exceeds the rate with which ETH miners can bring online additional mining power. Under such a scenario, the entire Ethereum ecosystem could become destabilized, due to the increased cost of running distributed applications. In turn, this could dampen interest in the Ethereum ecosystem and ETH. Insufficiency of computational resources and an associated rise in the price of ETH could result in businesses being unable to acquire scarce computational resources to run their distributed applications. This would represent revenue losses to businesses or worst case, cause businesses to cease operations because such operations have become uneconomical due to distortions in the crypto-economy.
If Ethereum is rapidly adopted, the demand for transaction processing and distributed application computations could rise dramatically and at a pace that exceeds the rate with which ETH and ETC miners can bring online additional mining power. Under such a scenario, the entire Ethereum ecosystem could become destabilized, due to the increased cost of running distributed applications. In turn, this could dampen interest in the Ethereum ecosystem and both ETH and ETC. Insufficiency of computational resources and an associated rise in the price of ETH or ETC could result in businesses being unable to acquire scarce computational resources to run their distributed applications. This would represent revenue losses to businesses or worst case, cause businesses to cease operations because such operations have become uneconomical due to distortions in the crypto-economy.

**Risk of temporary network incoherence**

We recommend any groups handling large or important transactions to maintain a voluntary 24 hour waiting period on any ETH deposited. If we become aware that the integrity of the network is at risk due to issues with Fether, we will endeavour to publish patches in a timely fashion to address the issues. We will endeavour to provide solutions within the voluntary 24 hour waiting period.
We recommend any groups handling large or important transactions to maintain a voluntary 24 hour waiting period on any ETH or ETC deposited. If we become aware that the integrity of the network is at risk due to issues with Fether, we will endeavour to publish patches in a timely fashion to address the issues. We will endeavour to provide solutions within the voluntary 24 hour waiting period.

**Use of Fether by you**

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ import withTokens from '../../utils/withTokens';
token: tokens[tokenAddress]
}))
@withAccount
@withBalance // Balance of current token (can be ETH)
@withEthBalance // ETH balance
@withBalance // Balance of current token (can be ETH or ETC)
@withEthBalance // ETH or ETC balance
@observer
class SignedTxSummary extends Component {
handleSubmit = values => {
Expand Down
15 changes: 12 additions & 3 deletions packages/fether-react/src/Send/TxForm/TxDetails/TxDetails.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import BigNumber from 'bignumber.js';
import { fromWei, toWei } from '@parity/api/lib/util/wei';

import i18n, { packageNS } from '../../../i18n';
import { chainIdToString, isNotErc20TokenAddress } from '../../../utils/chain';

class TxDetails extends Component {
renderDetails = () => {
Expand Down Expand Up @@ -54,7 +55,8 @@ ${this.renderTotalAmount()}`;
};

renderFee = () => {
const { estimatedTxFee } = this.props;
const { estimatedTxFee, values } = this.props;
const currentChainIdBN = values.chainId;

if (!estimatedTxFee) {
return;
Expand All @@ -64,24 +66,31 @@ ${this.renderTotalAmount()}`;
.toFixed(9)
.toString()}`;

return i18n.t(`${packageNS}:tx.form.details.fee`, { fee });
return i18n.t(`${packageNS}:tx.form.details.fee`, {
chain_id: chainIdToString(currentChainIdBN),
fee
});
};

renderTotalAmount = () => {
const { estimatedTxFee, token, values } = this.props;
const currentChainIdBN = values.chainId;

if (!estimatedTxFee || !values.amount || !token.address) {
return;
}

const totalAmount = `${fromWei(
estimatedTxFee.plus(
token.address === 'ETH' ? toWei(values.amount.toString()) : 0
isNotErc20TokenAddress(token.address)
? toWei(values.amount.toString())
: 0
),
'ether'
).toString()}`;

return i18n.t(`${packageNS}:tx.form.details.total_amount`, {
chain_id: chainIdToString(currentChainIdBN),
total_amount: totalAmount
});
};
Expand Down
Loading

0 comments on commit 95452ee

Please sign in to comment.