From e47836ed3d71068df0d8115db2c188d5d944df91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Powaga?= Date: Mon, 1 Apr 2019 13:33:57 +0100 Subject: [PATCH] Improve channel tests and error handling (#276) * Make sure that sign function is correctly called * Improve error handling for update method --- es/channel/handlers.js | 17 ++++- test/integration/channel.js | 138 ++++++++++++++++++++++++++---------- 2 files changed, 117 insertions(+), 38 deletions(-) diff --git a/es/channel/handlers.js b/es/channel/handlers.js index c94f06cf57..6ca2af586f 100644 --- a/es/channel/handlers.js +++ b/es/channel/handlers.js @@ -50,7 +50,7 @@ export async function awaitingChannelCreateTx (channel, message, state) { responder: 'responder_sign' }[options.get(channel).role] if (message.method === `channels.sign.${tag}`) { - const signedTx = await options.get(channel).sign(message.tag, message.params.data.tx) + const signedTx = await options.get(channel).sign(tag, message.params.data.tx) send(channel, { jsonrpc: '2.0', method: `channels.${tag}`, params: { tx: signedTx } }) return { handler: awaitingOnChainTx } } @@ -145,6 +145,17 @@ export async function awaitingOffChainTx (channel, message, state) { state.reject(new Error(message.data.message)) return { handler: channelOpen } } + if (message.error) { + const { data = [] } = message.error + if (data.find(i => i.code === 1001)) { + state.reject(new Error('Insufficient balance')) + } else if (data.find(i => i.code === 1002)) { + state.reject(new Error('Amount cannot be negative')) + } else { + state.reject(new Error(message.error.message)) + } + return { handler: channelOpen } + } } export function awaitingOffChainUpdate (channel, message, state) { @@ -157,6 +168,10 @@ export function awaitingOffChainUpdate (channel, message, state) { state.resolve({ accepted: false }) return { handler: channelOpen } } + if (message.error) { + state.reject(new Error(message.error.message)) + return { handler: channelOpen } + } } export async function awaitingTxSignRequest (channel, message, state) { diff --git a/test/integration/channel.js b/test/integration/channel.js index 2ecdda65bd..931e6e5a96 100644 --- a/test/integration/channel.js +++ b/test/integration/channel.js @@ -16,7 +16,7 @@ */ import { describe, it, before } from 'mocha' -import { spy } from 'sinon' +import * as sinon from 'sinon' import { configure, ready, plan, BaseAe, networkId } from './' import { generateKeyPair } from '../../es/utils/crypto' import Channel from '../../es/channel' @@ -46,12 +46,13 @@ describe('Channel', function () { let responderShouldRejectUpdate let existingChannelId let offchainTx - const responderSign = async (tag, tx) => { - if (!responderShouldRejectUpdate) { - return await responder.signTransaction(tx) + const initiatorSign = sinon.spy((tag, tx) => initiator.signTransaction(tx)) + const responderSign = sinon.spy((tag, tx) => { + if (responderShouldRejectUpdate) { + return null } - return null - } + return responder.signTransaction(tx) + }) const sharedParams = { url: wsUrl, pushAmount: 3, @@ -77,18 +78,27 @@ describe('Channel', function () { responderShouldRejectUpdate = false }) + afterEach(() => { + initiatorSign.resetHistory() + responderSign.resetHistory() + }) + it('can open a channel', async () => { initiatorCh = await Channel({ ...sharedParams, role: 'initiator', - sign: async (tag, tx) => await initiator.signTransaction(tx) + sign: initiatorSign }) responderCh = await Channel({ ...sharedParams, role: 'responder', sign: responderSign }) - return Promise.all([waitForChannel(initiatorCh), waitForChannel(responderCh)]) + await Promise.all([waitForChannel(initiatorCh), waitForChannel(responderCh)]) + sinon.assert.calledOnce(initiatorSign) + sinon.assert.calledWithExactly(initiatorSign, sinon.match('initiator_sign'), sinon.match.string) + sinon.assert.calledOnce(responderSign) + sinon.assert.calledWithExactly(responderSign, sinon.match('responder_sign'), sinon.match.string) }) it('can post update and accept', async () => { @@ -101,6 +111,9 @@ describe('Channel', function () { ) result.accepted.should.equal(true) result.state.should.be.a('string') + sinon.assert.notCalled(initiatorSign) + sinon.assert.calledOnce(responderSign) + sinon.assert.calledWithExactly(responderSign, sinon.match('update_ack'), sinon.match.string) }) it('can post update and reject', async () => { @@ -112,6 +125,9 @@ describe('Channel', function () { async (tx) => await initiator.signTransaction(tx) ) result.accepted.should.equal(false) + sinon.assert.notCalled(initiatorSign) + sinon.assert.calledOnce(responderSign) + sinon.assert.calledWithExactly(responderSign, sinon.match('update_ack'), sinon.match.string) }) it('can get proof of inclusion', async () => { @@ -154,9 +170,9 @@ describe('Channel', function () { }) it('can request a withdraw and accept', async () => { - const onOnChainTx = spy() - const onOwnWithdrawLocked = spy() - const onWithdrawLocked = spy() + const onOnChainTx = sinon.spy() + const onOwnWithdrawLocked = sinon.spy() + const onWithdrawLocked = sinon.spy() responderShouldRejectUpdate = false const result = await initiatorCh.withdraw( 2, @@ -164,16 +180,19 @@ describe('Channel', function () { { onOnChainTx, onOwnWithdrawLocked, onWithdrawLocked } ) result.should.eql({ accepted: true, state: initiatorCh.state() }) - onOnChainTx.callCount.should.equal(1) - onOnChainTx.getCall(0).args[0].should.be.a('string') - onOwnWithdrawLocked.callCount.should.equal(1) - onWithdrawLocked.callCount.should.equal(1) + sinon.assert.calledOnce(onOnChainTx) + sinon.assert.calledWithExactly(onOnChainTx, sinon.match.string) + sinon.assert.calledOnce(onOwnWithdrawLocked) + sinon.assert.calledOnce(onWithdrawLocked) + sinon.assert.notCalled(initiatorSign) + sinon.assert.calledOnce(responderSign) + sinon.assert.calledWithExactly(responderSign, sinon.match('withdraw_ack'), sinon.match.string) }) it('can request a withdraw and reject', async () => { - const onOnChainTx = spy() - const onOwnWithdrawLocked = spy() - const onWithdrawLocked = spy() + const onOnChainTx = sinon.spy() + const onOwnWithdrawLocked = sinon.spy() + const onWithdrawLocked = sinon.spy() responderShouldRejectUpdate = true const result = await initiatorCh.withdraw( 2, @@ -181,15 +200,18 @@ describe('Channel', function () { { onOnChainTx, onOwnWithdrawLocked, onWithdrawLocked } ) result.should.eql({ accepted: false }) - onOnChainTx.callCount.should.equal(0) - onOwnWithdrawLocked.callCount.should.equal(0) - onWithdrawLocked.callCount.should.equal(0) + sinon.assert.notCalled(onOnChainTx) + sinon.assert.notCalled(onOwnWithdrawLocked) + sinon.assert.notCalled(onWithdrawLocked) + sinon.assert.notCalled(initiatorSign) + sinon.assert.calledOnce(responderSign) + sinon.assert.calledWithExactly(responderSign, sinon.match('withdraw_ack'), sinon.match.string) }) it('can request a deposit and accept', async () => { - const onOnChainTx = spy() - const onOwnDepositLocked = spy() - const onDepositLocked = spy() + const onOnChainTx = sinon.spy() + const onOwnDepositLocked = sinon.spy() + const onDepositLocked = sinon.spy() responderShouldRejectUpdate = false const result = await initiatorCh.deposit( 2, @@ -197,16 +219,19 @@ describe('Channel', function () { { onOnChainTx, onOwnDepositLocked, onDepositLocked } ) result.should.eql({ accepted: true, state: initiatorCh.state() }) - onOnChainTx.callCount.should.equal(1) - onOnChainTx.getCall(0).args[0].should.be.a('string') - onOwnDepositLocked.callCount.should.equal(1) - onDepositLocked.callCount.should.equal(1) + sinon.assert.calledOnce(onOnChainTx) + sinon.assert.calledWithExactly(onOnChainTx, sinon.match.string) + sinon.assert.calledOnce(onOwnDepositLocked) + sinon.assert.calledOnce(onDepositLocked) + sinon.assert.notCalled(initiatorSign) + sinon.assert.calledOnce(responderSign) + sinon.assert.calledWithExactly(responderSign, sinon.match('deposit_ack'), sinon.match.string) }) it('can request a deposit and reject', async () => { - const onOnChainTx = spy() - const onOwnDepositLocked = spy() - const onDepositLocked = spy() + const onOnChainTx = sinon.spy() + const onOwnDepositLocked = sinon.spy() + const onDepositLocked = sinon.spy() responderShouldRejectUpdate = true const result = await initiatorCh.deposit( 2, @@ -214,21 +239,27 @@ describe('Channel', function () { { onOnChainTx, onOwnDepositLocked, onDepositLocked } ) result.should.eql({ accepted: false }) - onOnChainTx.callCount.should.equal(0) - onOwnDepositLocked.callCount.should.equal(0) - onDepositLocked.callCount.should.equal(0) + sinon.assert.notCalled(onOnChainTx) + sinon.assert.notCalled(onOwnDepositLocked) + sinon.assert.notCalled(onDepositLocked) + sinon.assert.notCalled(initiatorSign) + sinon.assert.calledOnce(responderSign) + sinon.assert.calledWithExactly(responderSign, sinon.match('deposit_ack'), sinon.match.string) }) it('can close a channel', async () => { const tx = await initiatorCh.shutdown(async (tx) => await initiator.signTransaction(tx)) tx.should.be.a('string') + sinon.assert.notCalled(initiatorSign) + sinon.assert.calledOnce(responderSign) + sinon.assert.calledWithExactly(responderSign, sinon.match('shutdown_sign_ack'), sinon.match.string) }) it('can leave a channel', async () => { initiatorCh = await Channel({ ...sharedParams, role: 'initiator', - sign: async (tag, tx) => await initiator.signTransaction(tx) + sign: initiatorSign }) responderCh = await Channel({ ...sharedParams, @@ -249,15 +280,48 @@ describe('Channel', function () { role: 'initiator', existingChannelId, offchainTx, - sign: async (tag, tx) => await initiator.signTransaction(tx) + sign: initiatorSign }) responderCh = await Channel({ ...sharedParams, role: 'responder', existingChannelId, offchainTx, - sign: async (tag, tx) => await responder.signTransaction(tx) + sign: responderSign }) await Promise.all([waitForChannel(initiatorCh), waitForChannel(responderCh)]) + sinon.assert.notCalled(initiatorSign) + sinon.assert.notCalled(responderSign) + }) + + describe('throws errors', function () { + async function update ({ from, to, amount, sign }) { + return initiatorCh.update( + from || await initiator.address(), + to || await responder.address(), + amount || 1, + sign || initiator.signTransaction + ) + } + + it('when posting an update with negative amount', async () => { + return update({ amount: -10 }).should.eventually.be.rejectedWith('Amount cannot be negative') + }) + + it('when posting an update with insufficient balance', async () => { + return update({ amount: 2000000000000000 }).should.eventually.be.rejectedWith('Insufficient balance') + }) + + it('when posting an update with incorrect address', async () => { + return update({ from: 'ak_123' }).should.eventually.be.rejectedWith('Rejected') + }) + + it('when posting an update with incorrect amount', async () => { + return update({ amount: '1' }).should.eventually.be.rejectedWith('Internal error') + }) + + it('when posting incorrect update tx', async () => { + return update({ sign: () => 'abcdefg' }).should.eventually.be.rejectedWith('Internal error') + }) }) })