From 38dd22953a96a62e3f2e2e466ca0c42e3fcf257b Mon Sep 17 00:00:00 2001 From: Daniel Ternyak Date: Fri, 8 Sep 2017 14:01:31 -0500 Subject: [PATCH] Ether Unit Types and Send UX Improvements (#174) * remove 'transate' property and ng-scopes * use bigs (surprised flow did not catch this) * fix dropdown not expanding -- switch to simpledropdown * Don't use generics for no real reason * Create Ether, Wei, and GWei types, and annotate. Also contains refactors and UX improvements 1. clear previously generated rawTX/signedTx when changes to transaction inputs are made. 2. reset generated rawTx/signedTx while new generateTx is loading * add toString helper method and use in place of .amount.toString() * support optional base in toString helper method and use * incorporate PR suggestions (destructure, resolve via callback) --- .../containers/Tabs/SendTransaction/index.jsx | 79 ++++++++---- common/libs/nodes/INode.js | 3 +- common/libs/nodes/rpc/index.js | 5 +- common/libs/transaction.js | 117 ++++++++---------- common/libs/units.js | 47 +++++++ common/libs/values.js | 9 +- common/libs/wallet/trezor.js | 4 +- common/sagas/wallet.js | 6 +- 8 files changed, 169 insertions(+), 101 deletions(-) diff --git a/common/containers/Tabs/SendTransaction/index.jsx b/common/containers/Tabs/SendTransaction/index.jsx index 52b857b2..4a3beb0c 100644 --- a/common/containers/Tabs/SendTransaction/index.jsx +++ b/common/containers/Tabs/SendTransaction/index.jsx @@ -51,7 +51,7 @@ import type { } from 'libs/transaction'; import type { TransactionWithoutGas } from 'libs/messages'; import type { UNIT } from 'libs/units'; -import { toWei } from 'libs/units'; +import { Wei, Ether, GWei } from 'libs/units'; import { generateCompleteTransaction, getBalanceMinusGasCosts, @@ -94,13 +94,13 @@ type Props = { } }, wallet: BaseWallet, - balance: Big, + balance: Ether, node: NodeConfig, nodeLib: RPCNode, network: NetworkConfig, tokens: Token[], tokenBalances: TokenBalance[], - gasPrice: string, + gasPrice: Wei, broadcastTx: (signedTx: string) => BroadcastTxRequestedAction, showNotification: ( level: string, @@ -366,6 +366,7 @@ export class SendTransaction extends React.Component { this.estimateGas(); } } catch (error) { + this.setState({ generateDisabled: true }); this.props.showNotification('danger', error.message, 5000); } } @@ -404,36 +405,63 @@ export class SendTransaction extends React.Component { this.setState({ gasLimit: value, gasChanged: true }); }; + handleEverythingAmountChange = (value: string, unit: string): string => { + if (unit === 'ether') { + const { balance, gasPrice } = this.props; + const { gasLimit } = this.state; + const weiBalance = balance.toWei(); + const bigGasLimit = new Big(gasLimit); + value = getBalanceMinusGasCosts( + bigGasLimit, + gasPrice, + weiBalance + ).toString(); + } else { + const tokenBalance = this.props.tokenBalances.find( + tokenBalance => tokenBalance.symbol === unit + ); + if (!tokenBalance) { + throw new Error(`${unit}: not found in token balances;`); + } + value = tokenBalance.balance.toString(); + } + return value; + }; + onAmountChange = (value: string, unit: string) => { if (value === 'everything') { - if (unit === 'ether') { - const { balance, gasPrice } = this.props; - const { gasLimit } = this.state; - const weiBalance = toWei(balance, 'ether'); - value = getBalanceMinusGasCosts( - new Big(gasLimit), - new Big(gasPrice), - weiBalance - ); - } else { - const tokenBalance = this.props.tokenBalances.find( - tokenBalance => tokenBalance.symbol === unit - ); - if (!tokenBalance) { - return; - } - value = tokenBalance.balance.toString(); - } + value = this.handleEverythingAmountChange(value, unit); + } + let transaction = this.state.transaction; + let generateDisabled = this.state.generateDisabled; + if (unit && unit !== this.state.unit) { + value = ''; + transaction = null; + generateDisabled = true; } let token = this.props.tokens.find(x => x.symbol === unit); this.setState({ value, unit, - token + token, + transaction, + generateDisabled + }); + }; + + resetJustTx = async (): Promise<*> => { + new Promise(resolve => { + this.setState( + { + transaction: null + }, + resolve + ); }); }; generateTxFromState = async () => { + await this.resetJustTx(); const { nodeLib, wallet, gasPrice, network } = this.props; const { token, unit, value, to, data, gasLimit } = this.state; const chainId = network.chainId; @@ -444,12 +472,13 @@ export class SendTransaction extends React.Component { to, data }; + const bigGasLimit = new Big(gasLimit); try { const signedTx = await generateCompleteTransaction( wallet, nodeLib, gasPrice, - gasLimit, + bigGasLimit, chainId, transactionInput ); @@ -483,13 +512,13 @@ export class SendTransaction extends React.Component { function mapStateToProps(state: AppState) { return { wallet: state.wallet.inst, - balance: state.wallet.balance, + balance: new Ether(state.wallet.balance), tokenBalances: getTokenBalances(state), node: getNodeConfig(state), nodeLib: getNodeLib(state), network: getNetworkConfig(state), tokens: getTokens(state), - gasPrice: toWei(new Big(getGasPriceGwei(state)), 'gwei').toString(), + gasPrice: new GWei(getGasPriceGwei(state)).toWei(), transactions: state.wallet.transactions }; } diff --git a/common/libs/nodes/INode.js b/common/libs/nodes/INode.js index 29f018fa..64adfe0e 100644 --- a/common/libs/nodes/INode.js +++ b/common/libs/nodes/INode.js @@ -2,9 +2,10 @@ import Big from 'bignumber.js'; import type { TransactionWithoutGas } from 'libs/messages'; import type { Token } from 'config/data'; +import type { Wei } from 'libs/units'; export interface INode { - getBalance(_address: string): Promise, + getBalance(_address: string): Promise, getTokenBalance(_address: string, _token: Token): Promise, getTokenBalances(_address: string, _tokens: Token[]): Promise, estimateGas(_tx: TransactionWithoutGas): Promise, diff --git a/common/libs/nodes/rpc/index.js b/common/libs/nodes/rpc/index.js index a5e0e48b..7c5486ea 100644 --- a/common/libs/nodes/rpc/index.js +++ b/common/libs/nodes/rpc/index.js @@ -10,6 +10,7 @@ import RPCClient, { sendRawTx } from './client'; import type { Token } from 'config/data'; +import { Wei } from 'libs/units'; export default class RpcNode implements INode { client: RPCClient; @@ -17,12 +18,12 @@ export default class RpcNode implements INode { this.client = new RPCClient(endpoint); } - getBalance(address: string): Promise { + getBalance(address: string): Promise { return this.client.call(getBalance(address)).then(response => { if (response.error) { throw new Error(response.error.message); } - return new Big(String(response.result)); + return new Wei(String(response.result)); }); } diff --git a/common/libs/transaction.js b/common/libs/transaction.js index a5e0c7dd..683f09b9 100644 --- a/common/libs/transaction.js +++ b/common/libs/transaction.js @@ -4,17 +4,15 @@ import translate from 'translations'; import { padToEven, addHexPrefix, toChecksumAddress } from 'ethereumjs-util'; import { isValidETHAddress } from 'libs/validators'; import ERC20 from 'libs/erc20'; -import { toTokenUnit } from 'libs/units'; -import { stripHex } from 'libs/values'; +import { stripHex, valueToHex } from 'libs/values'; +import { Wei, Ether, toTokenUnit } from 'libs/units'; +import { RPCNode } from 'libs/nodes'; +import { TransactionWithoutGas } from 'libs/messages'; import type { INode } from 'libs/nodes/INode'; import type { BaseWallet } from 'libs/wallet'; import type { Token } from 'config/data'; import type EthTx from 'ethereumjs-tx'; -import { toUnit } from 'libs/units'; -import { valueToHex } from 'libs/values'; import type { UNIT } from 'libs/units'; -import { RPCNode } from 'libs/nodes'; -import { TransactionWithoutGas } from 'libs/messages'; export type TransactionInput = { token: ?Token, @@ -34,8 +32,8 @@ export type BaseTransaction = {| to: string, value: string, data: string, - gasLimit: string, - gasPrice: string, + gasLimit: Big, + gasPrice: Wei, chainId: number |}; @@ -84,73 +82,69 @@ export async function generateCompleteTransactionFromRawTransaction( wallet: BaseWallet, token: ?Token ): Promise { + const { to, data, gasLimit, gasPrice, from, chainId, nonce } = tx; // Reject bad addresses - if (!isValidETHAddress(tx.to)) { - throw new Error(translate('ERROR_5')); + if (!isValidETHAddress(to)) { + throw new Error(translate('ERROR_5', true)); } - // Reject token transactions without data - if (token && !tx.data) { + if (token && !data) { throw new Error('Tokens must be sent with data'); } - // Reject gas limit under 21000 (Minimum for transaction) // Reject if limit over 5000000 // TODO: Make this dynamic, the limit shifts - const limitBig = new Big(tx.gasLimit); - if (limitBig.lessThan(21000)) { - throw new Error( - translate('Gas limit must be at least 21000 for transactions') - ); + if (gasLimit.lessThan(21000)) { + throw new Error('Gas limit must be at least 21000 for transactions'); } - - if (limitBig.greaterThan(5000000)) { - throw new Error(translate('GETH_GasLimit')); + // Reject gasLimit over 5000000gwei + if (gasLimit.greaterThan(5000000)) { + throw new Error(translate('GETH_GasLimit', true)); } - - // Reject gas over 1000gwei (1000000000000) - const gasPriceBig = new Big(tx.gasPrice); - if (gasPriceBig.greaterThan(new Big('1000000000000'))) { + // Reject gasPrice over 1000gwei (1000000000000) + if (gasPrice.amount.greaterThan(new Big('1000000000000'))) { throw new Error( 'Gas price too high. Please contact support if this was not a mistake.' ); } - + // build gasCost by multiplying gasPrice * gasLimit + const gasCost: Wei = new Wei(gasPrice.amount.times(gasLimit)); // Ensure their balance exceeds the amount they're sending - // TODO: Include gas price too, tokens should probably check ETH too let value; let balance; - + const ETHBalance: Wei = await node.getBalance(from); if (token) { value = new Big(ERC20.$transfer(tx.data).value); balance = toTokenUnit(await node.getTokenBalance(tx.from, token), token); } else { value = new Big(tx.value); - balance = await node.getBalance(tx.from); + balance = ETHBalance.amount; } - - if (value.gte(balance)) { - throw new Error(translate('GETH_Balance')); + if (value.gt(balance)) { + throw new Error(translate('GETH_Balance', true)); + } + // ensure gas cost is not greaterThan current eth balance + // TODO check that eth balance is not lesser than txAmount + gasCost + if (gasCost.amount.gt(ETHBalance.amount)) { + throw new Error( + `gasCost: ${gasCost.amount} greaterThan ETHBalance: ${ETHBalance.amount}` + ); } - // Taken from v3's `sanitizeHex`, ensures that the value is a %2 === 0 // prefix'd hex value. const cleanHex = hex => addHexPrefix(padToEven(stripHex(hex))); - const cleanedRawTx = { - nonce: cleanHex(tx.nonce), - gasPrice: cleanHex(new Big(tx.gasPrice).toString(16)), - gasLimit: cleanHex(new Big(tx.gasLimit).toString(16)), - to: cleanHex(tx.to), + nonce: cleanHex(nonce), + gasPrice: cleanHex(gasPrice.toString(16)), + gasLimit: cleanHex(gasLimit.toString(16)), + to: cleanHex(to), value: token ? '0x00' : cleanHex(value.toString(16)), - data: tx.data ? cleanHex(tx.data) : '', - chainId: tx.chainId || 1 + data: data ? cleanHex(data) : '', + chainId: chainId || 1 }; - // Sign the transaction const rawTxJson = JSON.stringify(cleanedRawTx); const signedTx = await wallet.signRawTransaction(cleanedRawTx); - // Repeat all of this shit for Flow typechecking. Sealed objects don't // like spreads, so we have to be explicit. return { @@ -174,7 +168,7 @@ export async function formatTxInput( return { to, from: await wallet.getAddress(), - value: valueToHex(value), + value: valueToHex(new Ether(value)), data }; } else { @@ -182,11 +176,12 @@ export async function formatTxInput( throw new Error('No matching token'); } const bigAmount = new Big(value); + const ERC20Data = ERC20.transfer(to, bigAmount); return { to: token.address, from: await wallet.getAddress(), value: '0x0', - data: ERC20.transfer(to, toTokenUnit(bigAmount, token)) + data: ERC20Data }; } } @@ -194,28 +189,26 @@ export async function formatTxInput( export async function generateCompleteTransaction( wallet: BaseWallet, nodeLib: RPCNode, - gasPrice: string, - gasLimit: string, + gasPrice: Wei, + gasLimit: Big, chainId: number, transactionInput: TransactionInput ): Promise { const { token } = transactionInput; - - const formattedTx = await formatTxInput(wallet, transactionInput); - - const from = await wallet.getAddress(); - + const { from, to, value, data } = await formatTxInput( + wallet, + transactionInput + ); const transaction: ExtendedRawTransaction = { nonce: await nodeLib.getTransactionCount(from), from, - to: formattedTx.to, + to, gasLimit, - value: formattedTx.value, - data: formattedTx.data, + value, + data, chainId, gasPrice }; - return await generateCompleteTransactionFromRawTransaction( nodeLib, transaction, @@ -226,11 +219,11 @@ export async function generateCompleteTransaction( // TODO determine best place for helper function export function getBalanceMinusGasCosts( - weiGasLimit: Big, - weiGasPrice: Big, - weiBalance: Big -): Big { - const weiGasCosts = weiGasPrice.times(weiGasLimit); - const weiBalanceMinusGasCosts = weiBalance.minus(weiGasCosts); - return toUnit(weiBalanceMinusGasCosts, 'wei', 'ether'); + gasLimit: Big, + gasPrice: Wei, + balance: Wei +): Ether { + const weiGasCosts = gasPrice.amount.times(gasLimit); + const weiBalanceMinusGasCosts = balance.amount.minus(weiGasCosts); + return new Ether(weiBalanceMinusGasCosts); } diff --git a/common/libs/units.js b/common/libs/units.js index c4eb0ba0..26fa4cf7 100644 --- a/common/libs/units.js +++ b/common/libs/units.js @@ -31,6 +31,53 @@ const UNITS = { export type UNIT = $Keys; +class Unit { + unit: UNIT; + amount: Big; + + constructor(amount: Big, unit: UNIT) { + if (!(unit in UNITS)) { + throw new Error(`Supplied unit: ${unit} is not a valid unit.`); + } + this.unit = unit; + this.amount = amount; + } + + toString(base?: number) { + return this.amount.toString(base); + } + + toWei(): Wei { + return new Wei(toWei(this.amount, this.unit)); + } + + toGWei(): GWei { + return new GWei(toUnit(this.amount, this.unit, 'gwei')); + } + + toEther(): Ether { + return new Ether(toUnit(this.amount, this.unit, 'ether')); + } +} + +export class Ether extends Unit { + constructor(amount: Big | number | string) { + super(new Big(amount), 'ether'); + } +} + +export class Wei extends Unit { + constructor(amount: Big | number | string) { + super(new Big(amount), 'wei'); + } +} + +export class GWei extends Unit { + constructor(amount: Big | number | string) { + super(new Big(amount), 'gwei'); + } +} + function getValueOfUnit(unit: UNIT) { return new Big(UNITS[unit]); } diff --git a/common/libs/values.js b/common/libs/values.js index 976c0d17..f7af6b72 100644 --- a/common/libs/values.js +++ b/common/libs/values.js @@ -1,16 +1,13 @@ // @flow -import Big from 'bignumber.js'; -import { toWei } from 'libs/units'; +import { Ether } from 'libs/units'; export function stripHex(address: string): string { return address.replace('0x', '').toLowerCase(); } -export function valueToHex(n: Big | number | string): string { - // Convert it to a Big to handle any and all values. - const big = new Big(n); +export function valueToHex(value: Ether): string { // Values are in ether, so convert to wei for RPC calls - const wei = toWei(big, 'ether'); + const wei = value.toWei(); // Finally, hex it up! return `0x${wei.toString(16)}`; } diff --git a/common/libs/wallet/trezor.js b/common/libs/wallet/trezor.js index 30c5e3e6..dcefb1e6 100644 --- a/common/libs/wallet/trezor.js +++ b/common/libs/wallet/trezor.js @@ -15,8 +15,8 @@ export default class TrezorWallet extends DeterministicWallet { // Args this.getPath(), stripHex(tx.nonce), - stripHex(tx.gasPrice), - stripHex(tx.gasLimit), + stripHex(tx.gasPrice.toString()), + stripHex(tx.gasLimit.toString()), stripHex(tx.to), stripHex(tx.value), stripHex(tx.data), diff --git a/common/sagas/wallet.js b/common/sagas/wallet.js index 88015488..c730ab95 100644 --- a/common/sagas/wallet.js +++ b/common/sagas/wallet.js @@ -23,7 +23,7 @@ import { } from 'libs/wallet'; import { INode } from 'libs/nodes/INode'; import { determineKeystoreType } from 'libs/keystore'; - +import type { Wei } from 'libs/units'; import { getNodeLib } from 'selectors/config'; import { getWalletInst, getTokens } from 'selectors/wallet'; @@ -38,8 +38,8 @@ function* updateAccountBalance(): Generator { const node: INode = yield select(getNodeLib); const address = yield wallet.getAddress(); // network request - let balance = yield apply(node, node.getBalance, [address]); - yield put(setBalance(balance)); + let balance: Wei = yield apply(node, node.getBalance, [address]); + yield put(setBalance(balance.amount)); } catch (error) { yield put({ type: 'updateAccountBalance_error', error }); }