From 6e2b74c79a9a88d97ad200c3974bdcb8aaf196e8 Mon Sep 17 00:00:00 2001 From: William O'Beirne Date: Tue, 9 Jan 2018 15:13:14 -0500 Subject: [PATCH] Use network unit everywhere, fix network redux state (#765) * Use network unit in confirmation modal. Make sure network is set at init. * Fix token display * Ensure that when the node changes, the network also changes. Show network unit in unit dropdown. * Type saga, fix tests. --- common/actions/config/actionCreators.ts | 10 +++- common/actions/config/actionTypes.ts | 3 +- .../ConfirmationModal/components/Amount.tsx | 17 ++++-- .../components/UnitDropDown/UnitDropDown.tsx | 11 ++-- common/reducers/config.ts | 1 + common/sagas/config.ts | 55 ++++++++++------- common/selectors/config.ts | 5 +- common/store.ts | 5 ++ spec/reducers/config.spec.ts | 12 +++- spec/sagas/config.spec.ts | 60 ++++++++++--------- 10 files changed, 109 insertions(+), 70 deletions(-) diff --git a/common/actions/config/actionCreators.ts b/common/actions/config/actionCreators.ts index 4a71626c..adb7b467 100644 --- a/common/actions/config/actionCreators.ts +++ b/common/actions/config/actionCreators.ts @@ -1,6 +1,6 @@ import * as interfaces from './actionTypes'; import { TypeKeys } from './constants'; -import { NodeConfig, CustomNodeConfig, CustomNetworkConfig } from 'config/data'; +import { NodeConfig, CustomNodeConfig, NetworkConfig, CustomNetworkConfig } from 'config/data'; export type TForceOfflineConfig = typeof forceOfflineConfig; export function forceOfflineConfig(): interfaces.ForceOfflineAction { @@ -25,10 +25,14 @@ export function changeLanguage(sign: string): interfaces.ChangeLanguageAction { } export type TChangeNode = typeof changeNode; -export function changeNode(nodeSelection: string, node: NodeConfig): interfaces.ChangeNodeAction { +export function changeNode( + nodeSelection: string, + node: NodeConfig, + network: NetworkConfig +): interfaces.ChangeNodeAction { return { type: TypeKeys.CONFIG_NODE_CHANGE, - payload: { nodeSelection, node } + payload: { nodeSelection, node, network } }; } diff --git a/common/actions/config/actionTypes.ts b/common/actions/config/actionTypes.ts index e42d47eb..4157ca52 100644 --- a/common/actions/config/actionTypes.ts +++ b/common/actions/config/actionTypes.ts @@ -1,5 +1,5 @@ import { TypeKeys } from './constants'; -import { NodeConfig, CustomNodeConfig, CustomNetworkConfig } from 'config/data'; +import { NodeConfig, CustomNodeConfig, NetworkConfig, CustomNetworkConfig } from 'config/data'; /*** Toggle Offline ***/ export interface ToggleOfflineAction { @@ -24,6 +24,7 @@ export interface ChangeNodeAction { payload: { nodeSelection: string; node: NodeConfig; + network: NetworkConfig; }; } diff --git a/common/components/ConfirmationModal/components/Amount.tsx b/common/components/ConfirmationModal/components/Amount.tsx index 44ac8c2f..a02d6e72 100644 --- a/common/components/ConfirmationModal/components/Amount.tsx +++ b/common/components/ConfirmationModal/components/Amount.tsx @@ -7,10 +7,12 @@ import React, { Component } from 'react'; import { connect } from 'react-redux'; import { AppState } from 'reducers'; import { getDecimal, getUnit } from 'selectors/transaction'; +import { getNetworkConfig } from 'selectors/config'; interface StateProps { unit: string; decimal: number; + network: AppState['config']['network']; } class AmountClass extends Component { @@ -20,14 +22,16 @@ class AmountClass extends Component { withSerializedTransaction={serializedTransaction => { const transactionInstance = makeTransaction(serializedTransaction); const { value, data } = getTransactionFields(transactionInstance); - const { decimal, unit } = this.props; + const { decimal, unit, network } = this.props; + const isToken = unit !== 'ether'; + const handledValue = isToken + ? TokenValue(ERC20.transfer.decodeInput(data)._value) + : Wei(value); return ( ); @@ -39,5 +43,6 @@ class AmountClass extends Component { export const Amount = connect((state: AppState) => ({ decimal: getDecimal(state), - unit: getUnit(state) + unit: getUnit(state), + network: getNetworkConfig(state) }))(AmountClass); diff --git a/common/components/UnitDropDown/UnitDropDown.tsx b/common/components/UnitDropDown/UnitDropDown.tsx index 1572545c..389f18d4 100644 --- a/common/components/UnitDropDown/UnitDropDown.tsx +++ b/common/components/UnitDropDown/UnitDropDown.tsx @@ -7,6 +7,7 @@ import { Query } from 'components/renderCbs'; import { connect } from 'react-redux'; import { AppState } from 'reducers'; import { getUnit } from 'selectors/transaction'; +import { getNetworkConfig } from 'selectors/config'; interface DispatchProps { setUnitMeta: TSetUnitMeta; @@ -17,6 +18,7 @@ interface StateProps { tokens: TokenBalance[]; allTokens: MergedToken[]; showAllTokens?: boolean; + network: AppState['config']['network']; } const StringDropdown = Dropdown as new () => Dropdown; @@ -24,7 +26,7 @@ const ConditionalStringDropDown = withConditional(StringDropdown); class UnitDropdownClass extends Component { public render() { - const { tokens, allTokens, showAllTokens, unit } = this.props; + const { tokens, allTokens, showAllTokens, unit, network } = this.props; const focusedTokens = showAllTokens ? allTokens : tokens; return (
@@ -32,8 +34,8 @@ class UnitDropdownClass extends Component { params={['readOnly']} withQuery={({ readOnly }) => ( state.config; let hasCheckedOnline = false; export function* pollOfflineStatus(): SagaIterator { while (true) { - const node = yield select(getNodeConfig); - const isOffline = yield select(getOffline); - const isForcedOffline = yield select(getForceOffline); + const node: NodeConfig = yield select(getNodeConfig); + const isOffline: boolean = yield select(getOffline); + const isForcedOffline: boolean = yield select(getForceOffline); // If they're forcing themselves offline, exit the loop. It will be // kicked off again if they toggle it in handleTogglePollOfflineStatus. @@ -104,7 +104,7 @@ export function* handlePollOfflineStatus(): SagaIterator { } export function* handleTogglePollOfflineStatus(): SagaIterator { - const isForcedOffline = yield select(getForceOffline); + const isForcedOffline: boolean = yield select(getForceOffline); if (isForcedOffline) { yield fork(handlePollOfflineStatus); } else { @@ -119,13 +119,20 @@ export function* reload(): SagaIterator { } export function* handleNodeChangeIntent(action: ChangeNodeIntentAction): SagaIterator { - const currentNode = yield select(getNode); - const currentConfig = yield select(getNodeConfig); - const currentNetwork = currentConfig.network; + const currentNode: string = yield select(getNode); + const currentConfig: NodeConfig = yield select(getNodeConfig); + const customNets: CustomNetworkConfig[] = yield select(getCustomNetworkConfigs); + const currentNetwork = + getNetworkConfigFromId(currentConfig.network, customNets) || NETWORKS[currentConfig.network]; + + function* bailOut(message: string) { + yield put(showNotification('danger', message, 5000)); + yield put(changeNode(currentNode, currentConfig, currentNetwork)); + } let actionConfig = NODES[action.payload]; if (!actionConfig) { - const customConfigs = yield select(getCustomNodeConfigs); + const customConfigs: CustomNodeConfig[] = yield select(getCustomNodeConfigs); const config = getCustomNodeConfigFromId(action.payload, customConfigs); if (config) { actionConfig = makeNodeConfigFromCustomConfig(config); @@ -133,11 +140,7 @@ export function* handleNodeChangeIntent(action: ChangeNodeIntentAction): SagaIte } if (!actionConfig) { - yield put( - showNotification('danger', `Attempted to switch to unknown node '${action.payload}'`, 5000) - ); - yield put(changeNode(currentNode, currentConfig)); - return; + return yield* bailOut(`Attempted to switch to unknown node '${action.payload}'`); } // Grab latest block from the node, before switching, to confirm it's online @@ -157,18 +160,24 @@ export function* handleNodeChangeIntent(action: ChangeNodeIntentAction): SagaIte } if (timeout) { - yield put(showNotification('danger', translate('ERROR_32'), 5000)); - yield put(changeNode(currentNode, currentConfig)); - return; + return yield* bailOut(translateRaw('ERROR_32')); + } + + const actionNetwork = getNetworkConfigFromId(actionConfig.network, customNets); + + if (!actionNetwork) { + return yield* bailOut( + `Unknown custom network for your node '${action.payload}', try re-adding it` + ); } yield put(setLatestBlock(latestBlock)); - yield put(changeNode(action.payload, actionConfig)); + yield put(changeNode(action.payload, actionConfig, actionNetwork)); - const currentWallet = yield select(getWalletInst); + const currentWallet: IWallet | null = yield select(getWalletInst); // if there's no wallet, do not reload as there's no component state to resync - if (currentWallet && currentNetwork !== actionConfig.network) { + if (currentWallet && currentConfig.network !== actionConfig.network) { yield call(reload); } } diff --git a/common/selectors/config.ts b/common/selectors/config.ts index 5cd1ce6b..9a8e5c40 100644 --- a/common/selectors/config.ts +++ b/common/selectors/config.ts @@ -8,7 +8,6 @@ import { } from 'config/data'; import { INode } from 'libs/nodes/INode'; import { AppState } from 'reducers'; -import { getNetworkConfigFromId } from 'utils/network'; import { getUnit } from 'selectors/transaction/meta'; import { isEtherUnit } from 'libs/units'; import { SHAPESHIFT_TOKEN_WHITELIST } from 'api/shapeshift'; @@ -25,8 +24,8 @@ export function getNodeLib(state: AppState): INode { return getNodeConfig(state).lib; } -export function getNetworkConfig(state: AppState): NetworkConfig | undefined { - return getNetworkConfigFromId(getNodeConfig(state).network, getCustomNetworkConfigs(state)); +export function getNetworkConfig(state: AppState): NetworkConfig { + return state.config.network; } export function getNetworkContracts(state: AppState): NetworkContract[] | null { diff --git a/common/store.ts b/common/store.ts index fa97bba3..bbc1a92b 100644 --- a/common/store.ts +++ b/common/store.ts @@ -18,6 +18,7 @@ import { loadStatePropertyOrEmptyObject, saveState } from 'utils/localStorage'; import RootReducer from './reducers'; import promiseMiddleware from 'redux-promise-middleware'; import { getNodeConfigFromId } from 'utils/node'; +import { getNetworkConfigFromId } from 'utils/network'; import sagas from './sagas'; import { gasPricetoBase } from 'libs/units'; @@ -72,6 +73,10 @@ const configureStore = () => { // If we couldn't find it, revert to defaults if (savedNode) { savedConfigState.node = savedNode; + const network = getNetworkConfigFromId(savedNode.network, savedConfigState.customNetworks); + if (network) { + savedConfigState.network = network; + } } else { savedConfigState.nodeSelection = configInitialState.nodeSelection; } diff --git a/spec/reducers/config.spec.ts b/spec/reducers/config.spec.ts index 0e908ac0..33b40017 100644 --- a/spec/reducers/config.spec.ts +++ b/spec/reducers/config.spec.ts @@ -1,6 +1,6 @@ import { config, INITIAL_STATE } from 'reducers/config'; import * as configActions from 'actions/config'; -import { NODES } from 'config/data'; +import { NODES, NETWORKS } from 'config/data'; import { makeCustomNodeId, makeNodeConfigFromCustomConfig } from 'utils/node'; const custNode = { @@ -21,8 +21,10 @@ describe('config reducer', () => { it('should handle CONFIG_NODE_CHANGE', () => { const key = Object.keys(NODES)[0]; + const node = NODES[key]; + const network = NETWORKS[node.network]; - expect(config(undefined, configActions.changeNode(key, NODES[key]))).toEqual({ + expect(config(undefined, configActions.changeNode(key, node, network))).toEqual({ ...INITIAL_STATE, node: NODES[key], nodeSelection: key @@ -85,7 +87,11 @@ describe('config reducer', () => { const addedState = config(undefined, configActions.addCustomNode(custNode)); const addedAndActiveState = config( addedState, - configActions.changeNode(customNodeId, makeNodeConfigFromCustomConfig(custNode)) + configActions.changeNode( + customNodeId, + makeNodeConfigFromCustomConfig(custNode), + NETWORKS[custNode.network] + ) ); const removedState = config(addedAndActiveState, configActions.removeCustomNode(custNode)); diff --git a/spec/sagas/config.spec.ts b/spec/sagas/config.spec.ts index ffa11969..fce3d408 100644 --- a/spec/sagas/config.spec.ts +++ b/spec/sagas/config.spec.ts @@ -13,20 +13,21 @@ import { unsetWeb3NodeOnWalletEvent, equivalentNodeOrDefault } from 'sagas/config'; -import { NODES, NodeConfig } from 'config/data'; +import { NODES, NodeConfig, NETWORKS } from 'config/data'; import { getNode, getNodeConfig, getOffline, getForceOffline, - getCustomNodeConfigs + getCustomNodeConfigs, + getCustomNetworkConfigs } from 'selectors/config'; import { INITIAL_STATE as configInitialState } from 'reducers/config'; import { getWalletInst } from 'selectors/wallet'; import { Web3Wallet } from 'libs/wallet'; import { RPCNode } from 'libs/nodes'; import { showNotification } from 'actions/notifications'; -import translate from 'translations'; +import { translateRaw } from 'translations'; // init module configuredStore.getState(); @@ -181,10 +182,13 @@ describe('handleNodeChangeIntent*', () => { // normal operation variables const defaultNode = configInitialState.nodeSelection; const defaultNodeConfig = NODES[defaultNode]; + const customNetworkConfigs = []; + const defaultNodeNetwork = NETWORKS[defaultNodeConfig.network]; const newNode = Object.keys(NODES).reduce( (acc, cur) => (NODES[acc].network === defaultNodeConfig.network ? cur : acc) ); const newNodeConfig = NODES[newNode]; + const newNodeNetwork = NETWORKS[newNodeConfig.network]; const changeNodeIntentAction = changeNodeIntent(newNode); const truthyWallet = true; const latestBlock = '0xa'; @@ -198,6 +202,14 @@ describe('handleNodeChangeIntent*', () => { const data = {} as any; data.gen = cloneableGenerator(handleNodeChangeIntent)(changeNodeIntentAction); + function shouldBailOut(gen, nextVal, errMsg) { + expect(gen.next(nextVal).value).toEqual(put(showNotification('danger', errMsg, 5000))); + expect(gen.next().value).toEqual( + put(changeNode(defaultNode, defaultNodeConfig, defaultNodeNetwork)) + ); + expect(gen.next().done).toEqual(true); + } + beforeAll(() => { originalRandom = Math.random; Math.random = () => 0.001; @@ -215,17 +227,17 @@ describe('handleNodeChangeIntent*', () => { expect(data.gen.next(defaultNode).value).toEqual(select(getNodeConfig)); }); - it('should race getCurrentBlock and delay', () => { - expect(data.gen.next(defaultNodeConfig).value).toMatchSnapshot(); + it('should select getCustomNetworkConfigs', () => { + expect(data.gen.next(defaultNodeConfig).value).toEqual(select(getCustomNetworkConfigs)); }); - it('should put showNotification and put changeNode if timeout', () => { + it('should race getCurrentBlock and delay', () => { + expect(data.gen.next(customNetworkConfigs).value).toMatchSnapshot(); + }); + + it('should show error and revert to previous node if check times out', () => { data.clone1 = data.gen.clone(); - expect(data.clone1.next(raceFailure).value).toEqual( - put(showNotification('danger', translate('ERROR_32'), 5000)) - ); - expect(data.clone1.next().value).toEqual(put(changeNode(defaultNode, defaultNodeConfig))); - expect(data.clone1.next().done).toEqual(true); + shouldBailOut(data.clone1, raceFailure, translateRaw('ERROR_32')); }); it('should put setLatestBlock', () => { @@ -234,7 +246,7 @@ describe('handleNodeChangeIntent*', () => { it('should put changeNode', () => { expect(data.gen.next().value).toEqual( - put(changeNode(changeNodeIntentAction.payload, newNodeConfig)) + put(changeNode(changeNodeIntentAction.payload, newNodeConfig, newNodeNetwork)) ); }); @@ -272,30 +284,24 @@ describe('handleNodeChangeIntent*', () => { it('should select getCustomNodeConfig and match race snapshot', () => { data.customNode.next(); data.customNode.next(defaultNode); - expect(data.customNode.next(defaultNodeConfig).value).toEqual(select(getCustomNodeConfigs)); + data.customNode.next(defaultNodeConfig); + expect(data.customNode.next(customNetworkConfigs).value).toEqual(select(getCustomNodeConfigs)); expect(data.customNode.next(customNodeConfigs).value).toMatchSnapshot(); }); // test custom node not found - it('should select getCustomNodeConfig, put showNotification, put changeNode', () => { + it('should handle unknown / missing custom node', () => { data.customNodeNotFound.next(); data.customNodeNotFound.next(defaultNode); - expect(data.customNodeNotFound.next(defaultNodeConfig).value).toEqual( + data.customNodeNotFound.next(defaultNodeConfig); + expect(data.customNodeNotFound.next(customNetworkConfigs).value).toEqual( select(getCustomNodeConfigs) ); - expect(data.customNodeNotFound.next(customNodeConfigs).value).toEqual( - put( - showNotification( - 'danger', - `Attempted to switch to unknown node '${customNodeNotFoundAction.payload}'`, - 5000 - ) - ) + shouldBailOut( + data.customNodeNotFound, + customNodeConfigs, + `Attempted to switch to unknown node '${customNodeNotFoundAction.payload}'` ); - expect(data.customNodeNotFound.next().value).toEqual( - put(changeNode(defaultNode, defaultNodeConfig)) - ); - expect(data.customNodeNotFound.next().done).toEqual(true); }); });