Skip to content

Commit

Permalink
Merge branch 'issue/2368' of github.com:ethereum/web3.js into issue/2368
Browse files Browse the repository at this point in the history
  • Loading branch information
Samuel Furter committed Feb 20, 2019
2 parents 085017d + 997ef47 commit 0841388
Show file tree
Hide file tree
Showing 10 changed files with 87 additions and 70 deletions.
10 changes: 6 additions & 4 deletions packages/web3-core-method/lib/methods/AbstractCallMethod.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,17 @@ export default class AbstractCallMethod extends AbstractMethod {
}

try {
const response = await moduleInstance.currentProvider.send(this.rpcMethod, this.parameters);
let response = await moduleInstance.currentProvider.send(this.rpcMethod, this.parameters);

const mappedResponse = this.afterExecution(response);
if (response) {
response = this.afterExecution(response);
}

if (this.callback) {
this.callback(false, mappedResponse);
this.callback(false, response);
}

return mappedResponse;
return response;
} catch (error) {
if (this.callback) {
this.callback(error, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export default class GetBlockTransactionCountMethod extends AbstractCallMethod {
* @constructor
*/
constructor(utils, formatters) {
super('eth_getTransactionByBlockNumberAndIndex', 1, utils, formatters);
super('eth_getBlockTransactionCountByNumber', 1, utils, formatters);
}

/**
Expand All @@ -42,7 +42,7 @@ export default class GetBlockTransactionCountMethod extends AbstractCallMethod {
*/
beforeExecution(moduleInstance) {
if (this.isHash(this.parameters[0])) {
this.rpcMethod = 'eth_getTransactionByBlockHashAndIndex';
this.rpcMethod = 'eth_getBlockTransactionCountByHash';
}

this.parameters[0] = this.formatters.inputBlockNumberFormatter(this.parameters[0]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,5 @@ export default class EcRecoverMethod extends AbstractCallMethod {
*/
beforeExecution(moduleInstance) {
this.parameters[0] = this.formatters.inputSignFormatter(this.parameters[0]);
this.parameters[1] = this.formatters.inputAddressFormatter(this.parameters[1]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,21 @@
* @date 2018
*/

import isObject from 'lodash/isObject';

export default class TransactionReceiptValidator {
/**
* Validates the receipt
*
* @method validate
*
* @param {Object} receipt
* @param {Array} methodParameters
* @param {AbstractMethod} method
*
* @returns {Error|Boolean}
*/
validate(receipt, methodParameters) {
validate(receipt, method) {
const receiptJSON = JSON.stringify(receipt, null, 2);

if (!this.isValidGasUsage(receipt, methodParameters)) {
if (!this.isValidGasUsage(receipt, method)) {
return new Error(`Transaction ran out of gas. Please provide more gas:\n${receiptJSON}`);
}

Expand Down Expand Up @@ -66,17 +64,11 @@ export default class TransactionReceiptValidator {
* @method isValidGasUsage
*
* @param {Object} receipt
* @param {Array} methodParameters
* @param {AbstractMethod} method
*
* @returns {Boolean}
*/
isValidGasUsage(receipt, methodParameters) {
let gasProvided = null;

if (isObject(methodParameters[0]) && methodParameters[0].gas) {
gasProvided = methodParameters[0].gas;
}

return !receipt.outOfGas && (!gasProvided || gasProvided !== receipt.gasUsed);
isValidGasUsage(receipt, method) {
return !receipt.outOfGas && method.utils.hexToNumber(method.parameters[0].gas) !== receipt.gasUsed;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export default class TransactionConfirmationWorkflow {

this.getTransactionReceiptMethod.execute(moduleInstance).then((receipt) => {
if (receipt && receipt.blockHash) {
const validationResult = this.transactionReceiptValidator.validate(receipt, method.parameters);
const validationResult = this.transactionReceiptValidator.validate(receipt, method);
if (validationResult === true) {
this.handleSuccessState(receipt, method, promiEvent);

Expand All @@ -72,10 +72,7 @@ export default class TransactionConfirmationWorkflow {
if (!this.isTimeoutTimeExceeded(moduleInstance, this.newHeadsWatcher.isPolling)) {
this.getTransactionReceiptMethod.execute(moduleInstance).then((receipt) => {
if (receipt && receipt.blockHash) {
const validationResult = this.transactionReceiptValidator.validate(
receipt,
method.parameters
);
const validationResult = this.transactionReceiptValidator.validate(receipt, method);

if (validationResult === true) {
this.confirmationsCounter++;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,20 @@ describe('AbstractCallMethodTest', () => {
expect(abstractCallMethod.beforeExecution).toHaveBeenCalledWith(moduleInstanceMock);
}
});

it('calls execute and it returns null', async () => {
providerMock.send.mockReturnValueOnce(Promise.resolve(null));

moduleInstanceMock.currentProvider = providerMock;

const response = await abstractCallMethod.execute(moduleInstanceMock);

expect(response).toEqual(null);

expect(providerMock.send).toHaveBeenCalledWith(abstractCallMethod.rpcMethod, abstractCallMethod.parameters);

expect(abstractCallMethod.callback).toHaveBeenCalledWith(false, null);

expect(abstractCallMethod.beforeExecution).toHaveBeenCalledWith(moduleInstanceMock);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ describe('GetBlockTransactionCountMethod', () => {
it('constructor check', () => {
expect(method).toBeInstanceOf(AbstractCallMethod);

expect(method.rpcMethod).toEqual('eth_getTransactionByBlockNumberAndIndex');
expect(method.rpcMethod).toEqual('eth_getBlockTransactionCountByNumber');

expect(method.parametersAmount).toEqual(1);

Expand All @@ -40,7 +40,7 @@ describe('GetBlockTransactionCountMethod', () => {

expect(formatters.inputBlockNumberFormatter).toHaveBeenCalledWith('0x0');

expect(method.rpcMethod).toEqual('eth_getTransactionByBlockHashAndIndex');
expect(method.rpcMethod).toEqual('eth_getBlockTransactionCountByHash');
});

it('beforeExecution should call method with block number as parameter and call inputBlockNumberFormatter', () => {
Expand All @@ -54,7 +54,7 @@ describe('GetBlockTransactionCountMethod', () => {

expect(formatters.inputBlockNumberFormatter).toHaveBeenCalledWith(100);

expect(method.rpcMethod).toEqual('eth_getTransactionByBlockNumberAndIndex');
expect(method.rpcMethod).toEqual('eth_getBlockTransactionCountByNumber');
});

it('afterExecution should map the hex string to a number', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,12 @@ describe('EcRecoverMethodTest', () => {

formatters.inputSignFormatter.mockReturnValueOnce({sign: true});

formatters.inputAddressFormatter.mockReturnValueOnce('0x0');

method.beforeExecution();

expect(method.parameters[0]).toHaveProperty('sign', true);

expect(method.parameters[1]).toEqual('0x0');

expect(formatters.inputSignFormatter).toHaveBeenCalledWith({});

expect(formatters.inputAddressFormatter).toHaveBeenCalledWith('0x0');
});
});
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import * as Utils from 'web3-utils';
import TransactionReceiptValidator from '../../../src/validators/TransactionReceiptValidator';

// Mocks
jest.mock('Utils');

/**
* TransactionReceiptValidator test
*/
describe('TransactionReceiptValidatorTest', () => {
let transactionReceiptValidator, receipt;
let transactionReceiptValidator, receipt, method;

beforeEach(() => {
receipt = {
Expand All @@ -13,52 +17,63 @@ describe('TransactionReceiptValidatorTest', () => {
gasUsed: 100
};

method = {};
method.utils = Utils;

transactionReceiptValidator = new TransactionReceiptValidator();
});

it('calls validate and returns true', () => {
expect(
transactionReceiptValidator.validate(receipt, [
{
gas: 110
}
])
).toEqual(true);
Utils.hexToNumber.mockReturnValueOnce(110);

method.parameters = [
{
gas: 110
}
];

expect(transactionReceiptValidator.validate(receipt, method)).toEqual(true);

expect(Utils.hexToNumber).toHaveBeenCalledWith(110);
});

it(
'calls validate and returns error because of invalid gasUsage',
() => {
const error = transactionReceiptValidator.validate(receipt, [
{
gas: 100
}
]);

expect(error).toBeInstanceOf(Error);

expect(error.message).toEqual(
`Transaction ran out of gas. Please provide more gas:\n${JSON.stringify(receipt, null, 2)}`
);
},
[
it('calls validate and returns error because of invalid gasUsage', () => {
Utils.hexToNumber.mockReturnValueOnce(100);

method.parameters = [
{
gas: 100
gas: 110
}
]
);
];

const error = transactionReceiptValidator.validate(receipt, method);

expect(error).toBeInstanceOf(Error);

expect(error.message).toEqual(
`Transaction ran out of gas. Please provide more gas:\n${JSON.stringify(receipt, null, 2)}`
);

expect(Utils.hexToNumber).toHaveBeenCalledWith(110);
});

it('calls validate and returns error because the EVM has reverted it', () => {
receipt.status = false;
Utils.hexToNumber.mockReturnValueOnce(110);

const error = transactionReceiptValidator.validate(receipt, [
method.parameters = [
{
gas: 101
}
]);
];

receipt.status = false;

const error = transactionReceiptValidator.validate(receipt, method);

expect(error).toBeInstanceOf(Error);

expect(error.message).toEqual(`Transaction has been reverted by the EVM:\n${JSON.stringify(receipt, null, 2)}`);

expect(Utils.hexToNumber).toHaveBeenCalledWith(101);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ describe('TransactionConfirmationWorkflowTest', () => {

expect(getTransactionReceiptMethodMock.execute).toHaveBeenCalledWith(moduleInstanceMock);

expect(transactionReceiptValidatorMock.validate).toHaveBeenCalledWith({blockHash: true}, []);
expect(transactionReceiptValidatorMock.validate).toHaveBeenCalledWith({blockHash: true}, methodMock);

expect(transactionConfirmationWorkflow.timeoutCounter).toEqual(0);

Expand Down Expand Up @@ -158,7 +158,10 @@ describe('TransactionConfirmationWorkflowTest', () => {

expect(contractDeployMethodMock.afterExecution).toHaveBeenCalledWith({blockHash: '0x0'});

expect(transactionReceiptValidatorMock.validate).toHaveBeenCalledWith({blockHash: '0x0'});
expect(transactionReceiptValidatorMock.validate).toHaveBeenCalledWith(
{blockHash: '0x0'},
contractDeployMethodMock
);
});
});

Expand Down Expand Up @@ -216,7 +219,7 @@ describe('TransactionConfirmationWorkflowTest', () => {

expect(methodMock.afterExecution).toHaveBeenCalledWith({blockHash: '0x0'});

expect(transactionReceiptValidatorMock.validate).toHaveBeenCalledWith({blockHash: '0x0'});
expect(transactionReceiptValidatorMock.validate).toHaveBeenCalledWith({blockHash: '0x0'}, methodMock);

done();
});
Expand Down Expand Up @@ -260,10 +263,7 @@ describe('TransactionConfirmationWorkflowTest', () => {

expect(newHeadsWatcherMock.stop).toHaveBeenCalled();

expect(transactionReceiptValidatorMock.validate).toHaveBeenCalledWith(
{blockHash: '0x0'},
methodMock.parameters
);
expect(transactionReceiptValidatorMock.validate).toHaveBeenCalledWith({blockHash: '0x0'}, methodMock);

expect(getTransactionReceiptMethodMock.execute).toHaveBeenNthCalledWith(2, moduleInstanceMock);

Expand Down

0 comments on commit 0841388

Please sign in to comment.