Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Peng/123 vuex tests #333

Merged
merged 20 commits into from
Jan 23, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions app/src/renderer/components/wallet/PageSend.vue
Original file line number Diff line number Diff line change
Expand Up @@ -105,16 +105,9 @@ export default {
await this.walletSend({
fees: { denom, amount: 0 },
to: address,
amount: [{ denom, amount }],
cb: (err) => {
amount: [{ denom, amount }]
}).then(() => {
this.sending = false
if (err) {
this.$store.commit('notifyError', {
title: 'Error Sending',
body: `An error occurred while trying to send: "${err.message}"`
})
return
}
this.$store.commit('notify', {
title: 'Successfully Sent',
body: `Successfully sent ${amount} ${denom.toUpperCase()} to ${address}`
Expand All @@ -125,7 +118,12 @@ export default {

// refreshes user transaction history
this.$store.dispatch('queryWalletHistory')
}
}, err => {
Copy link
Collaborator

@jbibla jbibla Jan 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is err => { ... } the same as .catch()?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but the then(_, onReject) will get called instead of .catchif available. See: https://codepen.io/faboweb/pen/WdmRoO

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool! thanks for the example. still not sure when to use one vs the other...?

this.sending = false
this.$store.commit('notifyError', {
title: 'Error Sending',
body: `An error occurred while trying to send: "${err.message}"`
})
})
},
...mapActions(['walletSend'])
Expand Down
23 changes: 23 additions & 0 deletions app/src/renderer/node.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict'
const RpcClient = require('tendermint')
const RestClient = require('cosmos-sdk')
const axios = require('axios')

const RELAY_SERVER = 'http://localhost:8999'

Expand Down Expand Up @@ -61,6 +62,28 @@ module.exports = function (nodeIP) {

node.rpcConnecting = false
return nodeIP
},
async sendTx (args, account, password) {
Copy link
Collaborator

@jbibla jbibla Jan 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this in node.js now?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this might not be the right module, although I understand why wallet might not have felt right either. Maybe this could just be its own tx module, with only this action and the queue of transactions plus the sequence number?

let tx = await (async function () {
switch (args.type) {
case 'buildDelegate': return axios.post('http://localhost:8998/build/stake/delegate', args)
.then(res => res.data)
case 'buildUnbond': return axios.post('http://localhost:8998/build/stake/unbond', args)
.then(res => res.data)
default: return node[args.type](args)
}
})()
let signedTx = await node.sign({
name: account,
password: password,
tx
})
let res = await node.postTx(signedTx)
Copy link
Contributor

@mappum mappum Jan 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's important that this goes through the same wallet sending code that it was separated from in this PR, to properly process multiple transactions serially and set the correct account sequence number on each transaction. Otherwise, transactions created here will only succeed if it is an account's first transaction.

// check response code
if (res.check_tx.code || res.deliver_tx.code) {
let message = res.check_tx.log || res.deliver_tx.log
throw new Error('Error sending transaction: ' + message)
}
}
})
// TODO: eventually, get all data from light-client connection instead of RPC
Expand Down
18 changes: 6 additions & 12 deletions app/src/renderer/vuex/modules/delegation.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,14 @@ export default ({ commit }) => {
await dispatch('walletTx', args)
},
async submitDelegation ({ state, dispatch }, delegation) {
for (let delegate of delegation.delegates) {
return Promise.all(delegation.delegates.map(delegate => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running these in parallel does not help since we have to sequentially increment the nonce and check that the tx went through. Really this should all happen in an atomic operation, but the LCD does not support that. Will open a ticket.

let candidateId = delegate.delegate.pub_key.data
let currentlyDelegated = state.committedDelegates[candidateId] || 0
let amountChange = delegate.atoms - currentlyDelegated
let action = amountChange > 0 ? 'walletDelegate' : 'walletUnbond'

// skip if no change
if (amountChange === 0) continue
if (amountChange === 0) return null

// bonding takes a 'coin' object, unbond just takes a number
let amount
Expand All @@ -84,17 +84,11 @@ export default ({ commit }) => {
amount = Math.abs(amountChange)
}

await new Promise((resolve, reject) => {
dispatch(action, {
amount,
pub_key: delegate.delegate.pub_key,
cb: (err, res) => {
if (err) return reject(err)
resolve(res)
}
})
return dispatch(action, {
amount,
pub_key: delegate.delegate.pub_key
})
}
}).filter(x => x !== null))
}
}

Expand Down
8 changes: 8 additions & 0 deletions app/src/renderer/vuex/modules/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export default function ({ node }) {

const state = {
nodeIP,
stopConnecting: false,
connected: false,
lastHeader: {
height: 0,
Expand All @@ -16,6 +17,9 @@ export default function ({ node }) {
}

const mutations = {
stopConnecting (state, stop) {
state.stopConnecting = stop
},
setConnected (state, connected) {
state.connected = connected
}
Expand All @@ -27,11 +31,15 @@ export default function ({ node }) {
dispatch('maybeUpdateValidators', header)
},
async reconnect ({commit, dispatch}) {
if (state.stopConnecting) return

commit('setConnected', false)
await node.rpcReconnect()
dispatch('nodeSubscribe')
},
nodeSubscribe ({commit, dispatch}) {
if (state.stopConnecting) return

// the rpc socket can be closed before we can even attach a listener
// so we remember if the connection is open
// we handle the reconnection here so we can attach all these listeners on reconnect
Expand Down
89 changes: 35 additions & 54 deletions app/src/renderer/vuex/modules/wallet.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
let fs = require('fs-extra')
let { join } = require('path')
let root = require('../../../root.js')
const axios = require('axios')

export default ({ commit, node }) => {
let state = {
Expand Down Expand Up @@ -39,7 +38,7 @@ export default ({ commit, node }) => {
state.sendQueue.push(sendReq)
},
shiftSendQueue (state) {
state.sendQueue = state.sendQueue.shift(1)
state.sendQueue = state.sendQueue.length > 1 ? state.sendQueue.shift(1) : []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, looks like I had a bug when I originally implemented this. This line should be state.sendQueue = state.sendQueue.slice(1), to shift off the first element (if any). (Also triggers a render since we're setting sendQueue to a different object.

},
setSending (state, sending) {
state.sending = sending
Expand Down Expand Up @@ -106,12 +105,12 @@ export default ({ commit, node }) => {
return blockMetaInfo
}
blockMetaInfo = await new Promise((resolve, reject) => {
node.rpc.blockchain({ minHeight: height, maxHeight: height }, (err, {block_metas}) => {
node.rpc.blockchain({ minHeight: height, maxHeight: height }, (err, data) => {
if (err) {
commit('notifyError', {title: `Couldn't query block`, body: err.message})
reject()
resolve(null)
} else {
resolve(block_metas[0])
resolve(data.block_metas[0])
}
})
})
Expand All @@ -125,34 +124,35 @@ export default ({ commit, node }) => {
app: 'sigs',
addr: args.to
}
await dispatch('walletTx', args)
return dispatch('walletTx', args)
},
async walletTx ({ state, dispatch, commit, rootState }, args) {
// wait until the current send operation is done
if (state.sending) {
commit('queueSend', args)
return
}
commit('setSending', true)
return new Promise(async (resolve, reject) => {
// wait until the current send operation is done
if (state.sending) {
commit('queueSend', args)
return
}
commit('setSending', true)

let cb = args.cb
delete args.cb
// once done, do next send in queue
function done (err, res) {
commit('setSending', false)

// once done, do next send in queue
function done (err, res) {
commit('setSending', false)
if (state.sendQueue.length > 0) {
// do next send
let send = state.sendQueue[0]
commit('shiftSendQueue')
dispatch('walletSend', send)
}

if (state.sendQueue.length > 0) {
// do next send
let send = state.sendQueue[0]
commit('shiftSendQueue')
dispatch('walletSend', send)
if (err) {
reject(err)
} else {
resolve(res)
}
}

cb(err, res)
}

try {
if (args.sequence == null) {
args.sequence = state.sequence + 1
}
Expand All @@ -161,35 +161,16 @@ export default ({ commit, node }) => {
app: 'sigs',
addr: state.key.address
}
// let tx = await node[args.type](args)
// TODO fix in cosmos-sdk-js
let tx = await (async function () {
switch (args.type) {
case 'buildDelegate': return axios.post('http://localhost:8998/build/stake/delegate', args)
.then(res => res.data)
case 'buildUnbond': return axios.post('http://localhost:8998/build/stake/unbond', args)
.then(res => res.data)
default: return node[args.type](args)
}
})()
let signedTx = await node.sign({
name: rootState.user.account,
password: rootState.user.password,
tx
})
let res = await node.postTx(signedTx)
// check response code
if (res.check_tx.code || res.deliver_tx.code) {
let message = res.check_tx.log || res.deliver_tx.log
return done(new Error('Error sending transaction: ' + message))
}

commit('setWalletSequence', args.sequence)
} catch (err) {
return done(err)
}
done(null, args)
dispatch('queryWalletBalances')
node.sendTx(args, rootState.user.account, rootState.user.password)
.then(() => {
commit('setWalletSequence', args.sequence)
done(null, args)
dispatch('queryWalletBalances')
}, err => {
done(err || Error('Sending TX failed'))
})
})
},
async loadDenoms ({ state, commit }) {
// read genesis.json to get default denoms
Expand Down
8 changes: 3 additions & 5 deletions test/unit/helpers/node_mock.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,11 @@ module.exports = {
seed_phrase: 'a b c d e f g h i j k l'
}),
queryAccount: () => null,
queryNonce: () => '123',
buildSend: (args) => {
if (args.to.addr.indexOf('fail') !== -1) return Promise.reject('Failed on purpose')
return Promise.resolve(null)
},
queryNonce: () => ({data: 123}),
buildSend: () => Promise.resolve(null),
coinTxs: () => Promise.resolve([]),
candidates: () => Promise.resolve({data: []}),
sendTx: () => Promise.resolve(),
postTx: () => Promise.resolve({
check_tx: { code: 0 },
deliver_tx: { code: 0 }
Expand Down
2 changes: 1 addition & 1 deletion test/unit/helpers/vuex-setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export default function vuexSetup () {
localVue.use(VueRouter)

function init (componentConstructor, testType = shallow, {stubs, getters = {}, propsData}) {
const node = require('../helpers/node_mock')
const node = Object.assign({}, require('../helpers/node_mock'))
const modules = Modules({
node
})
Expand Down
14 changes: 9 additions & 5 deletions test/unit/specs/PageSend.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,16 @@ import Vuelidate from 'vuelidate'
import PageSend from 'renderer/components/wallet/PageSend'

describe('PageSend', () => {
let wrapper, store
let wrapper, store, node

let {mount, localVue} = setup()
localVue.use(Vuelidate)

beforeEach(async () => {
let instance = mount(PageSend)
wrapper = instance.wrapper
store = instance.store
let test = mount(PageSend)
wrapper = test.wrapper
store = test.store
node = test.node
store.commit('setAccounts', [{
address: '1234567890123456789012345678901234567890',
name: 'default',
Expand Down Expand Up @@ -61,7 +62,10 @@ describe('PageSend', () => {
amount: 2
}
})
node.sendTx = () => Promise.reject()
await wrapper.vm.onSubmit()
expect(store.commit).toHaveBeenCalledWith('notifyError', expect.any(Object))
expect(store.state.notifications.length).toBe(1)
expect(store.state.notifications[0].title).toBe('Error Sending')
expect(store.state.notifications[0]).toMatchSnapshot()
})
})
11 changes: 11 additions & 0 deletions test/unit/specs/__snapshots__/PageSend.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -531,3 +531,14 @@ exports[`PageSend has the expected html structure 1`] = `
</main>
</div>
`;

exports[`PageSend should show notification for unsuccessful send 1`] = `
Object {
"body": "An error occurred while trying to send: \\"Sending TX failed\\"",
"icon": "error",
"layout": "alert",
"time": 1608,
"title": "Error Sending",
"type": "error",
}
`;
Loading