From 4a9dad7c40b97f1e625931d6b57fc9d7fdc5080d Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Thu, 11 Jan 2018 15:00:48 -0800 Subject: [PATCH 1/4] Improve gas price estimation by backfilling recent-blocks When first initializing, recent-block controller now back-fills up to its desired history length. This makes estimated gas prices reflect a longer recent history, even when first switching to a new network. Fixes #2925 --- CHANGELOG.md | 2 + app/scripts/controllers/recent-blocks.js | 79 +++++++++++++++++++++--- app/scripts/metamask-controller.js | 10 +-- 3 files changed, 80 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b4218210..30ddb3531 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Master +- Further improve gas price estimation. + ## 3.13.4 2018-1-9 - Remove recipient field if application initializes a tx with an empty string, or 0x, and tx data. Throw an error with the same condition, but without tx data. diff --git a/app/scripts/controllers/recent-blocks.js b/app/scripts/controllers/recent-blocks.js index 4a906261e..c65c2b1c4 100644 --- a/app/scripts/controllers/recent-blocks.js +++ b/app/scripts/controllers/recent-blocks.js @@ -1,11 +1,13 @@ const ObservableStore = require('obs-store') const extend = require('xtend') +const BN = require('ethereumjs-util').BN class RecentBlocksController { constructor (opts = {}) { - const { blockTracker } = opts + const { blockTracker, ethQuery } = opts this.blockTracker = blockTracker + this.ethQuery = ethQuery this.historyLength = opts.historyLength || 40 const initState = extend({ @@ -14,6 +16,7 @@ class RecentBlocksController { this.store = new ObservableStore(initState) this.blockTracker.on('block', this.processBlock.bind(this)) + this.backfill() } resetState () { @@ -23,12 +26,7 @@ class RecentBlocksController { } processBlock (newBlock) { - const block = extend(newBlock, { - gasPrices: newBlock.transactions.map((tx) => { - return tx.gasPrice - }), - }) - delete block.transactions + const block = this.mapTransactionsToPrices(newBlock) const state = this.store.getState() state.recentBlocks.push(block) @@ -39,6 +37,73 @@ class RecentBlocksController { this.store.updateState(state) } + + backfillBlock (newBlock) { + const block = this.mapTransactionsToPrices(newBlock) + + const state = this.store.getState() + + if (state.recentBlocks.length < this.historyLength) { + state.recentBlocks.unshift(block) + } + + this.store.updateState(state) + } + + mapTransactionsToPrices (newBlock) { + const block = extend(newBlock, { + gasPrices: newBlock.transactions.map((tx) => { + return tx.gasPrice + }), + }) + delete block.transactions + return block + } + + async backfill() { + this.blockTracker.once('block', async (block) => { + let blockNum = block.number + let recentBlocks + let state = this.store.getState() + recentBlocks = state.recentBlocks + + while (recentBlocks.length < this.historyLength) { + try { + let blockNumBn = new BN(blockNum.substr(2), 16) + const newNum = blockNumBn.subn(1).toString(10) + const newBlock = await this.getBlockByNumber(newNum) + + if (newBlock) { + this.backfillBlock(newBlock) + blockNum = newBlock.number + } + + state = this.store.getState() + recentBlocks = state.recentBlocks + } catch (e) { + log.error(e) + } + await this.wait() + } + }) + } + + async wait () { + return new Promise((resolve) => { + setTimeout(resolve, 100) + }) + } + + async getBlockByNumber (number) { + const bn = new BN(number) + return new Promise((resolve, reject) => { + this.ethQuery.getBlockByNumber('0x' + bn.toString(16), true, (err, block) => { + if (err) reject(err) + resolve(block) + }) + }) + } + } module.exports = RecentBlocksController diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index f62b5e5cd..81d70797a 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -94,12 +94,14 @@ module.exports = class MetamaskController extends EventEmitter { this.provider = this.initializeProvider() this.blockTracker = this.provider._blockTracker - this.recentBlocksController = new RecentBlocksController({ - blockTracker: this.blockTracker, - }) - // eth data query tools this.ethQuery = new EthQuery(this.provider) + + this.recentBlocksController = new RecentBlocksController({ + blockTracker: this.blockTracker, + ethQuery: this.ethQuery, + }) + // account tracker watches balances, nonces, and any code at their address. this.accountTracker = new AccountTracker({ provider: this.provider, From 7cb66ce4cba446f8149c4a8872dbdfbc53f72f7c Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Fri, 12 Jan 2018 10:25:36 -0800 Subject: [PATCH 2/4] Prefer passing a provider over an ethQuery instance --- app/scripts/controllers/recent-blocks.js | 5 +++-- app/scripts/metamask-controller.js | 7 +------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/app/scripts/controllers/recent-blocks.js b/app/scripts/controllers/recent-blocks.js index c65c2b1c4..4ae3810eb 100644 --- a/app/scripts/controllers/recent-blocks.js +++ b/app/scripts/controllers/recent-blocks.js @@ -1,13 +1,14 @@ const ObservableStore = require('obs-store') const extend = require('xtend') const BN = require('ethereumjs-util').BN +const EthQuery = require('eth-query') class RecentBlocksController { constructor (opts = {}) { - const { blockTracker, ethQuery } = opts + const { blockTracker, provider } = opts this.blockTracker = blockTracker - this.ethQuery = ethQuery + this.ethQuery = new EthQuery(provider) this.historyLength = opts.historyLength || 40 const initState = extend({ diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 81d70797a..000e17b9e 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -5,7 +5,6 @@ const Dnode = require('dnode') const ObservableStore = require('obs-store') const asStream = require('obs-store/lib/asStream') const AccountTracker = require('./lib/account-tracker') -const EthQuery = require('eth-query') const RpcEngine = require('json-rpc-engine') const debounce = require('debounce') const createEngineStream = require('json-rpc-middleware-stream/engineStream') @@ -94,12 +93,9 @@ module.exports = class MetamaskController extends EventEmitter { this.provider = this.initializeProvider() this.blockTracker = this.provider._blockTracker - // eth data query tools - this.ethQuery = new EthQuery(this.provider) - this.recentBlocksController = new RecentBlocksController({ blockTracker: this.blockTracker, - ethQuery: this.ethQuery, + provider: this.provider, }) // account tracker watches balances, nonces, and any code at their address. @@ -142,7 +138,6 @@ module.exports = class MetamaskController extends EventEmitter { signTransaction: this.keyringController.signTransaction.bind(this.keyringController), provider: this.provider, blockTracker: this.blockTracker, - ethQuery: this.ethQuery, getGasPrice: this.getGasPrice.bind(this), }) this.txController.on('newUnapprovedTx', opts.showUnapprovedTx.bind(opts)) From 6df3261debfa63dc7c667d8051000260dbde9570 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Sat, 13 Jan 2018 13:38:55 -0800 Subject: [PATCH 3/4] Fix signTypedData bytes signing Fixes #2826 --- CHANGELOG.md | 2 ++ package.json | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b4218210..e90134613 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Current Master +- Fix bug that prevented eth_signTypedData from signing bytes. + ## 3.13.4 2018-1-9 - Remove recipient field if application initializes a tx with an empty string, or 0x, and tx data. Throw an error with the same condition, but without tx data. diff --git a/package.json b/package.json index 8b846803b..fcccfd4ef 100644 --- a/package.json +++ b/package.json @@ -78,7 +78,7 @@ "eth-keyring-controller": "^2.1.2", "eth-phishing-detect": "^1.1.4", "eth-query": "^2.1.2", - "eth-sig-util": "^1.4.0", + "eth-sig-util": "^1.4.2", "eth-simple-keyring": "^1.2.0", "eth-token-tracker": "^1.1.4", "ethereumjs-tx": "^1.3.0", From 93d4b223631802d05830c2ce60e5d4e303330429 Mon Sep 17 00:00:00 2001 From: Dan Finlay Date: Mon, 15 Jan 2018 14:11:59 -0800 Subject: [PATCH 4/4] Bump keyringController version --- package.json | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/package.json b/package.json index fcccfd4ef..fa83c980e 100644 --- a/package.json +++ b/package.json @@ -72,14 +72,12 @@ "eth-bin-to-ops": "^1.0.1", "eth-block-tracker": "^2.2.0", "eth-contract-metadata": "^1.1.4", - "eth-hd-keyring": "^1.2.1", "eth-json-rpc-filters": "^1.2.5", "eth-json-rpc-infura": "^1.0.2", - "eth-keyring-controller": "^2.1.2", + "eth-keyring-controller": "^2.1.4", "eth-phishing-detect": "^1.1.4", "eth-query": "^2.1.2", "eth-sig-util": "^1.4.2", - "eth-simple-keyring": "^1.2.0", "eth-token-tracker": "^1.1.4", "ethereumjs-tx": "^1.3.0", "ethereumjs-util": "github:ethereumjs/ethereumjs-util#ac5d0908536b447083ea422b435da27f26615de9",