From 0e53f7daf10e44125d222932bc0a00afafe4d6ad Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Fri, 18 Nov 2016 18:49:21 +0100 Subject: [PATCH 01/11] Don't auto-subscribe for contracts #3240 --- js/src/api/contract/contract.js | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/js/src/api/contract/contract.js b/js/src/api/contract/contract.js index 06afb0d9d33..1671be7960b 100644 --- a/js/src/api/contract/contract.js +++ b/js/src/api/contract/contract.js @@ -48,7 +48,8 @@ export default class Contract { this._instance[fn.signature] = fn; }); - this._sendSubscriptionChanges(); + this._subscribedToChanges = false; + this._subscribedToChangesId = null; } get address () { @@ -271,12 +272,14 @@ export default class Contract { .getFilterLogs(filterId) .then((logs) => { callback(null, this.parseEventLogs(logs)); + this._subscriptions[subscriptionId] = { options, callback, filterId }; + this._subscribeToChanges(); return subscriptionId; }); }); @@ -287,12 +290,30 @@ export default class Contract { .uninstallFilter(this._subscriptions[subscriptionId].filterId) .then(() => { delete this._subscriptions[subscriptionId]; + + if (Object.keys(this._subscriptions).length === 0) { + this._unsubscribeToChanges(); + } }) .catch((error) => { console.error('unsubscribe', error); }); } + _subscribeToChanges = () => { + if (this._subscribedToChanges) { + return; + } + + this._subscribedToChanges = true; + this._sendSubscriptionChanges(); + } + + _unsubscribeToChanges = () => { + this._subscribedToChanges = false; + clearTimeout(this._subscribedToChangesId); + } + _sendSubscriptionChanges = () => { const subscriptions = Object.values(this._subscriptions); const timeout = () => setTimeout(this._sendSubscriptionChanges, 1000); @@ -316,11 +337,11 @@ export default class Contract { } }); - timeout(); + this._subscribedToChangesId = timeout(); }) .catch((error) => { console.error('_sendSubscriptionChanges', error); - timeout(); + this._subscribedToChangesId = timeout(); }); } } From 601e9ec784855a05fc4f27dc46283f66dd4857a1 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Fri, 18 Nov 2016 19:04:27 +0100 Subject: [PATCH 02/11] Smarter Balance : don't re-instantiate contracts, fetch tokens #3240 --- js/src/redux/providers/balances.js | 183 ++++++++++++++++------------- 1 file changed, 103 insertions(+), 80 deletions(-) diff --git a/js/src/redux/providers/balances.js b/js/src/redux/providers/balances.js index b80fad28f9a..75159ea462b 100644 --- a/js/src/redux/providers/balances.js +++ b/js/src/redux/providers/balances.js @@ -32,7 +32,9 @@ export default class Balances { this._api = api; this._store = store; this._accountsInfo = null; - this._tokens = []; + this._tokens = {}; + this._images = {}; + this._tokenreg = null; } start () { @@ -75,8 +77,12 @@ export default class Balances { }); } - _retrieveTokens () { - this._api.parity + getTokenRegistry () { + if (this._tokenreg) { + return Promise.resolve(this._tokenreg); + } + + return this._api.parity .registryAddress() .then((registryAddress) => { const registry = this._api.newContract(abis.registry, registryAddress); @@ -85,55 +91,42 @@ export default class Balances { }) .then((tokenregAddress) => { const tokenreg = this._api.newContract(abis.tokenreg, tokenregAddress); + this._tokenreg = tokenreg; + return tokenreg; + }); + } + + _retrieveTokens () { + this + .getTokenRegistry() + .then((tokenreg) => { return tokenreg.instance.tokenCount .call() .then((numTokens) => { - const promisesTokens = []; - const promisesImages = []; + const promises = []; - while (promisesTokens.length < numTokens.toNumber()) { - const index = promisesTokens.length; - - promisesTokens.push(tokenreg.instance.token.call({}, [index])); - promisesImages.push(tokenreg.instance.meta.call({}, [index, 'IMG'])); + for (let i = 0; i < numTokens.toNumber(); i++) { + promises.push(this.fetchTokenInfo(tokenreg, i)); } - return Promise.all([ - Promise.all(promisesTokens), - Promise.all(promisesImages) - ]); + return Promise.all(promises); }); }) - .then(([_tokens, images]) => { - const tokens = {}; - this._tokens = _tokens - .map((_token, index) => { - const [address, tag, format, name] = _token; - - const token = { - address, - name, - tag, - format: format.toString(), - contract: this._api.newContract(abis.eip20, address) - }; - tokens[address] = token; - this._store.dispatch(setAddressImage(address, images[index])); - - return token; - }) - .sort((a, b) => { - if (a.tag < b.tag) { - return -1; - } else if (a.tag > b.tag) { - return 1; - } - - return 0; - }); + .then((_tokens) => { + const prevHashes = Object.values(this._tokens).map((t) => t.hash).sort().join(''); + const nextHashes = _tokens.map((t) => t.hash).sort().join(''); + + if (prevHashes !== nextHashes) { + this._tokens = _tokens + .reduce((obj, token) => { + obj[token.address] = token; + return obj; + }, {}); + + this._store.dispatch(getTokens(this._tokens)); + } - this._store.dispatch(getTokens(tokens)); this._retrieveBalances(); }) .catch((error) => { @@ -151,44 +144,10 @@ export default class Balances { this._balances = {}; Promise - .all( - addresses.map((address) => Promise.all([ - this._api.eth.getBalance(address), - this._api.eth.getTransactionCount(address) - ])) - ) - .then((balanceTxCount) => { - return Promise.all( - balanceTxCount.map(([value, txCount], idx) => { - const address = addresses[idx]; - - this._balances[address] = { - txCount, - tokens: [{ - token: ETH, - value - }] - }; - - return Promise.all( - this._tokens.map((token) => { - return token.contract.instance.balanceOf.call({}, [address]); - }) - ); - }) - ); - }) - .then((tokenBalances) => { - addresses.forEach((address, idx) => { - const balanceOf = tokenBalances[idx]; - const balance = this._balances[address]; - - this._tokens.forEach((token, tidx) => { - balance.tokens.push({ - token, - value: balanceOf[tidx] - }); - }); + .all(addresses.map((a) => this.fetchAccountBalance(a))) + .then((balances) => { + addresses.forEach((a, idx) => { + this._balances[a] = balances[idx]; }); this._store.dispatch(getBalances(this._balances)); @@ -197,4 +156,68 @@ export default class Balances { console.warn('_retrieveBalances', error); }); } + + fetchTokenInfo (tokenreg, tokenId) { + return Promise + .all([ + tokenreg.instance.token.call({}, [tokenId]), + tokenreg.instance.meta.call({}, [tokenId, 'IMG']) + ]) + .then(([ token, image ]) => { + const [ address, tag, format, name ] = token; + const oldToken = this._tokens[address]; + + if (this._images[address] !== image.toString()) { + this._store.dispatch(setAddressImage(address, image)); + this._images[address] = image.toString(); + } + + const newToken = { + address, + name, + tag, + format: format.toString() + }; + + const hash = this._api.util.sha3(JSON.stringify(newToken)); + + const contract = oldToken + ? oldToken.contract + : this._api.newContract(abis.eip20, address); + + return { + ...newToken, + hash, + contract + }; + }); + } + + fetchAccountBalance (address) { + const _tokens = Object.values(this._tokens); + const tokensPromises = _tokens + .map((token) => { + return token.contract.instance.balanceOf.call({}, [ address ]); + }); + + return Promise + .all([ + this._api.eth.getTransactionCount(address), + this._api.eth.getBalance(address) + ].concat(tokensPromises)) + .then(([ txCount, ethBalance, ...tokensBalance ]) => { + const tokens = [] + .concat( + { token: ETH, value: ethBalance }, + _tokens + .map((token, index) => ({ + token, + value: tokensBalance[index] + })) + ); + + const balance = { txCount, tokens }; + return balance; + }); + } } From 9ebe84a661aef218c165be70d69f3c5e9d0cc337 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Sat, 19 Nov 2016 04:06:47 +0100 Subject: [PATCH 03/11] Smarter Balance Tokens fetching (update image when needed) #3240 --- js/src/api/contract/contract.js | 87 +++++++++++++++----- js/src/api/format/input.js | 10 ++- js/src/api/util/format.js | 4 + js/src/api/util/index.js | 3 +- js/src/redux/providers/balances.js | 122 ++++++++++++++++++++--------- 5 files changed, 165 insertions(+), 61 deletions(-) diff --git a/js/src/api/contract/contract.js b/js/src/api/contract/contract.js index 1671be7960b..51cdc64d1dd 100644 --- a/js/src/api/contract/contract.js +++ b/js/src/api/contract/contract.js @@ -233,6 +233,10 @@ export default class Contract { return this._subscribe(event, options, callback); }; + event.register = (options = {}, callback) => { + return this._register(event, options, callback); + }; + event.unsubscribe = (subscriptionId) => { return this.unsubscribe(subscriptionId); }; @@ -240,33 +244,79 @@ export default class Contract { return event; } - subscribe (eventName = null, options = {}, callback) { - return new Promise((resolve, reject) => { - let event = null; + _register (event = null, _options, callback) { + return this + ._createEthFilter(event, _options) + .then((filterId) => { + return { + fetch: () => { + return this._api.eth + .getFilterChanges(filterId) + .then((logs) => { + if (!logs || !logs.length) { + return; + } + + try { + callback(null, this.parseEventLogs(logs)); + } catch (error) { + callback(error); + } + }); + }, + + unsubscribe: () => { + return this._api.eth.uninstallFilter(filterId); + } + }; + }); + } - if (eventName) { - event = this._events.find((evt) => evt.name === eventName); + _findEvent (eventName = null) { + const event = eventName + ? this._events.find((evt) => evt.name === eventName) + : null; - if (!event) { - const events = this._events.map((evt) => evt.name).join(', '); - reject(new Error(`${eventName} is not a valid eventName, subscribe using one of ${events} (or null to include all)`)); - return; - } - } + if (eventName && !event) { + const events = this._events.map((evt) => evt.name).join(', '); + throw new Error(`${eventName} is not a valid eventName, subscribe using one of ${events} (or null to include all)`); + } - return this._subscribe(event, options, callback).then(resolve).catch(reject); - }); + return event; } - _subscribe (event = null, _options, callback) { - const subscriptionId = nextSubscriptionId++; + _createEthFilter (event = null, _options) { + const optionTopics = _options.topics || []; + const signature = event && event.signature || null; + + // If event provided, remove the potential event signature + // as the first element of the topics + const topics = signature + ? [ signature ].concat(optionTopics.filter((t, idx) => idx > 0 || t !== signature)) + : optionTopics; + const options = Object.assign({}, _options, { address: this._address, - topics: [event ? event.signature : null] + topics }); - return this._api.eth - .newFilter(options) + return this._api.eth.newFilter(options); + } + + subscribe (eventName = null, options = {}, callback) { + try { + const event = this._findEvent(eventName); + return this._subscribe(event, options, callback); + } catch (e) { + return Promise.reject(e); + } + } + + _subscribe (event = null, _options, callback) { + const subscriptionId = nextSubscriptionId++; + + return this + ._createEthFilter(event, _options) .then((filterId) => { return this._api.eth .getFilterLogs(filterId) @@ -274,7 +324,6 @@ export default class Contract { callback(null, this.parseEventLogs(logs)); this._subscriptions[subscriptionId] = { - options, callback, filterId }; diff --git a/js/src/api/format/input.js b/js/src/api/format/input.js index 4cd1c8a56d2..bd3d8ba854d 100644 --- a/js/src/api/format/input.js +++ b/js/src/api/format/input.js @@ -15,6 +15,7 @@ // along with Parity. If not, see . import BigNumber from 'bignumber.js'; +import { range } from 'lodash'; import { isArray, isHex, isInstanceOf, isString } from '../util/types'; @@ -50,10 +51,15 @@ export function inHash (hash) { return inHex(hash); } +export function pad (input, length) { + const value = inHex(input).substr(2, length * 2); + return '0x' + value + range(length * 2 - value.length).map(() => '0').join(''); +} + export function inTopics (_topics) { let topics = (_topics || []) - .filter((topic) => topic) - .map(inHex); + .filter((topic) => topic === null || topic) + .map((topic) => topic === null ? null : pad(topic, 32)); while (topics.length < 4) { topics.push(null); diff --git a/js/src/api/util/format.js b/js/src/api/util/format.js index 198e456ee5f..93f31a16191 100644 --- a/js/src/api/util/format.js +++ b/js/src/api/util/format.js @@ -29,3 +29,7 @@ export function hex2Ascii (_hex) { return str; } + +export function asciiToHex (string) { + return '0x' + string.split('').map((s) => s.charCodeAt(0).toString(16)).join(''); +} diff --git a/js/src/api/util/index.js b/js/src/api/util/index.js index 55cf008c532..2058cd011e4 100644 --- a/js/src/api/util/index.js +++ b/js/src/api/util/index.js @@ -16,7 +16,7 @@ import { isAddress as isAddressValid, toChecksumAddress } from '../../abi/util/address'; import { decodeCallData, decodeMethodInput, methodToAbi } from './decode'; -import { bytesToHex, hex2Ascii } from './format'; +import { bytesToHex, hex2Ascii, asciiToHex } from './format'; import { fromWei, toWei } from './wei'; import { sha3 } from './sha3'; import { isArray, isFunction, isHex, isInstanceOf, isString } from './types'; @@ -31,6 +31,7 @@ export default { isString, bytesToHex, hex2Ascii, + asciiToHex, createIdentityImg, decodeCallData, decodeMethodInput, diff --git a/js/src/redux/providers/balances.js b/js/src/redux/providers/balances.js index 75159ea462b..c13bf9778d4 100644 --- a/js/src/redux/providers/balances.js +++ b/js/src/redux/providers/balances.js @@ -35,11 +35,14 @@ export default class Balances { this._tokens = {}; this._images = {}; this._tokenreg = null; + this._fetchedTokens = false; + this._tokenregSub = null; } start () { this._subscribeBlockNumber(); this._subscribeAccountsInfo(); + this._retrieveTokens(); } _subscribeAccountsInfo () { @@ -50,10 +53,7 @@ export default class Balances { } this._accountsInfo = accountsInfo; - this._retrieveBalances(); - }) - .then((subscriptionId) => { - console.log('_subscribeAccountsInfo', 'subscriptionId', subscriptionId); + this._retrieveTokens(); }) .catch((error) => { console.warn('_subscribeAccountsInfo', error); @@ -68,9 +68,10 @@ export default class Balances { } this._retrieveTokens(); - }) - .then((subscriptionId) => { - console.log('_subscribeBlockNumber', 'subscriptionId', subscriptionId); + + if (this._tokenregSub) { + this._tokenregSub.fetch(); + } }) .catch((error) => { console.warn('_subscribeBlockNumber', error); @@ -92,12 +93,19 @@ export default class Balances { .then((tokenregAddress) => { const tokenreg = this._api.newContract(abis.tokenreg, tokenregAddress); this._tokenreg = tokenreg; + this.attachToTokens(); return tokenreg; }); } _retrieveTokens () { + if (this._fetchedTokens) { + return this._retrieveBalances(); + } + + this._fetchedTokens = true; + this .getTokenRegistry() .then((tokenreg) => { @@ -113,25 +121,12 @@ export default class Balances { return Promise.all(promises); }); }) - .then((_tokens) => { - const prevHashes = Object.values(this._tokens).map((t) => t.hash).sort().join(''); - const nextHashes = _tokens.map((t) => t.hash).sort().join(''); - - if (prevHashes !== nextHashes) { - this._tokens = _tokens - .reduce((obj, token) => { - obj[token.address] = token; - return obj; - }, {}); - - this._store.dispatch(getTokens(this._tokens)); - } - + .then(() => { + this._store.dispatch(getTokens(this._tokens)); this._retrieveBalances(); }) .catch((error) => { - console.warn('_retrieveTokens', error); - this._retrieveBalances(); + console.warn('balances::_retrieveTokens', error); }); } @@ -157,39 +152,88 @@ export default class Balances { }); } + attachToTokens () { + if (this._tokenregSub) { + return; + } + + this._tokenreg + .instance + .MetaChanged + .register({ + fromBlock: 0, + toBlock: 'latest', + topics: [ null, this._api.util.asciiToHex('IMG') ] + }, (error, logs) => { + if (error) { + return console.error('balances::attachToToken', 'failed to attacht to token registry', error.toString(), error.stack); + } + + // In case multiple logs for same token + // in one block. Take the last value. + const tokens = logs + .filter((log) => log.type === 'mined') + .reduce((_tokens, log) => { + const id = log.params.id.value.toNumber(); + const image = log.params.value.value; + + const token = Object.values(this._tokens).find((c) => c.id === id); + const { address } = token; + + _tokens[address] = { address, id, image }; + return _tokens; + }, {}); + + Object + .values(tokens) + .forEach((token) => { + const { address, image } = token; + + if (this._images[address] !== image.toString()) { + this._store.dispatch(setAddressImage(address, image)); + this._images[address] = image.toString(); + } + }); + }) + .then((tokenregSub) => { + this._tokenregSub = tokenregSub; + }) + .catch((e) => { + console.warn('balances::attachToTokens', e); + }); + } + fetchTokenInfo (tokenreg, tokenId) { return Promise .all([ tokenreg.instance.token.call({}, [tokenId]), tokenreg.instance.meta.call({}, [tokenId, 'IMG']) ]) - .then(([ token, image ]) => { - const [ address, tag, format, name ] = token; - const oldToken = this._tokens[address]; + .then(([ tokenData, image ]) => { + const [ address, tag, format, name ] = tokenData; + const contract = this._api.newContract(abis.eip20, address); if (this._images[address] !== image.toString()) { this._store.dispatch(setAddressImage(address, image)); this._images[address] = image.toString(); } - const newToken = { + const token = { + format: format.toString(), + id: tokenId, + address, - name, tag, - format: format.toString() + name, + contract }; - const hash = this._api.util.sha3(JSON.stringify(newToken)); + this._tokens[address] = token; - const contract = oldToken - ? oldToken.contract - : this._api.newContract(abis.eip20, address); - - return { - ...newToken, - hash, - contract - }; + return token; + }) + .catch((e) => { + console.warn('balances::fetchTokenInfo', `couldn't fetch token #${tokenId}`, e); }); } From 3f332a0309a72c2f76ac9a43e9b63c506d830ef6 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Mon, 21 Nov 2016 10:37:02 +0100 Subject: [PATCH 04/11] Attaching to TokenReg Events instead of fetching for each block #3240 --- js/src/redux/providers/balances.js | 82 ++++++++++++++++++++++++++---- 1 file changed, 71 insertions(+), 11 deletions(-) diff --git a/js/src/redux/providers/balances.js b/js/src/redux/providers/balances.js index c13bf9778d4..f2b5b07545a 100644 --- a/js/src/redux/providers/balances.js +++ b/js/src/redux/providers/balances.js @@ -31,12 +31,16 @@ export default class Balances { constructor (store, api) { this._api = api; this._store = store; - this._accountsInfo = null; + this._tokens = {}; this._images = {}; + + this._accountsInfo = null; this._tokenreg = null; this._fetchedTokens = false; + this._tokenregSub = null; + this._tokenregMetaSub = null; } start () { @@ -64,14 +68,11 @@ export default class Balances { this._api .subscribe('eth_blockNumber', (error) => { if (error) { - return; + return console.warn('_subscribeBlockNumber', error); } this._retrieveTokens(); - - if (this._tokenregSub) { - this._tokenregSub.fetch(); - } + this.fetchTokensChanges(); }) .catch((error) => { console.warn('_subscribeBlockNumber', error); @@ -135,7 +136,14 @@ export default class Balances { return; } - const addresses = Object.keys(this._accountsInfo); + const addresses = Object + .keys(this._accountsInfo) + .filter((address) => { + const account = this._accountsInfo[address]; + return !account.meta || !account.meta.deleted; + }); + + console.warn(`fetching ${addresses.length} accounts`); this._balances = {}; Promise @@ -152,11 +160,57 @@ export default class Balances { }); } + fetchTokensChanges () { + if (this._tokenregMetaSub) { + this._tokenregMetaSub.fetch(); + } + + if (this._tokenregSub) { + this._tokenregSub.fetch(); + } + } + attachToTokens () { + this.attachToTokenMetaChange(); + this.attachToNewToken(); + } + + attachToNewToken () { if (this._tokenregSub) { return; } + this._tokenreg + .instance + .Registered + .register({ + fromBlock: 0, + toBlock: 'latest' + }, (error, logs) => { + if (error) { + return console.error('balances::attachToNewToken', 'failed to attacht to tokenreg Registered', error.toString(), error.stack); + } + + const promises = logs.map((log) => { + const id = log.params.id.value.toNumber(); + return this.fetchTokenInfo(this._tokenreg, id); + }); + + return Promise.all(promises); + }) + .then((tokenregSub) => { + this._tokenregSub = tokenregSub; + }) + .catch((e) => { + console.warn('balances::attachToNewToken', e); + }); + } + + attachToTokenMetaChange () { + if (this._tokenregMetaSub) { + return; + } + this._tokenreg .instance .MetaChanged @@ -166,7 +220,7 @@ export default class Balances { topics: [ null, this._api.util.asciiToHex('IMG') ] }, (error, logs) => { if (error) { - return console.error('balances::attachToToken', 'failed to attacht to token registry', error.toString(), error.stack); + return console.error('balances::attachToTokenMetaChange', 'failed to attacht to tokenreg MetaChanged', error.toString(), error.stack); } // In case multiple logs for same token @@ -195,11 +249,11 @@ export default class Balances { } }); }) - .then((tokenregSub) => { - this._tokenregSub = tokenregSub; + .then((tokenregMetaSub) => { + this._tokenregMetaSub = tokenregMetaSub; }) .catch((e) => { - console.warn('balances::attachToTokens', e); + console.warn('balances::attachToTokenMetaChange', e); }); } @@ -237,6 +291,12 @@ export default class Balances { }); } + /** + * TODO?: txCount is only shown on an address page, so we + * might not need to fetch it for each address for each block, + * but only for one address when the user is on the account + * view. + */ fetchAccountBalance (address) { const _tokens = Object.values(this._tokens); const tokensPromises = _tokens From 7db0ad1c2c50a0a0b3c174862eeaadec177b86e8 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Mon, 21 Nov 2016 12:26:36 +0100 Subject: [PATCH 05/11] Unsubscribe from shapeshit... #3240 --- js/src/3rdparty/shapeshift/shapeshift.js | 26 ++++++++++++++++++++---- js/src/modals/Shapeshift/shapeshift.js | 14 +++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/js/src/3rdparty/shapeshift/shapeshift.js b/js/src/3rdparty/shapeshift/shapeshift.js index 344a448022a..8f388d0a796 100644 --- a/js/src/3rdparty/shapeshift/shapeshift.js +++ b/js/src/3rdparty/shapeshift/shapeshift.js @@ -15,7 +15,8 @@ // along with Parity. If not, see . export default function (rpc) { - const subscriptions = []; + let subscriptions = []; + let pollStatusIntervalId = null; function getCoins () { return rpc.get('getcoins'); @@ -45,6 +46,24 @@ export default function (rpc) { callback, idx }); + + // Only poll if there are subscriptions... + if (!pollStatusIntervalId) { + pollStatusIntervalId = setInterval(_pollStatus, 2000); + } + } + + function unsubscribe (depositAddress) { + const newSubscriptions = [] + .concat(subscriptions) + .filter((sub) => sub.depositAddress !== depositAddress); + + subscriptions = newSubscriptions; + + if (subscriptions.length === 0) { + clearInterval(pollStatusIntervalId); + pollStatusIntervalId = null; + } } function _getSubscriptionStatus (subscription) { @@ -81,13 +100,12 @@ export default function (rpc) { subscriptions.forEach(_getSubscriptionStatus); } - setInterval(_pollStatus, 2000); - return { getCoins, getMarketInfo, getStatus, shift, - subscribe + subscribe, + unsubscribe }; } diff --git a/js/src/modals/Shapeshift/shapeshift.js b/js/src/modals/Shapeshift/shapeshift.js index ba3252398d3..11409d84881 100644 --- a/js/src/modals/Shapeshift/shapeshift.js +++ b/js/src/modals/Shapeshift/shapeshift.js @@ -63,6 +63,16 @@ export default class Shapeshift extends Component { this.retrieveCoins(); } + componentWillUnmount () { + this.unsubscribe(); + } + + unsubscribe () { + // Unsubscribe from Shapeshit + const { depositAddress } = this.state; + shapeshift.unsubscribe(depositAddress); + } + render () { const { error, stage } = this.state; @@ -205,6 +215,10 @@ export default class Shapeshift extends Component { console.log('onShift', result); const depositAddress = result.deposit; + if (this.state.depositAddress) { + this.unsubscribe(); + } + shapeshift.subscribe(depositAddress, this.onExchangeInfo); this.setState({ depositAddress }); }) From 2ec182a02ea04de73c2b85e66a851e6c9d96aa0d Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Mon, 21 Nov 2016 12:38:46 +0100 Subject: [PATCH 06/11] Unsubscribe from EthFilter if used once (resolved) #3240 --- js/src/contracts/sms-verification.js | 37 ++++++++++++++++++++-------- js/src/redux/providers/balances.js | 11 ++++++++- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/js/src/contracts/sms-verification.js b/js/src/contracts/sms-verification.js index e93d57ffc5d..98c86aea925 100644 --- a/js/src/contracts/sms-verification.js +++ b/js/src/contracts/sms-verification.js @@ -21,18 +21,35 @@ export const checkIfVerified = (contract, account) => { }; export const checkIfRequested = (contract, account) => { + let subId = null; + let resolved = false; + return new Promise((resolve, reject) => { - contract.subscribe('Requested', { - fromBlock: 0, toBlock: 'pending' - }, (err, logs) => { - if (err) { - return reject(err); - } - const e = logs.find((l) => { - return l.type === 'mined' && l.params.who && l.params.who.value === account; + contract + .subscribe('Requested', { + fromBlock: 0, toBlock: 'pending' + }, (err, logs) => { + if (err) { + return reject(err); + } + const e = logs.find((l) => { + return l.type === 'mined' && l.params.who && l.params.who.value === account; + }); + + resolve(e ? e.transactionHash : false); + resolved = true; + + if (subId) { + contract.unsubscribe(subId); + } + }) + .then((_subId) => { + subId = _subId; + + if (resolved) { + contract.unsubscribe(subId); + } }); - resolve(e ? e.transactionHash : false); - }); }); }; diff --git a/js/src/redux/providers/balances.js b/js/src/redux/providers/balances.js index f2b5b07545a..776eff2d887 100644 --- a/js/src/redux/providers/balances.js +++ b/js/src/redux/providers/balances.js @@ -37,6 +37,7 @@ export default class Balances { this._accountsInfo = null; this._tokenreg = null; + this._fetchingTokens = false; this._fetchedTokens = false; this._tokenregSub = null; @@ -101,11 +102,16 @@ export default class Balances { } _retrieveTokens () { + if (this._fetchingTokens) { + return; + } + if (this._fetchedTokens) { return this._retrieveBalances(); } - this._fetchedTokens = true; + this._fetchingTokens = true; + this._fetchedTokens = false; this .getTokenRegistry() @@ -123,6 +129,9 @@ export default class Balances { }); }) .then(() => { + this._fetchingTokens = false; + this._fetchedTokens = true; + this._store.dispatch(getTokens(this._tokens)); this._retrieveBalances(); }) From 54e28447d532e46de1ced766ffe6378b6440e3a1 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Mon, 21 Nov 2016 12:39:35 +0100 Subject: [PATCH 07/11] Remove warning --- js/src/redux/providers/balances.js | 1 - 1 file changed, 1 deletion(-) diff --git a/js/src/redux/providers/balances.js b/js/src/redux/providers/balances.js index 776eff2d887..158e260c819 100644 --- a/js/src/redux/providers/balances.js +++ b/js/src/redux/providers/balances.js @@ -152,7 +152,6 @@ export default class Balances { return !account.meta || !account.meta.deleted; }); - console.warn(`fetching ${addresses.length} accounts`); this._balances = {}; Promise From ef2c170fd0f7235c3a7dba738e18db285c2e9417 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Mon, 21 Nov 2016 19:15:00 +0100 Subject: [PATCH 08/11] PR review fix --- js/src/api/contract/contract.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/src/api/contract/contract.js b/js/src/api/contract/contract.js index 51cdc64d1dd..b3b45830b5a 100644 --- a/js/src/api/contract/contract.js +++ b/js/src/api/contract/contract.js @@ -341,7 +341,7 @@ export default class Contract { delete this._subscriptions[subscriptionId]; if (Object.keys(this._subscriptions).length === 0) { - this._unsubscribeToChanges(); + this._unsubscribeFromChanges(); } }) .catch((error) => { @@ -358,7 +358,7 @@ export default class Contract { this._sendSubscriptionChanges(); } - _unsubscribeToChanges = () => { + _unsubscribeFromChanges = () => { this._subscribedToChanges = false; clearTimeout(this._subscribedToChangesId); } From 5b5dc9be2bd11afffcec20b4496aee546da91c58 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Tue, 22 Nov 2016 09:44:58 +0100 Subject: [PATCH 09/11] Typo --- js/src/redux/providers/balances.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/src/redux/providers/balances.js b/js/src/redux/providers/balances.js index 158e260c819..9ecffc6037a 100644 --- a/js/src/redux/providers/balances.js +++ b/js/src/redux/providers/balances.js @@ -196,7 +196,7 @@ export default class Balances { toBlock: 'latest' }, (error, logs) => { if (error) { - return console.error('balances::attachToNewToken', 'failed to attacht to tokenreg Registered', error.toString(), error.stack); + return console.error('balances::attachToNewToken', 'failed to attach to tokenreg Registered', error.toString(), error.stack); } const promises = logs.map((log) => { @@ -228,7 +228,7 @@ export default class Balances { topics: [ null, this._api.util.asciiToHex('IMG') ] }, (error, logs) => { if (error) { - return console.error('balances::attachToTokenMetaChange', 'failed to attacht to tokenreg MetaChanged', error.toString(), error.stack); + return console.error('balances::attachToTokenMetaChange', 'failed to attach to tokenreg MetaChanged', error.toString(), error.stack); } // In case multiple logs for same token From 7f366002d984a5766e10b3c6a3b10ba41537e03b Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Tue, 22 Nov 2016 10:50:03 +0100 Subject: [PATCH 10/11] Better contract JS API : one subscribe for all #3546 --- js/src/api/contract/contract.js | 159 +++++++++++++++++------------ js/src/redux/providers/balances.js | 37 +++---- 2 files changed, 106 insertions(+), 90 deletions(-) diff --git a/js/src/api/contract/contract.js b/js/src/api/contract/contract.js index b3b45830b5a..87efcf975a2 100644 --- a/js/src/api/contract/contract.js +++ b/js/src/api/contract/contract.js @@ -48,8 +48,11 @@ export default class Contract { this._instance[fn.signature] = fn; }); - this._subscribedToChanges = false; - this._subscribedToChangesId = null; + this._subscribedToPendings = false; + this._pendingsSubscriptionId = null; + + this._subscribedToBlock = false; + this._blockSubscriptionId = null; } get address () { @@ -233,10 +236,6 @@ export default class Contract { return this._subscribe(event, options, callback); }; - event.register = (options = {}, callback) => { - return this._register(event, options, callback); - }; - event.unsubscribe = (subscriptionId) => { return this.unsubscribe(subscriptionId); }; @@ -244,34 +243,6 @@ export default class Contract { return event; } - _register (event = null, _options, callback) { - return this - ._createEthFilter(event, _options) - .then((filterId) => { - return { - fetch: () => { - return this._api.eth - .getFilterChanges(filterId) - .then((logs) => { - if (!logs || !logs.length) { - return; - } - - try { - callback(null, this.parseEventLogs(logs)); - } catch (error) { - callback(error); - } - }); - }, - - unsubscribe: () => { - return this._api.eth.uninstallFilter(filterId); - } - }; - }); - } - _findEvent (eventName = null) { const event = eventName ? this._events.find((evt) => evt.name === eventName) @@ -314,60 +285,117 @@ export default class Contract { _subscribe (event = null, _options, callback) { const subscriptionId = nextSubscriptionId++; + const { skipInitFetch } = _options; + delete _options['skipInitFetch']; return this ._createEthFilter(event, _options) .then((filterId) => { - return this._api.eth - .getFilterLogs(filterId) - .then((logs) => { - callback(null, this.parseEventLogs(logs)); - - this._subscriptions[subscriptionId] = { - callback, - filterId - }; - - this._subscribeToChanges(); - return subscriptionId; - }); + this._subscriptions[subscriptionId] = { + options: _options, + callback, + filterId + }; + + if (!skipInitFetch) { + this._api.eth + .getFilterLogs(filterId) + .then((logs) => { + callback(null, this.parseEventLogs(logs)); + }); + } + + this._subscribeToChanges(); + return subscriptionId; }); } unsubscribe (subscriptionId) { return this._api.eth .uninstallFilter(this._subscriptions[subscriptionId].filterId) - .then(() => { - delete this._subscriptions[subscriptionId]; - - if (Object.keys(this._subscriptions).length === 0) { - this._unsubscribeFromChanges(); - } - }) .catch((error) => { console.error('unsubscribe', error); + }) + .then(() => { + delete this._subscriptions[subscriptionId]; + this._unsubscribeFromChanges(); }); } _subscribeToChanges = () => { - if (this._subscribedToChanges) { - return; + const subscriptions = Object.values(this._subscriptions); + + const pendingSubscriptions = subscriptions + .filter((s) => s.options.toBlock && s.options.toBlock === 'pending'); + + const otherSubscriptions = subscriptions + .filter((s) => !(s.options.toBlock && s.options.toBlock === 'pending')); + + if (pendingSubscriptions.length > 0 && !this._subscribedToPendings) { + this._subscribedToPendings = true; + this._subscribeToPendings(); } - this._subscribedToChanges = true; - this._sendSubscriptionChanges(); + if (otherSubscriptions.length > 0 && !this._subscribedToBlock) { + this._subscribedToBlock = true; + this._subscribeToBlock(); + } } _unsubscribeFromChanges = () => { - this._subscribedToChanges = false; - clearTimeout(this._subscribedToChangesId); + const subscriptions = Object.values(this._subscriptions); + + const pendingSubscriptions = subscriptions + .filter((s) => s.options.toBlock && s.options.toBlock === 'pending'); + + const otherSubscriptions = subscriptions + .filter((s) => !(s.options.toBlock && s.options.toBlock === 'pending')); + + if (pendingSubscriptions.length === 0 && this._subscribedToPendings) { + this._subscribedToPendings = false; + clearTimeout(this._pendingsSubscriptionId); + } + + if (otherSubscriptions.length === 0 && this._subscribedToBlock) { + this._subscribedToBlock = false; + this._api.unsubscribe(this._blockSubscriptionId); + } } - _sendSubscriptionChanges = () => { - const subscriptions = Object.values(this._subscriptions); - const timeout = () => setTimeout(this._sendSubscriptionChanges, 1000); + _subscribeToBlock = () => { + this._api + .subscribe('eth_blockNumber', (error) => { + if (error) { + console.error('::_subscribeToBlock', error, error && error.stack); + } + + const subscriptions = Object.values(this._subscriptions) + .filter((s) => !(s.options.toBlock && s.options.toBlock === 'pending')); - Promise + this._sendSubscriptionChanges(subscriptions); + }) + .then((blockSubId) => { + this._blockSubscriptionId = blockSubId; + }) + .catch((e) => { + console.error('::_subscribeToBlock', e, e && e.stack); + }); + } + + _subscribeToPendings = () => { + const subscriptions = Object.values(this._subscriptions) + .filter((s) => s.options.toBlock && s.options.toBlock === 'pending'); + + const timeout = () => setTimeout(() => this._subscribeFromPendings(), 1000); + + this._sendSubscriptionChanges(subscriptions) + .then(() => { + this._pendingsSubscriptionId = timeout(); + }); + } + + _sendSubscriptionChanges = (subscriptions) => { + return Promise .all( subscriptions.map((subscription) => { return this._api.eth.getFilterChanges(subscription.filterId); @@ -385,12 +413,9 @@ export default class Contract { console.error('_sendSubscriptionChanges', error); } }); - - this._subscribedToChangesId = timeout(); }) .catch((error) => { console.error('_sendSubscriptionChanges', error); - this._subscribedToChangesId = timeout(); }); } } diff --git a/js/src/redux/providers/balances.js b/js/src/redux/providers/balances.js index 9ecffc6037a..bcc8eca0adb 100644 --- a/js/src/redux/providers/balances.js +++ b/js/src/redux/providers/balances.js @@ -40,8 +40,8 @@ export default class Balances { this._fetchingTokens = false; this._fetchedTokens = false; - this._tokenregSub = null; - this._tokenregMetaSub = null; + this._tokenregSubId = null; + this._tokenregMetaSubId = null; } start () { @@ -73,7 +73,6 @@ export default class Balances { } this._retrieveTokens(); - this.fetchTokensChanges(); }) .catch((error) => { console.warn('_subscribeBlockNumber', error); @@ -168,32 +167,23 @@ export default class Balances { }); } - fetchTokensChanges () { - if (this._tokenregMetaSub) { - this._tokenregMetaSub.fetch(); - } - - if (this._tokenregSub) { - this._tokenregSub.fetch(); - } - } - attachToTokens () { this.attachToTokenMetaChange(); this.attachToNewToken(); } attachToNewToken () { - if (this._tokenregSub) { + if (this._tokenregSubId) { return; } this._tokenreg .instance .Registered - .register({ + .subscribe({ fromBlock: 0, - toBlock: 'latest' + toBlock: 'latest', + skipInitFetch: true }, (error, logs) => { if (error) { return console.error('balances::attachToNewToken', 'failed to attach to tokenreg Registered', error.toString(), error.stack); @@ -206,8 +196,8 @@ export default class Balances { return Promise.all(promises); }) - .then((tokenregSub) => { - this._tokenregSub = tokenregSub; + .then((tokenregSubId) => { + this._tokenregSubId = tokenregSubId; }) .catch((e) => { console.warn('balances::attachToNewToken', e); @@ -215,17 +205,18 @@ export default class Balances { } attachToTokenMetaChange () { - if (this._tokenregMetaSub) { + if (this._tokenregMetaSubId) { return; } this._tokenreg .instance .MetaChanged - .register({ + .subscribe({ fromBlock: 0, toBlock: 'latest', - topics: [ null, this._api.util.asciiToHex('IMG') ] + topics: [ null, this._api.util.asciiToHex('IMG') ], + skipInitFetch: true }, (error, logs) => { if (error) { return console.error('balances::attachToTokenMetaChange', 'failed to attach to tokenreg MetaChanged', error.toString(), error.stack); @@ -257,8 +248,8 @@ export default class Balances { } }); }) - .then((tokenregMetaSub) => { - this._tokenregMetaSub = tokenregMetaSub; + .then((tokenregMetaSubId) => { + this._tokenregMetaSubId = tokenregMetaSubId; }) .catch((e) => { console.warn('balances::attachToTokenMetaChange', e); From f4f2c5e8ddf9c63ea93bf48ddc60b4ef3298a3a0 Mon Sep 17 00:00:00 2001 From: Nicolas Gotchac Date: Tue, 22 Nov 2016 18:43:31 +0100 Subject: [PATCH 11/11] Fixed test --- js/src/api/contract/contract.js | 19 +++++++++++-------- js/src/api/contract/contract.spec.js | 15 ++++++++++----- js/src/api/format/input.js | 6 +++--- js/test/mockRpc.js | 3 ++- 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/js/src/api/contract/contract.js b/js/src/api/contract/contract.js index 87efcf975a2..f7cd727f8cd 100644 --- a/js/src/api/contract/contract.js +++ b/js/src/api/contract/contract.js @@ -297,16 +297,19 @@ export default class Contract { filterId }; - if (!skipInitFetch) { - this._api.eth - .getFilterLogs(filterId) - .then((logs) => { - callback(null, this.parseEventLogs(logs)); - }); + if (skipInitFetch) { + this._subscribeToChanges(); + return subscriptionId; } - this._subscribeToChanges(); - return subscriptionId; + return this._api.eth + .getFilterLogs(filterId) + .then((logs) => { + callback(null, this.parseEventLogs(logs)); + + this._subscribeToChanges(); + return subscriptionId; + }); }); } diff --git a/js/src/api/contract/contract.spec.js b/js/src/api/contract/contract.spec.js index 9c08024a974..970dd606d9c 100644 --- a/js/src/api/contract/contract.spec.js +++ b/js/src/api/contract/contract.spec.js @@ -437,6 +437,7 @@ describe('api/contract/Contract', () => { ] } ]; + const logs = [{ address: '0x22bff18ec62281850546a664bb63a5c06ac5f76c', blockHash: '0xa9280530a3b47bee2fc80f2862fd56502ae075350571d724d6442ea4c597347b', @@ -450,6 +451,7 @@ describe('api/contract/Contract', () => { transactionHash: '0xca16f537d761d13e4e80953b754e2b15541f267d6cad9381f750af1bae1e4917', transactionIndex: '0x0' }]; + const parsed = [{ address: '0x22bfF18ec62281850546a664bb63a5C06AC5F76C', blockHash: '0xa9280530a3b47bee2fc80f2862fd56502ae075350571d724d6442ea4c597347b', @@ -466,11 +468,13 @@ describe('api/contract/Contract', () => { sender: { type: 'address', value: '0x63Cf90D3f0410092FC0fca41846f596223979195' } }, topics: [ - '0x954ba6c157daf8a26539574ffa64203c044691aa57251af95f4b48d85ec00dd5', '0x0000000000000000000000000000000000000000000000000001000000004fe0' + '0x954ba6c157daf8a26539574ffa64203c044691aa57251af95f4b48d85ec00dd5', + '0x0000000000000000000000000000000000000000000000000001000000004fe0' ], transactionHash: '0xca16f537d761d13e4e80953b754e2b15541f267d6cad9381f750af1bae1e4917', transactionIndex: new BigNumber(0) }]; + let contract; beforeEach(() => { @@ -496,18 +500,19 @@ describe('api/contract/Contract', () => { scope = mockHttp([ { method: 'eth_newFilter', reply: { result: '0x123' } }, { method: 'eth_getFilterLogs', reply: { result: logs } }, + { method: 'eth_getFilterChanges', reply: { result: logs } }, { method: 'eth_newFilter', reply: { result: '0x123' } }, { method: 'eth_getFilterLogs', reply: { result: logs } } ]); cbb = sinon.stub(); cbe = sinon.stub(); - return contract.subscribe('Message', {}, cbb); + return contract.subscribe('Message', { toBlock: 'pending' }, cbb); }); it('sets the subscriptionId returned', () => { return contract - .subscribe('Message', {}, cbe) + .subscribe('Message', { toBlock: 'pending' }, cbe) .then((subscriptionId) => { expect(subscriptionId).to.equal(1); }); @@ -515,7 +520,7 @@ describe('api/contract/Contract', () => { it('creates a new filter and retrieves the logs on it', () => { return contract - .subscribe('Message', {}, cbe) + .subscribe('Message', { toBlock: 'pending' }, cbe) .then((subscriptionId) => { expect(scope.isDone()).to.be.true; }); @@ -523,7 +528,7 @@ describe('api/contract/Contract', () => { it('returns the logs to the callback', () => { return contract - .subscribe('Message', {}, cbe) + .subscribe('Message', { toBlock: 'pending' }, cbe) .then((subscriptionId) => { expect(cbe).to.have.been.calledWith(null, parsed); }); diff --git a/js/src/api/format/input.js b/js/src/api/format/input.js index bd3d8ba854d..55c85e4f3c1 100644 --- a/js/src/api/format/input.js +++ b/js/src/api/format/input.js @@ -61,9 +61,9 @@ export function inTopics (_topics) { .filter((topic) => topic === null || topic) .map((topic) => topic === null ? null : pad(topic, 32)); - while (topics.length < 4) { - topics.push(null); - } + // while (topics.length < 4) { + // topics.push(null); + // } return topics; } diff --git a/js/test/mockRpc.js b/js/test/mockRpc.js index 800f483c9da..147c64af04a 100644 --- a/js/test/mockRpc.js +++ b/js/test/mockRpc.js @@ -23,9 +23,10 @@ export const TEST_HTTP_URL = 'http://localhost:6688'; export const TEST_WS_URL = 'ws://localhost:8866'; export function mockHttp (requests) { + nock.cleanAll(); let scope = nock(TEST_HTTP_URL); - requests.forEach((request) => { + requests.forEach((request, index) => { scope = scope .post('/') .reply(request.code || 200, (uri, body) => {