From 1dce352523a1f79f052ef79c707d9bc44787b706 Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 30 Mar 2017 14:23:23 -0700 Subject: [PATCH 1/3] tx-manager - add eip155 support --- app/scripts/transaction-manager.js | 20 +++++-- test/unit/tx-manager-test.js | 89 +++++++++++++++++++----------- test/unit/tx-utils-test.js | 17 ++++++ 3 files changed, 91 insertions(+), 35 deletions(-) diff --git a/app/scripts/transaction-manager.js b/app/scripts/transaction-manager.js index a70159680..d7051b2cb 100644 --- a/app/scripts/transaction-manager.js +++ b/app/scripts/transaction-manager.js @@ -205,11 +205,23 @@ module.exports = class TransactionManager extends EventEmitter { }) } + getChainId() { + const networkState = this.networkStore.getState() + const getChainId = parseInt(networkState.network) + if (Number.isNaN(getChainId)) { + return 0 + } else { + return getChainId + } + } + signTransaction (txId, cb) { - let txMeta = this.getTx(txId) - let txParams = txMeta.txParams - let fromAddress = txParams.from - let ethTx = this.txProviderUtils.buildEthTxFromParams(txParams) + const txMeta = this.getTx(txId) + const txParams = txMeta.txParams + const fromAddress = txParams.from + // add network/chain id + txParams.chainId = this.getChainId() + const ethTx = this.txProviderUtils.buildEthTxFromParams(txParams) this.signEthTx(ethTx, fromAddress).then(() => { this.setTxStatusSigned(txMeta.id) cb(null, ethUtil.bufferToHex(ethTx.serialize())) diff --git a/test/unit/tx-manager-test.js b/test/unit/tx-manager-test.js index d4bffdd9b..20f89a226 100644 --- a/test/unit/tx-manager-test.js +++ b/test/unit/tx-manager-test.js @@ -1,19 +1,30 @@ -const assert = require('assert') +// const assert = require('assert') +const assert = require('chai').assert const extend = require('xtend') const EventEmitter = require('events') +const ethUtil = require('ethereumjs-util') +const EthTx = require('ethereumjs-tx') const ObservableStore = require('obs-store') const STORAGE_KEY = 'metamask-persistance-key' const TransactionManager = require('../../app/scripts/transaction-manager') const noop = () => true +const currentNetworkId = 42 +const otherNetworkId = 36 +const privKey = new Buffer('8718b9618a37d1fc78c436511fc6df3c8258d3250635bba617f33003270ec03e', 'hex') describe('Transaction Manager', function() { let txManager beforeEach(function() { txManager = new TransactionManager({ - networkStore: new ObservableStore({ network: 'unit test' }), + networkStore: new ObservableStore({ network: currentNetworkId }), txHistoryLimit: 10, blockTracker: new EventEmitter(), + signTransaction: (ethTx) => new Promise((resolve) => { + ethTx.sign(privKey) + const result = ethTx.serialize() + resolve() + }) }) }) @@ -27,7 +38,6 @@ describe('Transaction Manager', function() { }) }) - it('returns error for negative values', function() { var sample = { value: '-0x01' @@ -51,7 +61,7 @@ describe('Transaction Manager', function() { describe('#addTx', function() { it('adds a tx returned in getTxList', function() { - var tx = { id: 1, status: 'confirmed', metamaskNetworkId: 'unit test', txParams: {} } + var tx = { id: 1, status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} } txManager.addTx(tx, noop) var result = txManager.getTxList() assert.ok(Array.isArray(result)) @@ -60,8 +70,8 @@ describe('Transaction Manager', function() { }) it('does not override txs from other networks', function() { - var tx = { id: 1, status: 'confirmed', metamaskNetworkId: 'unit test', txParams: {} } - var tx2 = { id: 2, status: 'confirmed', metamaskNetworkId: 'another net', txParams: {} } + var tx = { id: 1, status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} } + var tx2 = { id: 2, status: 'confirmed', metamaskNetworkId: otherNetworkId, txParams: {} } txManager.addTx(tx, noop) txManager.addTx(tx2, noop) var result = txManager.getFullTxList() @@ -73,7 +83,7 @@ describe('Transaction Manager', function() { it('cuts off early txs beyond a limit', function() { const limit = txManager.txHistoryLimit for (let i = 0; i < limit + 1; i++) { - let tx = { id: i, time: new Date(), status: 'confirmed', metamaskNetworkId: 'unit test', txParams: {} } + let tx = { id: i, time: new Date(), status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} } txManager.addTx(tx, noop) } var result = txManager.getTxList() @@ -84,7 +94,7 @@ describe('Transaction Manager', function() { it('cuts off early txs beyond a limit whether or not it is confirmed or rejected', function() { const limit = txManager.txHistoryLimit for (let i = 0; i < limit + 1; i++) { - let tx = { id: i, time: new Date(), status: 'rejected', metamaskNetworkId: 'unit test', txParams: {} } + let tx = { id: i, time: new Date(), status: 'rejected', metamaskNetworkId: currentNetworkId, txParams: {} } txManager.addTx(tx, noop) } var result = txManager.getTxList() @@ -93,11 +103,11 @@ describe('Transaction Manager', function() { }) it('cuts off early txs beyond a limit but does not cut unapproved txs', function() { - var unconfirmedTx = { id: 0, time: new Date(), status: 'unapproved', metamaskNetworkId: 'unit test', txParams: {} } + var unconfirmedTx = { id: 0, time: new Date(), status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } txManager.addTx(unconfirmedTx, noop) const limit = txManager.txHistoryLimit for (let i = 1; i < limit + 1; i++) { - let tx = { id: i, time: new Date(), status: 'confirmed', metamaskNetworkId: 'unit test', txParams: {} } + let tx = { id: i, time: new Date(), status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} } txManager.addTx(tx, noop) } var result = txManager.getTxList() @@ -110,7 +120,7 @@ describe('Transaction Manager', function() { describe('#setTxStatusSigned', function() { it('sets the tx status to signed', function() { - var tx = { id: 1, status: 'unapproved', metamaskNetworkId: 'unit test', txParams: {} } + var tx = { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } txManager.addTx(tx, noop) txManager.setTxStatusSigned(1) var result = txManager.getTxList() @@ -121,7 +131,7 @@ describe('Transaction Manager', function() { it('should emit a signed event to signal the exciton of callback', (done) => { this.timeout(10000) - var tx = { id: 1, status: 'unapproved', metamaskNetworkId: 'unit test', txParams: {} } + var tx = { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } let noop = function () { assert(true, 'event listener has been triggered and noop executed') done() @@ -134,7 +144,7 @@ describe('Transaction Manager', function() { describe('#setTxStatusRejected', function() { it('sets the tx status to rejected', function() { - var tx = { id: 1, status: 'unapproved', metamaskNetworkId: 'unit test', txParams: {} } + var tx = { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } txManager.addTx(tx) txManager.setTxStatusRejected(1) var result = txManager.getTxList() @@ -145,7 +155,7 @@ describe('Transaction Manager', function() { it('should emit a rejected event to signal the exciton of callback', (done) => { this.timeout(10000) - var tx = { id: 1, status: 'unapproved', metamaskNetworkId: 'unit test', txParams: {} } + var tx = { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} } txManager.addTx(tx) let noop = function (err, txId) { assert(true, 'event listener has been triggered and noop executed') @@ -159,9 +169,9 @@ describe('Transaction Manager', function() { describe('#updateTx', function() { it('replaces the tx with the same id', function() { - txManager.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: 'unit test', txParams: {} }, noop) - txManager.addTx({ id: '2', status: 'confirmed', metamaskNetworkId: 'unit test', txParams: {} }, noop) - txManager.updateTx({ id: '1', status: 'blah', hash: 'foo', metamaskNetworkId: 'unit test', txParams: {} }) + txManager.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) + txManager.addTx({ id: '2', status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) + txManager.updateTx({ id: '1', status: 'blah', hash: 'foo', metamaskNetworkId: currentNetworkId, txParams: {} }) var result = txManager.getTx('1') assert.equal(result.hash, 'foo') }) @@ -169,8 +179,8 @@ describe('Transaction Manager', function() { describe('#getUnapprovedTxList', function() { it('returns unapproved txs in a hash', function() { - txManager.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: 'unit test', txParams: {} }, noop) - txManager.addTx({ id: '2', status: 'confirmed', metamaskNetworkId: 'unit test', txParams: {} }, noop) + txManager.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) + txManager.addTx({ id: '2', status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) let result = txManager.getUnapprovedTxList() assert.equal(typeof result, 'object') assert.equal(result['1'].status, 'unapproved') @@ -180,8 +190,8 @@ describe('Transaction Manager', function() { describe('#getTx', function() { it('returns a tx with the requested id', function() { - txManager.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: 'unit test', txParams: {} }, noop) - txManager.addTx({ id: '2', status: 'confirmed', metamaskNetworkId: 'unit test', txParams: {} }, noop) + txManager.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) + txManager.addTx({ id: '2', status: 'confirmed', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) assert.equal(txManager.getTx('1').status, 'unapproved') assert.equal(txManager.getTx('2').status, 'confirmed') }) @@ -190,16 +200,16 @@ describe('Transaction Manager', function() { describe('#getFilteredTxList', function() { it('returns a tx with the requested data', function() { let txMetas = [ - { id: 0, status: 'unapproved', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: 'unit test' }, - { id: 1, status: 'unapproved', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: 'unit test' }, - { id: 2, status: 'unapproved', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: 'unit test' }, - { id: 3, status: 'unapproved', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: 'unit test' }, - { id: 4, status: 'unapproved', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: 'unit test' }, - { id: 5, status: 'confirmed', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: 'unit test' }, - { id: 6, status: 'confirmed', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: 'unit test' }, - { id: 7, status: 'confirmed', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: 'unit test' }, - { id: 8, status: 'confirmed', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: 'unit test' }, - { id: 9, status: 'confirmed', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: 'unit test' }, + { id: 0, status: 'unapproved', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: currentNetworkId }, + { id: 1, status: 'unapproved', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: currentNetworkId }, + { id: 2, status: 'unapproved', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: currentNetworkId }, + { id: 3, status: 'unapproved', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: currentNetworkId }, + { id: 4, status: 'unapproved', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: currentNetworkId }, + { id: 5, status: 'confirmed', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: currentNetworkId }, + { id: 6, status: 'confirmed', txParams: { from: '0xaa', to: '0xbb' }, metamaskNetworkId: currentNetworkId }, + { id: 7, status: 'confirmed', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: currentNetworkId }, + { id: 8, status: 'confirmed', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: currentNetworkId }, + { id: 9, status: 'confirmed', txParams: { from: '0xbb', to: '0xaa' }, metamaskNetworkId: currentNetworkId }, ] txMetas.forEach((txMeta) => txManager.addTx(txMeta, noop)) let filterParams @@ -219,4 +229,21 @@ describe('Transaction Manager', function() { }) }) + + describe('#sign replay-protected tx', function() { + it('prepares a tx with the chainId set', function() { + txManager.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) + txManager.signTransaction('1', (err, rawTx) => { + if (err) return assert.fail('it should not fail') + const ethTx = new EthTx(ethUtil.toBuffer(rawTx)) + console.log('------------------------------------------') + console.log('ethTx.getChainId(), currentNetworkId') + console.log(ethTx.getChainId(), currentNetworkId) + assert.equal(ethTx.getChainId(), currentNetworkId) + }) + }) + }) + + + }) diff --git a/test/unit/tx-utils-test.js b/test/unit/tx-utils-test.js index 8b4d2f059..93e9e4134 100644 --- a/test/unit/tx-utils-test.js +++ b/test/unit/tx-utils-test.js @@ -12,6 +12,23 @@ describe('txUtils', function() { txUtils = new TxUtils() }) + describe('chain Id', function() { + it('prepares a transaction with the provided chainId', function() { + const txParams = { + to: '0x70ad465e0bab6504002ad58c744ed89c7da38524', + from: '0x69ad465e0bab6504002ad58c744ed89c7da38525', + value: '0x0', + gas: '0x7b0c', + gasPrice: '0x199c82cc00', + data: '0x', + nonce: '0x3', + chainId: 42, + } + const ethTx = txUtils.buildEthTxFromParams(txParams) + assert.equal(ethTx.getChainId(), 42, 'chainId is set from tx params') + }) + }) + describe('addGasBuffer', function() { it('multiplies by 1.5, when within block gas limit', function() { // naive estimatedGas: 0x16e360 (1.5 mil) From 12918e18942a72223d7d97334934c31442e51caf Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 30 Mar 2017 16:06:27 -0700 Subject: [PATCH 2/3] tests - tx-manager - fix assert and clean formatting --- package.json | 2 +- test/unit/tx-manager-test.js | 10 +--------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index 9e04bc6f7..ba6a81da9 100644 --- a/package.json +++ b/package.json @@ -56,7 +56,7 @@ "eth-query": "^1.0.3", "eth-sig-util": "^1.1.1", "eth-simple-keyring": "^1.1.1", - "ethereumjs-tx": "^1.0.0", + "ethereumjs-tx": "^1.2.5", "ethereumjs-util": "ethereumjs/ethereumjs-util#ac5d0908536b447083ea422b435da27f26615de9", "ethereumjs-wallet": "^0.6.0", "ethjs-ens": "^1.0.2", diff --git a/test/unit/tx-manager-test.js b/test/unit/tx-manager-test.js index 20f89a226..21e94357b 100644 --- a/test/unit/tx-manager-test.js +++ b/test/unit/tx-manager-test.js @@ -1,5 +1,4 @@ -// const assert = require('assert') -const assert = require('chai').assert +const assert = require('assert') const extend = require('xtend') const EventEmitter = require('events') const ethUtil = require('ethereumjs-util') @@ -22,7 +21,6 @@ describe('Transaction Manager', function() { blockTracker: new EventEmitter(), signTransaction: (ethTx) => new Promise((resolve) => { ethTx.sign(privKey) - const result = ethTx.serialize() resolve() }) }) @@ -229,21 +227,15 @@ describe('Transaction Manager', function() { }) }) - describe('#sign replay-protected tx', function() { it('prepares a tx with the chainId set', function() { txManager.addTx({ id: '1', status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }, noop) txManager.signTransaction('1', (err, rawTx) => { if (err) return assert.fail('it should not fail') const ethTx = new EthTx(ethUtil.toBuffer(rawTx)) - console.log('------------------------------------------') - console.log('ethTx.getChainId(), currentNetworkId') - console.log(ethTx.getChainId(), currentNetworkId) assert.equal(ethTx.getChainId(), currentNetworkId) }) }) }) - - }) From 965c806a45bfc52f66de38a727a87dcde25f5607 Mon Sep 17 00:00:00 2001 From: kumavis Date: Thu, 30 Mar 2017 19:26:01 -0700 Subject: [PATCH 3/3] test - fix notice-controller test --- test/unit/notice-controller-test.js | 33 ++++++++++++++++------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/test/unit/notice-controller-test.js b/test/unit/notice-controller-test.js index 73fdb2f2e..ea37108bb 100644 --- a/test/unit/notice-controller-test.js +++ b/test/unit/notice-controller-test.js @@ -11,8 +11,6 @@ describe('notice-controller', function() { beforeEach(function() { // simple localStorage polyfill - window.localStorage = {} - if (window.localStorage.clear) window.localStorage.clear() let configManager = configManagerGen() noticeController = new NoticeController({ configManager: configManager, @@ -21,7 +19,7 @@ describe('notice-controller', function() { describe('notices', function() { describe('#getNoticesList', function() { - it('should return an empty array when new', function() { + it('should return an empty array when new', function(done) { var testList = [{ id:0, read:false, @@ -29,11 +27,12 @@ describe('notice-controller', function() { }] var result = noticeController.getNoticesList() assert.equal(result.length, 0) + done() }) }) describe('#setNoticesList', function() { - it('should set data appropriately', function () { + it('should set data appropriately', function (done) { var testList = [{ id:0, read:false, @@ -42,11 +41,12 @@ describe('notice-controller', function() { noticeController.setNoticesList(testList) var testListId = noticeController.getNoticesList()[0].id assert.equal(testListId, 0) + done() }) }) describe('#updateNoticeslist', function() { - it('should integrate the latest changes from the source', function() { + it('should integrate the latest changes from the source', function(done) { var testList = [{ id:55, read:false, @@ -57,26 +57,26 @@ describe('notice-controller', function() { var newList = noticeController.getNoticesList() assert.ok(newList[0].id === 55) assert.ok(newList[1]) + done() }) }) - it('should not overwrite any existing fields', function () { + it('should not overwrite any existing fields', function (done) { var testList = [{ id:0, read:false, title:"Futuristic Notice" }] noticeController.setNoticesList(testList) - noticeController.updateNoticesList().then(() => { - var newList = noticeController.getNoticesList() - assert.equal(newList[0].id, 0) - assert.equal(newList[0].title, "Futuristic Notice") - assert.equal(newList.length, 1) - }) + var newList = noticeController.getNoticesList() + assert.equal(newList[0].id, 0) + assert.equal(newList[0].title, "Futuristic Notice") + assert.equal(newList.length, 1) + done() }) }) describe('#markNoticeRead', function () { - it('should mark a notice as read', function () { + it('should mark a notice as read', function (done) { var testList = [{ id:0, read:false, @@ -86,11 +86,12 @@ describe('notice-controller', function() { noticeController.markNoticeRead(testList[0]) var newList = noticeController.getNoticesList() assert.ok(newList[0].read) + done() }) }) describe('#getLatestUnreadNotice', function () { - it('should retrieve the latest unread notice', function () { + it('should retrieve the latest unread notice', function (done) { var testList = [ {id:0,read:true,title:"Past Notice"}, {id:1,read:false,title:"Current Notice"}, @@ -99,8 +100,9 @@ describe('notice-controller', function() { noticeController.setNoticesList(testList) var latestUnread = noticeController.getLatestUnreadNotice() assert.equal(latestUnread.id, 2) + done() }) - it('should return undefined if no unread notices exist.', function () { + it('should return undefined if no unread notices exist.', function (done) { var testList = [ {id:0,read:true,title:"Past Notice"}, {id:1,read:true,title:"Current Notice"}, @@ -109,6 +111,7 @@ describe('notice-controller', function() { noticeController.setNoticesList(testList) var latestUnread = noticeController.getLatestUnreadNotice() assert.ok(!latestUnread) + done() }) }) })