Skip to content
This repository has been archived by the owner on Mar 8, 2020. It is now read-only.

Commit

Permalink
move response compare to within catch block (#4046)
Browse files Browse the repository at this point in the history 
Signed-off-by: Nick Lincoln <[email protected]>
  • Loading branch information
@nklincoln
nklincoln authored and Dave Kelsey committed May 23, 2018
1 parent 796c14a commit c9fa729
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 33 deletions.
31 changes: 17 additions & 14 deletions packages/composer-connector-hlfv1/lib/hlfconnection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ class HLFConnection extends Connection {

LOG.debug(method, 'sending instantiate proposal', proposal);
const proposalResponse = await this.channel.sendInstantiateProposal(proposal);
await this._sendTransactionForProposal(proposalResponse, transactionId);
await this._sendTransaction(proposalResponse, transactionId);
} catch(error) {
const newError = new Error('Error trying to start business network. ' + error);
LOG.error(method, error);
Expand All @@ -684,8 +684,8 @@ class HLFConnection extends Connection {
* @async
* @private
*/
async _sendTransactionForProposal(proposalResponse, transactionId) {
const method = '_sendTransactionForProposal';
async _sendTransaction(proposalResponse, transactionId) {
const method = '_sendTransaction';
LOG.entry(method, proposalResponse);

// Validate the instantiate proposal results
Expand All @@ -708,7 +708,18 @@ class HLFConnection extends Connection {
eventHandler.cancelListening();
throw new Error(`Failed to send peer responses for transaction '${transactionId.getTransactionID()}' to orderer. Response status '${response.status}'`);
}
await eventHandler.waitForEvents();

try {
await eventHandler.waitForEvents();
} catch (error) {
// Investigate proposal response results and log if they differ and rethrow
if (!this.channel.compareProposalResponseResults(validResponses)) {
const warning = 'Peers do not agree, Read Write sets differ';
LOG.warn(method, warning);
}
throw error;
}

LOG.exit(method);
}

Expand Down Expand Up @@ -764,21 +775,13 @@ class HLFConnection extends Connection {

}
});

if (validResponses.length === 0 && ignoredErrors < responses.length) {
const errorMessages = [ 'No valid responses from any peers.' ];
invalidResponseMsgs.forEach(invalidResponse => errorMessages.push(invalidResponse));
throw new Error(errorMessages.join('\n'));
}

// if it was a proposal and some of the responses were good, check that they compare
// but we can't reject it as we don't know if it would still pass the endorsement policy
// and if we did this would allow a malicious peer to stop transactions so we
// issue a warning so that it get's logged, but we don't know which peer(s) it was
if (isProposal && !this.channel.compareProposalResponseResults(validResponses)) {
const warning = 'Peers do not agree, Read Write sets differ';
LOG.warn(method, warning);
invalidResponseMsgs.push(warning);
}
LOG.exit(method, ignoredErrors);
return {ignoredErrors, validResponses, invalidResponseMsgs};
}
Expand Down Expand Up @@ -1144,7 +1147,7 @@ class HLFConnection extends Connection {

LOG.debug(method, 'sending upgrade proposal', proposal);
const proposalResults = await this.channel.sendUpgradeProposal(proposal);
await this._sendTransactionForProposal(proposalResults, transactionId);
await this._sendTransaction(proposalResults, transactionId);
} catch(error) {
const newError = new Error('Error trying to upgrade business network. ' + error);
LOG.error(method, error);
Expand Down
91 changes: 75 additions & 16 deletions packages/composer-connector-hlfv1/test/hlfconnection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const HLFConnection = require('../lib/hlfconnection');
const HLFConnectionManager = require('../lib/hlfconnectionmanager');
const HLFSecurityContext = require('../lib/hlfsecuritycontext');
const HLFQueryHandler = require('../lib/hlfqueryhandler');
const HLFEventHandler = require('../lib/hlftxeventhandler');

const path = require('path');
const semver = require('semver');
Expand Down Expand Up @@ -1220,6 +1221,19 @@ describe('HLFConnection', () => {
sinon.assert.calledOnce(connection._checkEventhubs);
});
});

it('should throw an error if peers do not agree', () => {
let mockEventHandler = sinon.createStubInstance(HLFEventHandler);
mockEventHandler.waitForEvents.throws(new Error('such error'));
sandbox.stub(HLFConnection, 'createTxEventHandler').returns(mockEventHandler);

mockChannel.sendTransaction.resolves({ status: 'SUCCESS' });
mockChannel.verifyProposalResponse.returns(true);
mockChannel.compareProposalResponseResults.returns(false);
mockEventHub1.registerTxEvent.yields(mockTransactionID.getTransactionID().toString(), 'INVALID');
connection.start(mockSecurityContext, mockBusinessNetwork.getName(), mockBusinessNetwork.getVersion(), '{"start":"json"}')
.should.be.rejectedWith(/such error'/);
});
});

describe('#upgrade', () => {
Expand Down Expand Up @@ -1295,6 +1309,67 @@ describe('HLFConnection', () => {
});
});

describe('#_sendTransaction', () => {

it('should log if compareProposals returns false', async () => {

let mockEventHandler = sinon.createStubInstance(HLFEventHandler);
mockEventHandler.waitForEvents.throws(new Error('such error'));
sandbox.stub(HLFConnection, 'createTxEventHandler').returns(mockEventHandler);

const responses = [
{
response: {
status: 500,
payload: 'such error'
}
}
];
sandbox.stub(connection, '_validatePeerResponses').returns({ignoredErrors: 0, validResponses: responses});

mockChannel.sendTransaction.resolves({ status: 'SUCCESS' });
mockChannel.verifyProposalResponse.returns(true);
mockChannel.compareProposalResponseResults.returns(false);

try {
await connection._sendTransaction(responses, mockTransactionID);
sinon.assert.fail('should have thrown');
} catch (err) {
sinon.assert.calledWith(logWarnSpy, '_sendTransaction', 'Peers do not agree, Read Write sets differ');
}

});

it('should not log if compareProposals returns false', async () => {

let mockEventHandler = sinon.createStubInstance(HLFEventHandler);
mockEventHandler.waitForEvents.throws(new Error('such error'));
sandbox.stub(HLFConnection, 'createTxEventHandler').returns(mockEventHandler);

const responses = [
{
response: {
status: 500,
payload: 'such error'
}
}
];
sandbox.stub(connection, '_validatePeerResponses').returns({ignoredErrors: 0, validResponses: responses});

mockChannel.sendTransaction.resolves({ status: 'SUCCESS' });
mockChannel.verifyProposalResponse.returns(true);
mockChannel.compareProposalResponseResults.returns(true);

try {
await connection._sendTransaction(responses, mockTransactionID);
sinon.assert.fail('should have thrown');
} catch (err) {
sinon.assert.notCalled(logWarnSpy);
}

});
});

describe('#_validatePeerResponses', () => {
it('should return all responses because all are valid', () => {
const responses = [
Expand Down Expand Up @@ -1430,22 +1505,6 @@ describe('HLFConnection', () => {
sinon.assert.calledWith(logWarnSpy, '_validatePeerResponses', sinon.match(/Proposal response from peer failed verification/));
});

it('should log if compareProposals returns false', () => {
const responses = [
{
response: {
status: 200,
payload: 'no error'
}
}
];

mockChannel.verifyProposalResponse.returns(true);
mockChannel.compareProposalResponseResults.returns(false);
connection._validatePeerResponses(responses, true);
sinon.assert.calledWith(logWarnSpy, '_validatePeerResponses', 'Peers do not agree, Read Write sets differ');
});

it('should not try to check proposal responses if not a response from a proposal', () => {
const responses = [
{
Expand Down
3 changes: 0 additions & 3 deletions packages/composer-connector-hlfv1/test/hlftxeventhandler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ const chai = require('chai');
const should = chai.should();
chai.use(require('chai-as-promised'));




describe('HLFTxEventHandler', () => {

let sandbox, logWarnSpy;
Expand Down

0 comments on commit c9fa729

Please sign in to comment.