From 8d1964bc3b408abd590b3ffffdac374ca3171fa6 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Mon, 9 Oct 2017 13:11:18 +0200 Subject: [PATCH] Fix wallet view (#6597) * Add safe fail for empty logs * Filter transactions * Add more logging * Fix Wallet Creation and wallet tx list * Remove logs * Prevent selecting twice same wallet owner * Fix tests * Remove unused props * Remove unused props --- .../WalletDetails/walletDetails.js | 32 ++++- .../WalletDetails/walletDetails.spec.js | 23 +++- .../modals/CreateWallet/createWalletStore.js | 3 +- .../ParametersStep/parametersStep.js | 4 +- .../modals/DeployContract/deployContract.js | 1 - .../DetailsStep/detailsStep.js | 3 +- .../modals/WalletSettings/walletSettings.js | 8 +- js/src/ui/Form/AddressSelect/addressSelect.js | 13 ++ .../Form/AddressSelect/addressSelectStore.js | 3 +- .../InputAddressSelect/inputAddressSelect.js | 27 +++- js/src/ui/Form/TypedInput/typedInput.js | 10 +- js/src/util/wallets.js | 30 +++- js/src/util/wallets/consensys-wallet.js | 1 + js/src/util/wallets/foundation-wallet.js | 128 ++++++++++++------ js/src/views/Contract/Queries/inputQuery.js | 4 +- js/src/views/Contract/Queries/queries.js | 6 +- js/src/views/Contract/contract.js | 7 +- 17 files changed, 221 insertions(+), 82 deletions(-) diff --git a/js/src/modals/CreateWallet/WalletDetails/walletDetails.js b/js/src/modals/CreateWallet/WalletDetails/walletDetails.js index d3776e2cb..ee0cc77e2 100644 --- a/js/src/modals/CreateWallet/WalletDetails/walletDetails.js +++ b/js/src/modals/CreateWallet/WalletDetails/walletDetails.js @@ -16,18 +16,21 @@ import React, { Component, PropTypes } from 'react'; import { FormattedMessage } from 'react-intl'; +import { connect } from 'react-redux'; import { Form, TypedInput, Input, AddressSelect, InputAddress } from '~/ui'; import styles from '../createWallet.css'; -export default class WalletDetails extends Component { +class WalletDetails extends Component { static propTypes = { accounts: PropTypes.object.isRequired, wallet: PropTypes.object.isRequired, errors: PropTypes.object.isRequired, onChange: PropTypes.func.isRequired, - walletType: PropTypes.string.isRequired + walletType: PropTypes.string.isRequired, + + knownAddresses: PropTypes.array }; render () { @@ -103,7 +106,10 @@ export default class WalletDetails extends Component { } renderMultisigDetails () { - const { accounts, wallet, errors } = this.props; + const { accounts, knownAddresses, wallet, errors } = this.props; + const allowedOwners = knownAddresses + // Exclude sender and already owners of the wallet + .filter((address) => !wallet.owners.includes(address) && address !== wallet.account); return (
@@ -163,7 +169,7 @@ export default class WalletDetails extends Component { /> ({ + knownAddresses + }); +} + +export default connect( + mapStateToProps, + null +)(WalletDetails); diff --git a/js/src/modals/CreateWallet/WalletDetails/walletDetails.spec.js b/js/src/modals/CreateWallet/WalletDetails/walletDetails.spec.js index 6518e6893..39bfecc04 100644 --- a/js/src/modals/CreateWallet/WalletDetails/walletDetails.spec.js +++ b/js/src/modals/CreateWallet/WalletDetails/walletDetails.spec.js @@ -25,6 +25,22 @@ import { ACCOUNTS } from '../createWallet.test.js'; let component; let onChange; +function createRedux () { + return { + dispatch: sinon.stub(), + subscribe: sinon.stub(), + getState: () => { + return { + personal: { + accounts: {}, + contacts: {}, + contracts: {} + } + }; + } + }; +} + function render (walletType = 'MULTISIG') { onChange = sinon.stub(); component = shallow( @@ -36,7 +52,12 @@ function render (walletType = 'MULTISIG') { owners: [] } } walletType={ walletType } - /> + />, + { + context: { + store: createRedux() + } + } ); return component; diff --git a/js/src/modals/CreateWallet/createWalletStore.js b/js/src/modals/CreateWallet/createWalletStore.js index c155ad1c1..75f34d269 100644 --- a/js/src/modals/CreateWallet/createWalletStore.js +++ b/js/src/modals/CreateWallet/createWalletStore.js @@ -283,7 +283,8 @@ export default class CreateWalletStore { const owners = _wallet.owners.filter((owner) => !/^(0x)?0*$/.test(owner)); - if (_wallet.required > owners.length) { + // Real number of owners is owners + creator + if (_wallet.required > owners.length + 1) { requiredValidation.valueError = 'the number of required validators should be lower or equal the number of owners'; } diff --git a/js/src/modals/DeployContract/ParametersStep/parametersStep.js b/js/src/modals/DeployContract/ParametersStep/parametersStep.js index 0213bfa01..e6d0bc731 100644 --- a/js/src/modals/DeployContract/ParametersStep/parametersStep.js +++ b/js/src/modals/DeployContract/ParametersStep/parametersStep.js @@ -43,7 +43,6 @@ export default class ParametersStep extends Component { }; static propTypes = { - accounts: PropTypes.object.isRequired, onParamsChange: PropTypes.func.isRequired, inputs: PropTypes.array, @@ -60,7 +59,7 @@ export default class ParametersStep extends Component { } renderConstructorInputs () { - const { accounts, params, paramsError } = this.props; + const { params, paramsError } = this.props; const { inputs } = this.props; if (!inputs || !inputs.length) { @@ -78,7 +77,6 @@ export default class ParametersStep extends Component { return (
diff --git a/js/src/modals/ExecuteContract/DetailsStep/detailsStep.js b/js/src/modals/ExecuteContract/DetailsStep/detailsStep.js index 913def0ee..ecbcbed26 100644 --- a/js/src/modals/ExecuteContract/DetailsStep/detailsStep.js +++ b/js/src/modals/ExecuteContract/DetailsStep/detailsStep.js @@ -177,7 +177,7 @@ export default class DetailsStep extends Component { } renderParameters () { - const { accounts, func, values, valuesError, onValueChange } = this.props; + const { func, values, valuesError, onValueChange } = this.props; if (!func) { return null; @@ -197,7 +197,6 @@ export default class DetailsStep extends Component { value={ values[index] } error={ valuesError[index] } onChange={ onChange } - accounts={ accounts } param={ input.type } isEth={ false } /> diff --git a/js/src/modals/WalletSettings/walletSettings.js b/js/src/modals/WalletSettings/walletSettings.js index 1984e6213..b6266b861 100644 --- a/js/src/modals/WalletSettings/walletSettings.js +++ b/js/src/modals/WalletSettings/walletSettings.js @@ -34,7 +34,6 @@ class WalletSettings extends Component { }; static propTypes = { - accountsInfo: PropTypes.object.isRequired, wallet: PropTypes.object.isRequired, onClose: PropTypes.func.isRequired, senders: PropTypes.object.isRequired @@ -74,7 +73,7 @@ class WalletSettings extends Component { default: case 'EDIT': const { errors, fromString, wallet } = this.store; - const { accountsInfo, senders } = this.props; + const { senders } = this.props; return ( @@ -143,7 +142,6 @@ class WalletSettings extends Component { } value={ wallet.owners.slice() } onChange={ this.store.onOwnersChange } - accounts={ accountsInfo } param='address[]' /> @@ -443,13 +441,13 @@ class WalletSettings extends Component { } function mapStateToProps (initState, initProps) { - const { accountsInfo, accounts } = initState.personal; + const { accounts } = initState.personal; const { owners } = initProps.wallet; const senders = pick(accounts, owners); return () => { - return { accountsInfo, senders }; + return { senders }; }; } diff --git a/js/src/ui/Form/AddressSelect/addressSelect.js b/js/src/ui/Form/AddressSelect/addressSelect.js index 8a2c6d577..41e58f19f 100644 --- a/js/src/ui/Form/AddressSelect/addressSelect.js +++ b/js/src/ui/Form/AddressSelect/addressSelect.js @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . +import { eq } from 'lodash'; import React, { Component, PropTypes } from 'react'; import ReactDOM from 'react-dom'; import { connect } from 'react-redux'; @@ -93,6 +94,18 @@ class AddressSelect extends Component { } componentWillReceiveProps (nextProps) { + if (!eq(Object.keys(this.props.accounts), Object.keys(nextProps.accounts))) { + return this.setValues(nextProps); + } + + if (!eq(Object.keys(this.props.contacts), Object.keys(nextProps.contacts))) { + return this.setValues(nextProps); + } + + if (!eq(Object.keys(this.props.contracts), Object.keys(nextProps.contracts))) { + return this.setValues(nextProps); + } + if (this.store.values && this.store.values.length > 0) { return; } diff --git a/js/src/ui/Form/AddressSelect/addressSelectStore.js b/js/src/ui/Form/AddressSelect/addressSelectStore.js index 7fc1db480..d897dc9a1 100644 --- a/js/src/ui/Form/AddressSelect/addressSelectStore.js +++ b/js/src/ui/Form/AddressSelect/addressSelectStore.js @@ -165,7 +165,8 @@ export default class AddressSelectStore { const contactsN = Object.keys(contacts).length; if (accountsN + contractsN + contactsN === 0) { - return; + this.initValues = []; + return this.handleChange(); } this.initValues = [ diff --git a/js/src/ui/Form/InputAddressSelect/inputAddressSelect.js b/js/src/ui/Form/InputAddressSelect/inputAddressSelect.js index 8199fece6..c8909ac72 100644 --- a/js/src/ui/Form/InputAddressSelect/inputAddressSelect.js +++ b/js/src/ui/Form/InputAddressSelect/inputAddressSelect.js @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . +import { pick } from 'lodash'; import React, { Component, PropTypes } from 'react'; import { connect } from 'react-redux'; @@ -28,6 +29,7 @@ class InputAddressSelect extends Component { contracts: PropTypes.object.isRequired, allowCopy: PropTypes.bool, + allowedValues: PropTypes.array, className: PropTypes.string, error: nodeOrStringProptype(), hint: nodeOrStringProptype(), @@ -38,16 +40,33 @@ class InputAddressSelect extends Component { }; render () { - const { accounts, allowCopy, className, contacts, contracts, label, hint, error, value, onChange, readOnly } = this.props; + const { accounts, allowCopy, allowedValues, className, contacts, contracts, label, hint, error, value, onChange, readOnly } = this.props; + // Add the currently selected value to the list + // of allowed values, if any given + const nextAllowedValues = allowedValues + ? [].concat(allowedValues, value || []) + : null; + + const filteredAccounts = nextAllowedValues + ? pick(accounts, nextAllowedValues) + : accounts; + + const filteredContacts = nextAllowedValues + ? pick(contacts, nextAllowedValues) + : accounts; + + const filteredContracts = nextAllowedValues + ? pick(contracts, nextAllowedValues) + : accounts; return ( { return transactions.sort((txA, txB) => { - const comp = txB.blockNumber.comparedTo(txA.blockNumber); + const bnA = txA.blockNumber; + const bnB = txB.blockNumber; + + if (!bnA) { + console.warn('could not find block number in transaction', txA); + return 1; + } + + if (!bnB) { + console.warn('could not find block number in transaction', txB); + return -1; + } + + const comp = bnA.comparedTo(bnB); if (comp !== 0) { return comp; } - return txB.transactionIndex.comparedTo(txA.transactionIndex); + const txIdxA = txA.transactionIndex; + const txIdxB = txB.transactionIndex; + + if (!txIdxA) { + console.warn('could not find transaction index in transaction', txA); + return 1; + } + + if (!txIdxB) { + console.warn('could not find transaction index in transaction', txB); + return -1; + } + + return txIdxA.comparedTo(txIdxB); }); }); } diff --git a/js/src/util/wallets/consensys-wallet.js b/js/src/util/wallets/consensys-wallet.js index 9f9f9d5fa..bfc8cb679 100644 --- a/js/src/util/wallets/consensys-wallet.js +++ b/js/src/util/wallets/consensys-wallet.js @@ -212,6 +212,7 @@ export default class ConsensysWalletUtils { const transaction = { transactionHash: log.transactionHash, + transactionIndex: log.transactionIndex, blockNumber: log.blockNumber }; diff --git a/js/src/util/wallets/foundation-wallet.js b/js/src/util/wallets/foundation-wallet.js index 4fb1cfe22..926b40a80 100644 --- a/js/src/util/wallets/foundation-wallet.js +++ b/js/src/util/wallets/foundation-wallet.js @@ -130,27 +130,67 @@ export default class FoundationWalletUtils { .ConfirmationNeeded .getAllLogs() .then((logs) => { - return logs.map((log) => ({ - initiator: log.params.initiator.value, - to: log.params.to.value, - data: log.params.data.value, - value: log.params.value.value, - operation: bytesToHex(log.params.operation.value), - transactionIndex: log.transactionIndex, - transactionHash: log.transactionHash, - blockNumber: log.blockNumber, - confirmedBy: [] - })); + return logs + .filter((log) => { + if (!log.blockNumber) { + console.warn('got a log without blockNumber', log); + return false; + } + + if (!log.transactionIndex) { + console.warn('got a log without transactionIndex', log); + return false; + } + + return true; + }) + .map((log) => ({ + initiator: log.params.initiator.value, + to: log.params.to.value, + data: log.params.data.value, + value: log.params.value.value, + operation: bytesToHex(log.params.operation.value), + transactionIndex: log.transactionIndex, + transactionHash: log.transactionHash, + blockNumber: log.blockNumber, + confirmedBy: [] + })); }) .then((logs) => { return logs.sort((logA, logB) => { - const comp = logA.blockNumber.comparedTo(logB.blockNumber); + const bnA = logA.blockNumber; + const bnB = logA.blockNumber; + + if (!bnA) { + console.warn('could not find block number in log', logA); + return 1; + } + + if (!bnB) { + console.warn('could not find block number in log', logB); + return -1; + } + + const comp = bnA.comparedTo(bnB); if (comp !== 0) { return comp; } - return logA.transactionIndex.comparedTo(logB.transactionIndex); + const txIdxA = logA.transactionIndex; + const txIdxB = logB.transactionIndex; + + if (!txIdxA) { + console.warn('could not find transaction index in log', logA); + return 1; + } + + if (!txIdxB) { + console.warn('could not find transaction index in log', logB); + return -1; + } + + return txIdxA.comparedTo(txIdxB); }); }) .then((pendingTxs) => { @@ -205,40 +245,48 @@ export default class FoundationWalletUtils { ] ] }) .then((logs) => { - const transactions = logs.map((log) => { - const signature = toHex(log.topics[0]); + const transactions = logs + .map((log) => { + const signature = toHex(log.topics[0]); - const value = log.params.value.value; - const from = signature === WalletSignatures.Deposit - ? log.params['_from'].value - : walletContract.address; + const value = log.params.value.value; + const from = signature === WalletSignatures.Deposit + ? log.params['_from'].value + : walletContract.address; - const to = signature === WalletSignatures.Deposit - ? walletContract.address - : log.params.to.value; + const to = signature === WalletSignatures.Deposit + ? walletContract.address + : log.params.to.value; - const transaction = { - transactionHash: log.transactionHash, - blockNumber: log.blockNumber, - from, to, value - }; + const transaction = { + transactionHash: log.transactionHash, + transactionIndex: log.transactionIndex, + blockNumber: log.blockNumber, + from, to, value + }; - if (log.params.created && log.params.created.value && !/^(0x)?0*$/.test(log.params.created.value)) { - transaction.creates = log.params.created.value; - delete transaction.to; - } + if (!transaction.blockNumber) { + console.warn('log without block number', log); + return null; + } - if (log.params.operation) { - transaction.operation = bytesToHex(log.params.operation.value); - checkPendingOperation(api, log, transaction.operation); - } + if (log.params.created && log.params.created.value && !/^(0x)?0*$/.test(log.params.created.value)) { + transaction.creates = log.params.created.value; + delete transaction.to; + } - if (log.params.data) { - transaction.data = log.params.data.value; - } + if (log.params.operation) { + transaction.operation = bytesToHex(log.params.operation.value); + checkPendingOperation(api, log, transaction.operation); + } - return transaction; - }); + if (log.params.data) { + transaction.data = log.params.data.value; + } + + return transaction; + }) + .filter((tx) => tx); return transactions; }); diff --git a/js/src/views/Contract/Queries/inputQuery.js b/js/src/views/Contract/Queries/inputQuery.js index 7a75727f6..0e8b043d0 100644 --- a/js/src/views/Contract/Queries/inputQuery.js +++ b/js/src/views/Contract/Queries/inputQuery.js @@ -35,7 +35,6 @@ class InputQuery extends Component { }; static propTypes = { - accountsInfo: PropTypes.object.isRequired, contract: PropTypes.object.isRequired, inputs: arrayOrObjectProptype().isRequired, outputs: arrayOrObjectProptype().isRequired, @@ -122,7 +121,7 @@ class InputQuery extends Component { renderResults () { const { results, isLoading } = this.state; - const { accountsInfo, outputs } = this.props; + const { outputs } = this.props; if (isLoading) { return ( @@ -143,7 +142,6 @@ class InputQuery extends Component { .map((out, index) => { const input = ( @@ -530,12 +528,11 @@ class Contract extends Component { } function mapStateToProps (state) { - const { accounts, accountsInfo, contracts } = state.personal; + const { accounts, contracts } = state.personal; const { netVersion } = state.nodeStatus; return { accounts, - accountsInfo, contracts, netVersion };