From 30cf8afd9c75aef7138553437cab7785b31468b9 Mon Sep 17 00:00:00 2001 From: nivida Date: Wed, 27 Nov 2019 16:40:03 +0100 Subject: [PATCH 01/22] revert reason string handling added --- packages/web3-core-helpers/src/errors.js | 7 ++ packages/web3-core-method/src/index.js | 120 ++++++++++++++++++++--- packages/web3-eth-contract/src/index.js | 6 +- packages/web3-eth/src/index.js | 9 +- packages/web3-utils/src/index.js | 8 +- 5 files changed, 127 insertions(+), 23 deletions(-) diff --git a/packages/web3-core-helpers/src/errors.js b/packages/web3-core-helpers/src/errors.js index 820b30384b2..96b29b5966f 100644 --- a/packages/web3-core-helpers/src/errors.js +++ b/packages/web3-core-helpers/src/errors.js @@ -43,5 +43,12 @@ module.exports = { }, ConnectionTimeout: function (ms){ return new Error('CONNECTION TIMEOUT: timeout of ' + ms + ' ms achived'); + }, + RevertInstructionError: function(reason, signature) { + var error = new Error('Your request got reverted with the following reason string: ' + reason); + error.reason = reason; + error.signature = signature; + + return error; } }; diff --git a/packages/web3-core-method/src/index.js b/packages/web3-core-method/src/index.js index 98cea3d63c5..040d310bd5e 100644 --- a/packages/web3-core-method/src/index.js +++ b/packages/web3-core-method/src/index.js @@ -43,6 +43,7 @@ var Method = function Method(options) { this.outputFormatter = options.outputFormatter; this.transformPayload = options.transformPayload; this.extraFormatters = options.extraFormatters; + this.abiCoder = options.abiCoder; // Will be used to encode the revert reason string this.requestManager = options.requestManager; @@ -391,7 +392,7 @@ Method.prototype._confirmTransaction = function(defer, result, payload) { return receipt; }) // CHECK for normal tx check for receipt only - .then(function(receipt) { + .then(async function(receipt) { if (!isContractDeployment && !promiseResolved) { if (!receipt.outOfGas && (!gasProvided || gasProvided !== receipt.gasUsed) && @@ -408,13 +409,36 @@ Method.prototype._confirmTransaction = function(defer, result, payload) { receiptJSON = JSON.stringify(receipt, null, 2); if (receipt.status === false || receipt.status === '0x0') { - utils._fireError( - new Error('Transaction has been reverted by the EVM:\n' + receiptJSON), - defer.eventEmitter, - defer.reject, - null, - receipt - ); + try { + var revertMessage = await method.getRevertReason(payload.params[0], receipt.blockNumber); + + if (revertMessage.reason) { + utils._fireError( + errors.RevertInstructionError(revertMessage.reason, revertMessage.signature), + defer.eventEmitter, + defer.reject, + payload.callback, + receipt + ); + } else { + utils._fireError( + new Error('Transaction has been reverted by the EVM:\n' + receiptJSON), + defer.eventEmitter, + defer.reject, + null, + receipt + ); + } + } catch (error) { + utils._fireError( + new Error('Transaction has been reverted by the EVM:\n' + receiptJSON), + defer.eventEmitter, + defer.reject, + null, + receipt + ); + } + } else { utils._fireError( new Error('Transaction ran out of gas. Please provide more gas:\n' + receiptJSON), @@ -526,16 +550,34 @@ var getWallet = function(from, accounts) { Method.prototype.buildCall = function() { var method = this, - isSendTx = (method.call === 'eth_sendTransaction' || method.call === 'eth_sendRawTransaction'); // || method.call === 'personal_sendTransaction' + isSendTx = (method.call === 'eth_sendTransaction' || method.call === 'eth_sendRawTransaction'), // || method.call === 'personal_sendTransaction' + isCall = (method.call === 'eth_call'); // actual send function var send = function() { var defer = promiEvent(!isSendTx), payload = method.toPayload(Array.prototype.slice.call(arguments)); - // CALLBACK function var sendTxCallback = function(err, result) { + if (isCall && (method.isRevertReasonString(result) && method.abiCoder)) { + var reason = method.abiCoder.decodeParameter('string', '0x' + result.substring(10)); + var signature = 'Error(String)'; + + utils._fireError( + errors.RevertInstructionError(reason, signature), + defer.eventEmitter, + defer.reject, + payload.callback, + { + reason: reason, + signature: signature + } + ); + + return; + } + try { result = method.formatOutput(result); } catch (e) { @@ -560,10 +602,8 @@ Method.prototype.buildCall = function() { // return PROMISE if (!isSendTx) { - if (!err) { defer.resolve(result); - } // return PROMIEVENT @@ -620,8 +660,7 @@ Method.prototype.buildCall = function() { if (_.isFunction(defer.eventEmitter.listeners) && defer.eventEmitter.listeners('error').length) { defer.eventEmitter.emit('error', err); defer.eventEmitter.removeAllListeners(); - defer.eventEmitter.catch(function() { - }); + defer.eventEmitter.catch(function() {}); } defer.reject(err); }); @@ -683,6 +722,59 @@ Method.prototype.buildCall = function() { return send; }; +/** + * + * @param txOptions + * @param revertedReceipt + * @param abiCoder + * @returns {Promise<*>} + */ +Method.prototype.getRevertReason = function(txOptions, blockNumber) { + var self = this; + + return new Promise(function(resolve, reject) { + (new Method({ + name: 'call', + call: 'eth_call', + params: 2, + abiCoder: self.abiCoder + })) + .createFunction(self.requestManager)(txOptions, utils.numberToHex(blockNumber)) + .then(function() { + resolve({reason: false}); + }) + .catch(function(error) { + if (error.reason) { + resolve({ + reason: error.reason, + signature: error.signature + }); + } else { + reject(error); + } + }); + }); +}; + +/** + * Checks if the given hex string is a revert message from the EVM + * + * @method isRevertReasonString + * + * @param {String} data - Hex string prefixed with 0x + * + * @returns {Boolean} + */ +Method.prototype.isRevertReasonString = function (data) { + if (data.substring(0, 2) !== '0x') { + throw new Error('Hex prefix "0x" is missing.'); + } + + var length = (data.length - 2) / 2; + + return length % 32 === 4 && data.substring(0, 10) === '0x08c379a0'; +}; + /** * Should be called to create the pure JSONRPC request which can be used in a batch request * diff --git a/packages/web3-eth-contract/src/index.js b/packages/web3-eth-contract/src/index.js index a3c97690e74..7d32b384ae7 100644 --- a/packages/web3-eth-contract/src/index.js +++ b/packages/web3-eth-contract/src/index.js @@ -826,7 +826,8 @@ Contract.prototype._executeMethod = function _executeMethod(){ requestManager: _this._parent._requestManager, accounts: ethAccounts, // is eth.accounts (necessary for wallet signing) defaultAccount: _this._parent.defaultAccount, - defaultBlock: _this._parent.defaultBlock + defaultBlock: _this._parent.defaultBlock, + abiCoder: abi })).createFunction(); return call(args.options, args.defaultBlock, args.callback); @@ -903,7 +904,8 @@ Contract.prototype._executeMethod = function _executeMethod(){ defaultCommon: _this._parent.defaultCommon, defaultChain: _this._parent.defaultChain, defaultHardfork: _this._parent.defaultHardfork, - extraFormatters: extraFormatters + extraFormatters: extraFormatters, + abiCoder: abi })).createFunction(); return sendTransaction(args.options, args.callback); diff --git a/packages/web3-eth/src/index.js b/packages/web3-eth/src/index.js index 192afe6a547..dfbb56b218f 100644 --- a/packages/web3-eth/src/index.js +++ b/packages/web3-eth/src/index.js @@ -423,7 +423,8 @@ var Eth = function Eth() { name: 'sendSignedTransaction', call: 'eth_sendRawTransaction', params: 1, - inputFormatter: [null] + inputFormatter: [null], + abiCoder: abi }), new Method({ name: 'signTransaction', @@ -435,7 +436,8 @@ var Eth = function Eth() { name: 'sendTransaction', call: 'eth_sendTransaction', params: 1, - inputFormatter: [formatter.inputTransactionFormatter] + inputFormatter: [formatter.inputTransactionFormatter], + abiCoder: abi }), new Method({ name: 'sign', @@ -451,7 +453,8 @@ var Eth = function Eth() { name: 'call', call: 'eth_call', params: 2, - inputFormatter: [formatter.inputCallFormatter, formatter.inputDefaultBlockNumberFormatter] + inputFormatter: [formatter.inputCallFormatter, formatter.inputDefaultBlockNumberFormatter], + abiCoder: abi }), new Method({ name: 'estimateGas', diff --git a/packages/web3-utils/src/index.js b/packages/web3-utils/src/index.js index fe9951f06a8..bc2dca70157 100644 --- a/packages/web3-utils/src/index.js +++ b/packages/web3-utils/src/index.js @@ -63,9 +63,11 @@ var _fireError = function (error, emitter, reject, callback, optionalData) { if (_.isFunction(reject)) { // suppress uncatched error if an error listener is present // OR suppress uncatched error if an callback listener is present - if (emitter && + if ( + emitter && (_.isFunction(emitter.listeners) && - emitter.listeners('error').length) || _.isFunction(callback)) { + emitter.listeners('error').length) || _.isFunction(callback) + ) { emitter.catch(function(){}); } // reject later, to be able to return emitter @@ -314,8 +316,6 @@ var toChecksumAddress = function (address) { return checksumAddress; }; - - module.exports = { _fireError: _fireError, _jsonInterfaceMethodToString: _jsonInterfaceMethodToString, From d74843efb0aedf8c9baadd03e4e7024740f96543 Mon Sep 17 00:00:00 2001 From: nivida Date: Thu, 28 Nov 2019 13:39:18 +0100 Subject: [PATCH 02/22] method.isRevertReasonString simplified --- packages/web3-core-method/src/index.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/web3-core-method/src/index.js b/packages/web3-core-method/src/index.js index 040d310bd5e..c57caf35a85 100644 --- a/packages/web3-core-method/src/index.js +++ b/packages/web3-core-method/src/index.js @@ -766,13 +766,7 @@ Method.prototype.getRevertReason = function(txOptions, blockNumber) { * @returns {Boolean} */ Method.prototype.isRevertReasonString = function (data) { - if (data.substring(0, 2) !== '0x') { - throw new Error('Hex prefix "0x" is missing.'); - } - - var length = (data.length - 2) / 2; - - return length % 32 === 4 && data.substring(0, 10) === '0x08c379a0'; + return ((data.length - 2) / 2) % 32 === 4 && data.substring(0, 10) === '0x08c379a0'; }; /** From 182783e58e3580a3d3aef78bb5be1fde1c8d8f7b Mon Sep 17 00:00:00 2001 From: nivida Date: Thu, 28 Nov 2019 13:45:42 +0100 Subject: [PATCH 03/22] method.getRevertReason funcDoc added and return values improved --- packages/web3-core-method/src/index.js | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/web3-core-method/src/index.js b/packages/web3-core-method/src/index.js index c57caf35a85..2de6045cb48 100644 --- a/packages/web3-core-method/src/index.js +++ b/packages/web3-core-method/src/index.js @@ -410,9 +410,12 @@ Method.prototype._confirmTransaction = function(defer, result, payload) { if (receipt.status === false || receipt.status === '0x0') { try { - var revertMessage = await method.getRevertReason(payload.params[0], receipt.blockNumber); + var revertMessage = await method.getRevertReason( + payload.params[0], + receipt.blockNumber + ); - if (revertMessage.reason) { + if (revertMessage) { utils._fireError( errors.RevertInstructionError(revertMessage.reason, revertMessage.signature), defer.eventEmitter, @@ -723,11 +726,14 @@ Method.prototype.buildCall = function() { }; /** + * Returns the revert reason string if existing or otherwise false. + * + * @method getRevertReason + * + * @param {Object} txOptions + * @param {Number} blockNumber * - * @param txOptions - * @param revertedReceipt - * @param abiCoder - * @returns {Promise<*>} + * @returns {Promise} */ Method.prototype.getRevertReason = function(txOptions, blockNumber) { var self = this; @@ -741,7 +747,7 @@ Method.prototype.getRevertReason = function(txOptions, blockNumber) { })) .createFunction(self.requestManager)(txOptions, utils.numberToHex(blockNumber)) .then(function() { - resolve({reason: false}); + resolve(false); }) .catch(function(error) { if (error.reason) { From 854e5793702398fd1e7734703f6d39db852d19a1 Mon Sep 17 00:00:00 2001 From: nivida Date: Thu, 28 Nov 2019 13:49:58 +0100 Subject: [PATCH 04/22] types extended with RevertInstructionError in core-helpers module --- packages/web3-core-helpers/types/index.d.ts | 6 ++++++ packages/web3-core-helpers/types/tests/errors-test.ts | 3 +++ 2 files changed, 9 insertions(+) diff --git a/packages/web3-core-helpers/types/index.d.ts b/packages/web3-core-helpers/types/index.d.ts index cb297c2ee9b..43aeacd36cc 100644 --- a/packages/web3-core-helpers/types/index.d.ts +++ b/packages/web3-core-helpers/types/index.d.ts @@ -66,6 +66,7 @@ export class errors { static InvalidProvider(): Error; static InvalidResponse(result: Error): Error; static ConnectionTimeout(ms: string): Error; + static RevertInstructionError(reason: string, signature: string): RevertInstructionError } export class WebsocketProviderBase { @@ -182,3 +183,8 @@ export interface JsonRpcResponse { result?: any; error?: string; } + +export interface RevertInstructionError extends Error { + reason: string; + signature: string; +} diff --git a/packages/web3-core-helpers/types/tests/errors-test.ts b/packages/web3-core-helpers/types/tests/errors-test.ts index 1cc503fafc0..87448197af5 100644 --- a/packages/web3-core-helpers/types/tests/errors-test.ts +++ b/packages/web3-core-helpers/types/tests/errors-test.ts @@ -36,3 +36,6 @@ errors.InvalidResponse(new Error('hey')); // $ExpectType Error errors.ConnectionTimeout('timeout'); + +// $ExpectType RevertInstructionError +errors.RevertInstructionError('reason', 'signature'); From 8dcc9d72b748cc2e042023e44ce64172637f5328 Mon Sep 17 00:00:00 2001 From: nivida Date: Thu, 28 Nov 2019 13:54:20 +0100 Subject: [PATCH 05/22] web3-eth types updated --- packages/web3-core-method/types/index.d.ts | 1 + packages/web3-eth/types/index.d.ts | 13 +++++++------ packages/web3-eth/types/tests/eth.tests.ts | 22 +++++++++++----------- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/packages/web3-core-method/types/index.d.ts b/packages/web3-core-method/types/index.d.ts index 2fbd3fdc937..a3b01415778 100644 --- a/packages/web3-core-method/types/index.d.ts +++ b/packages/web3-core-method/types/index.d.ts @@ -29,4 +29,5 @@ export interface Method { extraFormatters?: any; defaultBlock?: string; defaultAccount?: string | null; + abiCoder?: any } diff --git a/packages/web3-eth/types/index.d.ts b/packages/web3-eth/types/index.d.ts index 83d67ffd0ce..1ed16d0d1f4 100644 --- a/packages/web3-eth/types/index.d.ts +++ b/packages/web3-eth/types/index.d.ts @@ -38,6 +38,7 @@ import { LogsOptions, PastLogsOptions } from 'web3-core'; +import {RevertInstructionError} from 'web3-core-helpers'; import {Subscription} from 'web3-core-subscriptions'; import {AbiCoder} from 'web3-eth-abi'; import {Accounts} from 'web3-eth-accounts'; @@ -294,12 +295,12 @@ export class Eth { sendTransaction( transactionConfig: TransactionConfig, callback?: (error: Error, hash: string) => void - ): PromiEvent; + ): PromiEvent; sendSignedTransaction( signedTransactionData: string, callback?: (error: Error, hash: string) => void - ): PromiEvent; + ): PromiEvent; sign( dataToSign: string, @@ -327,20 +328,20 @@ export class Eth { ) => void ): Promise; - call(transactionConfig: TransactionConfig): Promise; + call(transactionConfig: TransactionConfig): Promise; call( transactionConfig: TransactionConfig, defaultBlock?: BlockNumber - ): Promise; + ): Promise; call( transactionConfig: TransactionConfig, callback?: (error: Error, data: string) => void - ): Promise; + ): Promise; call( transactionConfig: TransactionConfig, defaultBlock: BlockNumber, callback: (error: Error, data: string) => void - ): Promise; + ): Promise; estimateGas( transactionConfig: TransactionConfig, diff --git a/packages/web3-eth/types/tests/eth.tests.ts b/packages/web3-eth/types/tests/eth.tests.ts index c62a89c8365..ea572e4a419 100644 --- a/packages/web3-eth/types/tests/eth.tests.ts +++ b/packages/web3-eth/types/tests/eth.tests.ts @@ -385,12 +385,12 @@ eth.getTransactionCount( const code = '603d80600c6000396000f3007c0'; -// $ExpectType PromiEvent +// $ExpectType PromiEvent eth.sendTransaction({ from: '0xde0B295669a9FD93d5F28D9Ec85E40f4cb697BAe', data: 'code' }); -// $ExpectType PromiEvent +// $ExpectType PromiEvent eth.sendTransaction( { from: '0xde0B295669a9FD93d5F28D9Ec85E40f4cb697BAe', @@ -399,9 +399,9 @@ eth.sendTransaction( (error: Error, hash: string) => {} ); -// $ExpectType PromiEvent +// $ExpectType PromiEvent eth.sendSignedTransaction('0xf889808609184e72a0008227109'); -// $ExpectType PromiEvent +// $ExpectType PromiEvent eth.sendSignedTransaction( '0xf889808609184e72a0008227109', (error: Error, hash: string) => {} @@ -467,13 +467,13 @@ eth.signTransaction( (error: Error, signedTransaction: RLPEncodedTransaction) => {} ); -// $ExpectType Promise +// $ExpectType Promise eth.call({ to: '0x11f4d0A3c12e86B4b5F39B213F7E19D048276DAe', // contract address data: '0xc6888fa10000000000000000000000000000000000000000000000000000000000000003' }); -// $ExpectType Promise +// $ExpectType Promise eth.call( { to: '0x11f4d0A3c12e86B4b5F39B213F7E19D048276DAe', // contract address @@ -482,7 +482,7 @@ eth.call( }, 100 ); -// $ExpectType Promise +// $ExpectType Promise eth.call( { to: '0x11f4d0A3c12e86B4b5F39B213F7E19D048276DAe', // contract address @@ -491,7 +491,7 @@ eth.call( }, '100' ); -// $ExpectType Promise +// $ExpectType Promise eth.call( { to: '0x11f4d0A3c12e86B4b5F39B213F7E19D048276DAe', // contract address @@ -500,7 +500,7 @@ eth.call( }, (error: Error, data: string) => {} ); -// $ExpectType Promise +// $ExpectType Promise eth.call( { to: '0x11f4d0A3c12e86B4b5F39B213F7E19D048276DAe', // contract address @@ -510,7 +510,7 @@ eth.call( '100', (error: Error, data: string) => {} ); -// $ExpectType Promise +// $ExpectType Promise eth.call( { to: '0x11f4d0A3c12e86B4b5F39B213F7E19D048276DAe', // contract address @@ -521,7 +521,7 @@ eth.call( (error: Error, data: string) => {} ); -// $ExpectType Promise +// $ExpectType Promise eth.call( { to: '0x11f4d0A3c12e86B4b5F39B213F7E19D048276DAe', // contract address From 7c246cb3cc7f30c1baf71b89ee9ade500c2a4c88 Mon Sep 17 00:00:00 2001 From: nivida Date: Thu, 28 Nov 2019 14:35:51 +0100 Subject: [PATCH 06/22] error handling in Method object of the web3-core-method module fixed --- packages/web3-core-method/src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/web3-core-method/src/index.js b/packages/web3-core-method/src/index.js index 2de6045cb48..a8fa6a71339 100644 --- a/packages/web3-core-method/src/index.js +++ b/packages/web3-core-method/src/index.js @@ -563,7 +563,7 @@ Method.prototype.buildCall = function() { // CALLBACK function var sendTxCallback = function(err, result) { - if (isCall && (method.isRevertReasonString(result) && method.abiCoder)) { + if (!err && isCall && (method.isRevertReasonString(result) && method.abiCoder)) { var reason = method.abiCoder.decodeParameter('string', '0x' + result.substring(10)); var signature = 'Error(String)'; From 3c2b6b2a8acb0f0105581673bb09dc004abcf25d Mon Sep 17 00:00:00 2001 From: nivida Date: Thu, 28 Nov 2019 14:45:54 +0100 Subject: [PATCH 07/22] Method.isRevertReasonString updated --- packages/web3-core-method/src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/web3-core-method/src/index.js b/packages/web3-core-method/src/index.js index a8fa6a71339..cc36b39f3df 100644 --- a/packages/web3-core-method/src/index.js +++ b/packages/web3-core-method/src/index.js @@ -772,7 +772,7 @@ Method.prototype.getRevertReason = function(txOptions, blockNumber) { * @returns {Boolean} */ Method.prototype.isRevertReasonString = function (data) { - return ((data.length - 2) / 2) % 32 === 4 && data.substring(0, 10) === '0x08c379a0'; + return _.isString(data) && ((data.length - 2) / 2) % 32 === 4 && data.substring(0, 10) === '0x08c379a0'; }; /** From b63fca3e1887f48a7280551fbcc2eb1be9f7710a Mon Sep 17 00:00:00 2001 From: nivida Date: Mon, 2 Dec 2019 09:41:51 +0100 Subject: [PATCH 08/22] console.log added to test case to check the solely remotely failing test case --- test/e2e.method.signing.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/e2e.method.signing.js b/test/e2e.method.signing.js index 5631de6c800..bc74bac95a1 100644 --- a/test/e2e.method.signing.js +++ b/test/e2e.method.signing.js @@ -261,6 +261,8 @@ describe('transaction and message signing [ @E2E ]', function() { .eth .sendTransaction(tx) .on('error', function(err){ + console.log(err); + assert(err.message.includes('gas')) done(); }) From 04156ae73db00ed5622c30d9282904da31b1a1da Mon Sep 17 00:00:00 2001 From: nivida Date: Mon, 2 Dec 2019 10:10:43 +0100 Subject: [PATCH 09/22] handleRevert module option added to eth and eth-contract. Duplicated error removed in Method object --- packages/web3-core-method/src/index.js | 52 ++++++++++++------------- packages/web3-eth-contract/src/index.js | 3 ++ packages/web3-eth/src/index.js | 20 ++++++++++ test/e2e.method.signing.js | 2 - 4 files changed, 49 insertions(+), 28 deletions(-) diff --git a/packages/web3-core-method/src/index.js b/packages/web3-core-method/src/index.js index cc36b39f3df..4eb7b856580 100644 --- a/packages/web3-core-method/src/index.js +++ b/packages/web3-core-method/src/index.js @@ -58,6 +58,7 @@ var Method = function Method(options) { this.defaultCommon = options.defaultCommon; this.defaultChain = options.defaultChain; this.defaultHardfork = options.defaultHardfork; + this.handleRevert = options.handleRevert; }; Method.prototype.setRequestManager = function(requestManager, accounts) { @@ -409,21 +410,30 @@ Method.prototype._confirmTransaction = function(defer, result, payload) { receiptJSON = JSON.stringify(receipt, null, 2); if (receipt.status === false || receipt.status === '0x0') { - try { - var revertMessage = await method.getRevertReason( - payload.params[0], - receipt.blockNumber - ); - - if (revertMessage) { - utils._fireError( - errors.RevertInstructionError(revertMessage.reason, revertMessage.signature), - defer.eventEmitter, - defer.reject, - payload.callback, - receipt - ); - } else { + try { + var revertMessage = null; + + if (method.handleRevert) { + revertMessage = await method.getRevertReason( + payload.params[0], + receipt.blockNumber + ); + + if (revertMessage) { + utils._fireError( + errors.RevertInstructionError(revertMessage.reason, revertMessage.signature), + defer.eventEmitter, + defer.reject, + payload.callback, + receipt + ); + } else { + throw false; + } + } else { + throw false; + } + } catch (error) { utils._fireError( new Error('Transaction has been reverted by the EVM:\n' + receiptJSON), defer.eventEmitter, @@ -432,16 +442,6 @@ Method.prototype._confirmTransaction = function(defer, result, payload) { receipt ); } - } catch (error) { - utils._fireError( - new Error('Transaction has been reverted by the EVM:\n' + receiptJSON), - defer.eventEmitter, - defer.reject, - null, - receipt - ); - } - } else { utils._fireError( new Error('Transaction ran out of gas. Please provide more gas:\n' + receiptJSON), @@ -563,7 +563,7 @@ Method.prototype.buildCall = function() { // CALLBACK function var sendTxCallback = function(err, result) { - if (!err && isCall && (method.isRevertReasonString(result) && method.abiCoder)) { + if (method.handleRevert && !err && isCall && (method.isRevertReasonString(result) && method.abiCoder)) { var reason = method.abiCoder.decodeParameter('string', '0x' + result.substring(10)); var signature = 'Error(String)'; diff --git a/packages/web3-eth-contract/src/index.js b/packages/web3-eth-contract/src/index.js index 7d32b384ae7..684d4fcb497 100644 --- a/packages/web3-eth-contract/src/index.js +++ b/packages/web3-eth-contract/src/index.js @@ -186,6 +186,7 @@ var Contract = function Contract(jsonInterface, address, options) { this.defaultChain = this.constructor.defaultChain; this.defaultHardfork = this.constructor.defaultHardfork; this.defaultCommon = this.constructor.defaultCommon; + this.handleRevert = this.constructor.handleRevert; Object.defineProperty(this, 'defaultAccount', { get: function () { @@ -827,6 +828,7 @@ Contract.prototype._executeMethod = function _executeMethod(){ accounts: ethAccounts, // is eth.accounts (necessary for wallet signing) defaultAccount: _this._parent.defaultAccount, defaultBlock: _this._parent.defaultBlock, + handleRevert: _this._parent.handleRevert, abiCoder: abi })).createFunction(); @@ -904,6 +906,7 @@ Contract.prototype._executeMethod = function _executeMethod(){ defaultCommon: _this._parent.defaultCommon, defaultChain: _this._parent.defaultChain, defaultHardfork: _this._parent.defaultHardfork, + handleRevert: _this._parent.handleRevert, extraFormatters: extraFormatters, abiCoder: abi })).createFunction(); diff --git a/packages/web3-eth/src/index.js b/packages/web3-eth/src/index.js index dfbb56b218f..3a6d176e775 100644 --- a/packages/web3-eth/src/index.js +++ b/packages/web3-eth/src/index.js @@ -79,6 +79,7 @@ var Eth = function Eth() { }; + var handleRevert = false; var defaultAccount = null; var defaultBlock = 'latest'; var transactionBlockTimeout = 50; @@ -86,6 +87,23 @@ var Eth = function Eth() { var transactionPollingTimeout = 750; var defaultChain, defaultHardfork, defaultCommon; + Object.defineProperty(this, 'handleRevert', { + get: function () { + return handleRevert; + }, + set: function (val) { + handleRevert = val; + + // also set on the Contract object + _this.Contract.handleRevert = defaultCommon; + + // update handleRevert + methods.forEach(function(method) { + method.handleRevert = handleRevert; + }); + }, + enumerable: true + }); Object.defineProperty(this, 'defaultCommon', { get: function () { return defaultCommon; @@ -282,6 +300,7 @@ var Eth = function Eth() { this.Contract.transactionBlockTimeout = this.transactionBlockTimeout; this.Contract.transactionConfirmationBlocks = this.transactionConfirmationBlocks; this.Contract.transactionPollingTimeout = this.transactionPollingTimeout; + this.Contract.handleRevert = this.handleRevert; this.Contract.setProvider(this.currentProvider, this.accounts); // add IBAN @@ -595,6 +614,7 @@ var Eth = function Eth() { method.transactionBlockTimeout = _this.transactionBlockTimeout; method.transactionConfirmationBlocks = _this.transactionConfirmationBlocks; method.transactionPollingTimeout = _this.transactionPollingTimeout; + method.handleRevert = _this.handleRevert; }); }; diff --git a/test/e2e.method.signing.js b/test/e2e.method.signing.js index bc74bac95a1..5631de6c800 100644 --- a/test/e2e.method.signing.js +++ b/test/e2e.method.signing.js @@ -261,8 +261,6 @@ describe('transaction and message signing [ @E2E ]', function() { .eth .sendTransaction(tx) .on('error', function(err){ - console.log(err); - assert(err.message.includes('gas')) done(); }) From 357b59e01d1f2fb1f4cfd6146be035a586ff537a Mon Sep 17 00:00:00 2001 From: nivida Date: Mon, 2 Dec 2019 13:10:41 +0100 Subject: [PATCH 10/22] Basic contract updated, handleRevert option handling fixed, and e2e.method.send extended with severall revert test cases --- packages/web3-core-method/src/index.js | 138 +++++++++-------- packages/web3-eth/src/index.js | 2 +- test/e2e.method.send.js | 205 ++++++++++++++++++++----- test/sources/Basic.json | 170 +++++++++++--------- test/sources/Basic.sol | 10 +- 5 files changed, 343 insertions(+), 182 deletions(-) diff --git a/packages/web3-core-method/src/index.js b/packages/web3-core-method/src/index.js index 4eb7b856580..9f372c08819 100644 --- a/packages/web3-core-method/src/index.js +++ b/packages/web3-core-method/src/index.js @@ -61,7 +61,7 @@ var Method = function Method(options) { this.handleRevert = options.handleRevert; }; -Method.prototype.setRequestManager = function(requestManager, accounts) { +Method.prototype.setRequestManager = function (requestManager, accounts) { this.requestManager = requestManager; // reference to eth.accounts @@ -71,7 +71,7 @@ Method.prototype.setRequestManager = function(requestManager, accounts) { }; -Method.prototype.createFunction = function(requestManager, accounts) { +Method.prototype.createFunction = function (requestManager, accounts) { var func = this.buildCall(); func.call = this.call; @@ -80,7 +80,7 @@ Method.prototype.createFunction = function(requestManager, accounts) { return func; }; -Method.prototype.attachToObject = function(obj) { +Method.prototype.attachToObject = function (obj) { var func = this.buildCall(); func.call = this.call; var name = this.name.split('.'); @@ -99,7 +99,7 @@ Method.prototype.attachToObject = function(obj) { * @param {Array} arguments * @return {String} name of jsonrpc method */ -Method.prototype.getCall = function(args) { +Method.prototype.getCall = function (args) { return _.isFunction(this.call) ? this.call(args) : this.call; }; @@ -110,7 +110,7 @@ Method.prototype.getCall = function(args) { * @param {Array} arguments * @return {Function|Null} callback, if exists */ -Method.prototype.extractCallback = function(args) { +Method.prototype.extractCallback = function (args) { if (_.isFunction(args[args.length - 1])) { return args.pop(); // modify the args array! } @@ -123,7 +123,7 @@ Method.prototype.extractCallback = function(args) { * @param {Array} arguments * @throws {Error} if it is not */ -Method.prototype.validateArgs = function(args) { +Method.prototype.validateArgs = function (args) { if (args.length !== this.params) { throw errors.InvalidNumberOfParams(args.length, this.params, this.name); } @@ -136,14 +136,14 @@ Method.prototype.validateArgs = function(args) { * @param {Array} * @return {Array} */ -Method.prototype.formatInput = function(args) { +Method.prototype.formatInput = function (args) { var _this = this; if (!this.inputFormatter) { return args; } - return this.inputFormatter.map(function(formatter, index) { + return this.inputFormatter.map(function (formatter, index) { // bind this for defaultBlock, and defaultAccount return formatter ? formatter.call(_this, args[index]) : args[index]; }); @@ -156,11 +156,11 @@ Method.prototype.formatInput = function(args) { * @param {Object} * @return {Object} */ -Method.prototype.formatOutput = function(result) { +Method.prototype.formatOutput = function (result) { var _this = this; if (_.isArray(result)) { - return result.map(function(res) { + return result.map(function (res) { return _this.outputFormatter && res ? _this.outputFormatter(res) : res; }); } else { @@ -175,7 +175,7 @@ Method.prototype.formatOutput = function(result) { * @param {Array} args * @return {Object} */ -Method.prototype.toPayload = function(args) { +Method.prototype.toPayload = function (args) { var call = this.getCall(args); var callback = this.extractCallback(args); var params = this.formatInput(args); @@ -195,7 +195,7 @@ Method.prototype.toPayload = function(args) { }; -Method.prototype._confirmTransaction = function(defer, result, payload) { +Method.prototype._confirmTransaction = function (defer, result, payload) { var method = this, promiseResolved = false, canUnsubscribe = true, @@ -216,7 +216,7 @@ Method.prototype._confirmTransaction = function(defer, result, payload) { name: 'getBlockByNumber', call: 'eth_getBlockByNumber', params: 2, - inputFormatter: [formatters.inputBlockNumberFormatter, function(val) { + inputFormatter: [formatters.inputBlockNumberFormatter, function (val) { return !!val; }], outputFormatter: formatters.outputBlockFormatter @@ -248,19 +248,19 @@ Method.prototype._confirmTransaction = function(defer, result, payload) { ]; // attach methods to this._ethereumCall var _ethereumCall = {}; - _.each(_ethereumCalls, function(mthd) { + _.each(_ethereumCalls, function (mthd) { mthd.attachToObject(_ethereumCall); mthd.requestManager = method.requestManager; // assign rather than call setRequestManager() }); // fire "receipt" and confirmation events and resolve after - var checkConfirmation = function(existingReceipt, isPolling, err, blockHeader, sub) { + var checkConfirmation = function (existingReceipt, isPolling, err, blockHeader, sub) { if (!err) { // create fake unsubscribe if (!sub) { sub = { - unsubscribe: function() { + unsubscribe: function () { clearInterval(intervalId); } }; @@ -268,7 +268,7 @@ Method.prototype._confirmTransaction = function(defer, result, payload) { // if we have a valid receipt we don't need to send a request return (existingReceipt ? promiEvent.resolve(existingReceipt) : _ethereumCall.getTransactionReceipt(result)) // catch error from requesting receipt - .catch(function(err) { + .catch(function (err) { sub.unsubscribe(); promiseResolved = true; utils._fireError( @@ -281,7 +281,7 @@ Method.prototype._confirmTransaction = function(defer, result, payload) { ); }) // if CONFIRMATION listener exists check for confirmations, by setting canUnsubscribe = false - .then(async function(receipt) { + .then(async function (receipt) { if (!receipt || !receipt.blockHash) { throw new Error('Receipt missing or blockHash null'); } @@ -330,7 +330,7 @@ Method.prototype._confirmTransaction = function(defer, result, payload) { return receipt; }) // CHECK for CONTRACT DEPLOYMENT - .then(function(receipt) { + .then(function (receipt) { if (isContractDeployment && !promiseResolved) { @@ -351,7 +351,7 @@ Method.prototype._confirmTransaction = function(defer, result, payload) { return; } - _ethereumCall.getCode(receipt.contractAddress, function(e, code) { + _ethereumCall.getCode(receipt.contractAddress, function (e, code) { if (!code) { return; @@ -393,7 +393,7 @@ Method.prototype._confirmTransaction = function(defer, result, payload) { return receipt; }) // CHECK for normal tx check for receipt only - .then(async function(receipt) { + .then(async function (receipt) { if (!isContractDeployment && !promiseResolved) { if (!receipt.outOfGas && (!gasProvided || gasProvided !== receipt.gasUsed) && @@ -410,38 +410,38 @@ Method.prototype._confirmTransaction = function(defer, result, payload) { receiptJSON = JSON.stringify(receipt, null, 2); if (receipt.status === false || receipt.status === '0x0') { - try { - var revertMessage = null; - - if (method.handleRevert) { - revertMessage = await method.getRevertReason( - payload.params[0], - receipt.blockNumber - ); - - if (revertMessage) { - utils._fireError( - errors.RevertInstructionError(revertMessage.reason, revertMessage.signature), - defer.eventEmitter, - defer.reject, - payload.callback, - receipt - ); - } else { - throw false; - } + try { + var revertMessage = null; + + if (method.handleRevert) { + revertMessage = await method.getRevertReason( + payload.params[0], + receipt.blockNumber + ); + + if (revertMessage) { + utils._fireError( + errors.RevertInstructionError(revertMessage.reason, revertMessage.signature), + defer.eventEmitter, + defer.reject, + payload.callback, + receipt + ); } else { throw false; } - } catch (error) { - utils._fireError( - new Error('Transaction has been reverted by the EVM:\n' + receiptJSON), - defer.eventEmitter, - defer.reject, - null, - receipt - ); + } else { + throw false; } + } catch (error) { + utils._fireError( + new Error('Transaction has been reverted by the EVM:\n' + receiptJSON), + defer.eventEmitter, + defer.reject, + null, + receipt + ); + } } else { utils._fireError( new Error('Transaction ran out of gas. Please provide more gas:\n' + receiptJSON), @@ -461,7 +461,7 @@ Method.prototype._confirmTransaction = function(defer, result, payload) { }) // time out the transaction if not mined after 50 blocks - .catch(function() { + .catch(function () { timeoutCount++; // check to see if we are http polling @@ -501,7 +501,7 @@ Method.prototype._confirmTransaction = function(defer, result, payload) { }; // start watching for confirmation depending on the support features of the provider - var startWatching = function(existingReceipt) { + var startWatching = function (existingReceipt) { // if provider allows PUB/SUB if (_.isFunction(this.requestManager.provider.on)) { _ethereumCall.subscribe('newBlockHeaders', checkConfirmation.bind(null, existingReceipt, false)); @@ -513,7 +513,7 @@ Method.prototype._confirmTransaction = function(defer, result, payload) { // first check if we already have a confirmed transaction _ethereumCall.getTransactionReceipt(result) - .then(function(receipt) { + .then(function (receipt) { if (receipt && receipt.blockHash) { if (defer.eventEmitter.listeners('confirmation').length > 0) { // We must keep on watching for new Blocks, if a confirmation listener is present @@ -525,14 +525,14 @@ Method.prototype._confirmTransaction = function(defer, result, payload) { startWatching(); } }) - .catch(function() { + .catch(function () { if (!promiseResolved) startWatching(); }); }; -var getWallet = function(from, accounts) { +var getWallet = function (from, accounts) { var wallet = null; // is index given @@ -551,18 +551,18 @@ var getWallet = function(from, accounts) { return wallet; }; -Method.prototype.buildCall = function() { +Method.prototype.buildCall = function () { var method = this, isSendTx = (method.call === 'eth_sendTransaction' || method.call === 'eth_sendRawTransaction'), // || method.call === 'personal_sendTransaction' isCall = (method.call === 'eth_call'); // actual send function - var send = function() { + var send = function () { var defer = promiEvent(!isSendTx), payload = method.toPayload(Array.prototype.slice.call(arguments)); // CALLBACK function - var sendTxCallback = function(err, result) { + var sendTxCallback = function (err, result) { if (method.handleRevert && !err && isCall && (method.isRevertReasonString(result) && method.abiCoder)) { var reason = method.abiCoder.decodeParameter('string', '0x' + result.substring(10)); var signature = 'Error(String)'; @@ -619,7 +619,7 @@ Method.prototype.buildCall = function() { }; // SENDS the SIGNED SIGNATURE - var sendSignedTx = function(sign) { + var sendSignedTx = function (sign) { var signedPayload = _.extend({}, payload, { method: 'eth_sendRawTransaction', @@ -630,7 +630,7 @@ Method.prototype.buildCall = function() { }; - var sendRequest = function(payload, method) { + var sendRequest = function (payload, method) { if (method && method.accounts && method.accounts.wallet && method.accounts.wallet.length) { var wallet; @@ -659,11 +659,12 @@ Method.prototype.buildCall = function() { return method.accounts.signTransaction(txOptions, wallet.privateKey) .then(sendSignedTx) - .catch(function(err) { + .catch(function (err) { if (_.isFunction(defer.eventEmitter.listeners) && defer.eventEmitter.listeners('error').length) { defer.eventEmitter.emit('error', err); defer.eventEmitter.removeAllListeners(); - defer.eventEmitter.catch(function() {}); + defer.eventEmitter.catch(function () { + }); } defer.reject(err); }); @@ -702,7 +703,7 @@ Method.prototype.buildCall = function() { params: 0 })).createFunction(method.requestManager); - getGasPrice(function(err, gasPrice) { + getGasPrice(function (err, gasPrice) { if (gasPrice) { payload.params[0].gasPrice = gasPrice; @@ -735,21 +736,22 @@ Method.prototype.buildCall = function() { * * @returns {Promise} */ -Method.prototype.getRevertReason = function(txOptions, blockNumber) { +Method.prototype.getRevertReason = function (txOptions, blockNumber) { var self = this; - return new Promise(function(resolve, reject) { + return new Promise(function (resolve, reject) { (new Method({ name: 'call', call: 'eth_call', params: 2, - abiCoder: self.abiCoder + abiCoder: self.abiCoder, + handleRevert: true })) .createFunction(self.requestManager)(txOptions, utils.numberToHex(blockNumber)) - .then(function() { + .then(function () { resolve(false); }) - .catch(function(error) { + .catch(function (error) { if (error.reason) { resolve({ reason: error.reason, @@ -781,7 +783,7 @@ Method.prototype.isRevertReasonString = function (data) { * @method request * @return {Object} jsonrpc request */ -Method.prototype.request = function() { +Method.prototype.request = function () { var payload = this.toPayload(Array.prototype.slice.call(arguments)); payload.format = this.formatOutput.bind(this); return payload; diff --git a/packages/web3-eth/src/index.js b/packages/web3-eth/src/index.js index 3a6d176e775..8cdb5c1f564 100644 --- a/packages/web3-eth/src/index.js +++ b/packages/web3-eth/src/index.js @@ -95,7 +95,7 @@ var Eth = function Eth() { handleRevert = val; // also set on the Contract object - _this.Contract.handleRevert = defaultCommon; + _this.Contract.handleRevert = handleRevert; // update handleRevert methods.forEach(function(method) { diff --git a/test/e2e.method.send.js b/test/e2e.method.send.js index d9bfa9f1713..268cb3d45e2 100644 --- a/test/e2e.method.send.js +++ b/test/e2e.method.send.js @@ -3,7 +3,7 @@ var Basic = require('./sources/Basic'); var utils = require('./helpers/test.utils'); var Web3 = utils.getWeb3(); -describe('method.send [ @E2E ]', function() { +describe('method.send [ @E2E ]', function () { var web3; var accounts; var basic; @@ -16,16 +16,16 @@ describe('method.send [ @E2E ]', function() { gas: 4000000 }; - describe('http', function() { - before(async function(){ + describe('http', function () { + before(async function () { web3 = new Web3('http://localhost:8545'); accounts = await web3.eth.getAccounts(); basic = new web3.eth.Contract(Basic.abi, basicOptions); instance = await basic.deploy().send({from: accounts[0]}); - }) + }); - it('returns a receipt', async function(){ + it('returns a receipt', async function () { var receipt = await instance .methods .setValue('1') @@ -35,7 +35,7 @@ describe('method.send [ @E2E ]', function() { assert(web3.utils.isHexStrict(receipt.transactionHash)); }); - it('errors on OOG', async function(){ + it('errors on OOG', async function () { try { await instance .methods @@ -44,12 +44,12 @@ describe('method.send [ @E2E ]', function() { assert.fail(); - } catch(err){ - assert(err.message.includes('gas')) + } catch (err) { + assert(err.message.includes('gas')); } }); - it('errors on revert', async function(){ + it('errors on revert', async function () { try { await instance .methods @@ -58,20 +58,20 @@ describe('method.send [ @E2E ]', function() { assert.fail(); - } catch(err){ + } catch (err) { var receipt = utils.extractReceipt(err.message); - assert(err.message.includes('revert')) + assert(err.message.includes('revert')); assert(receipt.status === false); } }); }); - describe('ws', function() { + describe('ws', function () { // Websockets extremely erratic for geth instamine... if (process.env.GETH_INSTAMINE) return; - before(async function(){ + before(async function () { var port = utils.getWebsocketPort(); web3 = new Web3('ws://localhost:' + port); @@ -81,7 +81,7 @@ describe('method.send [ @E2E ]', function() { instance = await basic.deploy().send({from: accounts[0]}); }) - it('returns a receipt', async function(){ + it('returns a receipt', async function () { var receipt = await instance .methods .setValue('1') @@ -91,7 +91,7 @@ describe('method.send [ @E2E ]', function() { assert(web3.utils.isHexStrict(receipt.transactionHash)); }); - it('errors on OOG', async function(){ + it('errors on OOG', async function () { try { await instance .methods @@ -100,12 +100,12 @@ describe('method.send [ @E2E ]', function() { assert.fail(); - } catch(err){ - assert(err.message.includes('gas')) + } catch (err) { + assert(err.message.includes('gas')); } }); - it('errors on revert', async function(){ + it('errors on revert', async function () { try { await instance .methods @@ -114,37 +114,37 @@ describe('method.send [ @E2E ]', function() { assert.fail(); - } catch(err){ + } catch (err) { var receipt = utils.extractReceipt(err.message); - assert(err.message.includes('revert')) + assert(err.message.includes('revert')); assert(receipt.status === false); } }); - it('fires the transactionHash event', function(done){ + it('fires the transactionHash event', function (done) { instance .methods .setValue('1') .send({from: accounts[0]}) .on('transactionHash', hash => { - assert(web3.utils.isHex(hash)) + assert(web3.utils.isHex(hash)); done(); - }) + }); }); - it('fires the receipt event', function(done){ + it('fires the receipt event', function (done) { instance .methods .setValue('1') .send({from: accounts[0]}) .on('receipt', receipt => { - assert(receipt.status === true) + assert(receipt.status === true); done(); - }) - }) + }); + }); - it('fires the confirmation handler', function(){ + it('fires the confirmation handler', function () { return new Promise(async (resolve, reject) => { var startBlock = await web3.eth.getBlockNumber(); @@ -159,25 +159,25 @@ describe('method.send [ @E2E ]', function() { assert(endBlock >= (startBlock + 2)); resolve(); } - }) + }); // Necessary for instamine, should not interfere with automine. await utils.mine(web3, accounts[0]); }); }); - it('fires the error handler on OOG', function(done){ + it('fires the error handler on OOG', function (done) { instance .methods .setValue('1') .send({from: accounts[0], gas: 100}) .on('error', err => { - assert(err.message.includes('gas')) + assert(err.message.includes('gas')); done(); - }) - }) + }); + }); - it('fires the error handler on revert', function(done){ + it('fires the error handler on revert', function (done) { instance .methods .reverts() @@ -185,8 +185,141 @@ describe('method.send [ @E2E ]', function() { .on('error', err => { assert(err.message.includes('revert')); done(); - }) - }) + }); + }); + }); + + describe('with revert handling activated', function () { + before(async function () { + web3 = new Web3('http://localhost:8545'); + accounts = await web3.eth.getAccounts(); + + web3.eth.handleRevert = true; + basic = new web3.eth.Contract(Basic.abi, basicOptions); + + instance = await basic.deploy().send({from: accounts[0]}); + }); + + it('errors on OOG', async function () { + try { + await instance + .methods + .setValue('1') + .send({from: accounts[0], gas: 100}); + + assert.fail(); + + } catch (err) { + assert(err.message.includes('gas')); + } + }); + + it('Promise throws on revert', async function () { + try { + await instance + .methods + .reverts() + .send({from: accounts[0]}); + + assert.fail(); + + } catch (err) { + assert(err.signature === 'Error(String)'); + assert(err.reason === 'REVERTED WITH REVERT'); + assert(err.message.includes('reverted')); + } + }); + + it('Promise throws on failing require with a revert reason given', async function () { + try { + await instance + .methods + .requireWithReason() + .send({from: accounts[0]}); + + assert.fail(); + + } catch (err) { + assert(err.signature === 'Error(String)'); + assert(err.reason === 'REVERTED WITH REQUIRE'); + assert(err.message.includes('reverted')); + } + }); + + it('Promise throws on failing require without a revert reason given', async function () { + try { + await instance + .methods + .requireWithoutReason() + .send({from: accounts[0]}); + + assert.fail(); + + } catch (err) { + var receipt = utils.extractReceipt(err.message); + + assert(receipt.status === false); + assert(err.signature === undefined); + assert(err.reason === undefined); + assert(err.message.includes('EVM')); + } + }); + + it('fires the error handler on OOG', function (done) { + instance + .methods + .setValue('1') + .send({from: accounts[0], gas: 100}) + .on('error', err => { + assert(err.message.includes('gas')); + done(); + }); + }); + + it('fires the error handler on failing require without a revert reason given', function (done) { + instance + .methods + .requireWithoutReason() + .send({from: accounts[0]}) + .on('error', (err, receipt) => { + assert(receipt.status === false); + assert(err.signature === undefined); + assert(err.reason === undefined); + assert(err.message.includes('EVM')); + + done(); + }); + }); + + it('fires the error handler on failing require with a revert reason given', function (done) { + instance + .methods + .requireWithReason() + .send({from: accounts[0]}) + .on('error', (err, receipt) => { + assert(receipt.status === false); + assert(err.signature === 'Error(String)'); + assert(err.reason === 'REVERTED WITH REQUIRE'); + assert(err.message.includes('reverted')); + + done(); + }); + }); + + it('fires the error handler on revert', function (done) { + instance + .methods + .reverts() + .send({from: accounts[0]}) + .on('error', (err, receipt) => { + assert(receipt.status === false); + assert(err.signature === 'Error(String)'); + assert(err.reason === 'REVERTED WITH REVERT'); + assert(err.message.includes('reverted')); + + done(); + }); + }); }); }); diff --git a/test/sources/Basic.json b/test/sources/Basic.json index d7b7c8f41e9..17eea6aaa9f 100644 --- a/test/sources/Basic.json +++ b/test/sources/Basic.json @@ -1,87 +1,105 @@ { - "contractName": "Basic", - "abi": [ - { - "constant": true, - "inputs": [], - "name": "getValue", - "outputs": [ + "contractName": "Basic", + "abi": [ { - "internalType": "uint256", - "name": "val", - "type": "uint256" - } - ], - "payable": false, - "stateMutability": "view", - "type": "function" - }, - { - "constant": false, - "inputs": [], - "name": "reverts", - "outputs": [], - "payable": false, - "stateMutability": "nonpayable", - "type": "function" - }, - { - "constant": false, - "inputs": [ + "anonymous": false, + "inputs": [ + { + "indexed": false, + "internalType": "address", + "name": "addr", + "type": "address" + }, + { + "indexed": true, + "internalType": "uint256", + "name": "val", + "type": "uint256" + } + ], + "name": "BasicEvent", + "type": "event" + }, { - "internalType": "uint256", - "name": "_value", - "type": "uint256" - } - ], - "name": "setValue", - "outputs": [], - "payable": false, - "stateMutability": "nonpayable", - "type": "function" - }, - { - "constant": false, - "inputs": [ + "constant": true, + "inputs": [], + "name": "getValue", + "outputs": [ + { + "internalType": "uint256", + "name": "val", + "type": "uint256" + } + ], + "payable": false, + "stateMutability": "view", + "type": "function" + }, { - "internalType": "address", - "name": "addr", - "type": "address" + "constant": false, + "inputs": [ + { + "internalType": "uint256", + "name": "_value", + "type": "uint256" + } + ], + "name": "setValue", + "outputs": [], + "payable": false, + "stateMutability": "nonpayable", + "type": "function" }, { - "internalType": "uint256", - "name": "val", - "type": "uint256" - } - ], - "name": "firesEvent", - "outputs": [], - "payable": false, - "stateMutability": "nonpayable", - "type": "function" - }, - { - "anonymous": false, - "inputs": [ + "constant": false, + "inputs": [], + "name": "requireWithoutReason", + "outputs": [], + "payable": false, + "stateMutability": "nonpayable", + "type": "function" + }, + { + "constant": false, + "inputs": [], + "name": "requireWithReason", + "outputs": [], + "payable": false, + "stateMutability": "nonpayable", + "type": "function" + }, { - "indexed": false, - "internalType": "address", - "name": "addr", - "type": "address" + "constant": false, + "inputs": [], + "name": "reverts", + "outputs": [], + "payable": false, + "stateMutability": "nonpayable", + "type": "function" }, { - "indexed": true, - "internalType": "uint256", - "name": "val", - "type": "uint256" + "constant": false, + "inputs": [ + { + "internalType": "address", + "name": "addr", + "type": "address" + }, + { + "internalType": "uint256", + "name": "val", + "type": "uint256" + } + ], + "name": "firesEvent", + "outputs": [], + "payable": false, + "stateMutability": "nonpayable", + "type": "function" } - ], - "name": "BasicEvent", - "type": "event" - } - ], - "bytecode": "0x608060405234801561001057600080fd5b506101b2806100206000396000f3fe608060405234801561001057600080fd5b506004361061004c5760003560e01c806320965255146100515780633bccbbc91461006f578063552410771461007957806385693803146100a7575b600080fd5b6100596100f5565b6040518082815260200191505060405180910390f35b6100776100fe565b005b6100a56004803603602081101561008f57600080fd5b810190808035906020019092919050505061010b565b005b6100f3600480360360408110156100bd57600080fd5b81019080803573ffffffffffffffffffffffffffffffffffffffff16906020019092919080359060200190929190505050610115565b005b60008054905090565b600061010957600080fd5b565b8060008190555050565b807f929af6b98ce8455e51b1e90cceeeec03ce38d7c7396f1f0b0233d043c898a29c83604051808273ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16815260200191505060405180910390a2505056fea265627a7a72315820d53bd5b7c0f2ec6f164aed5f4f24b456fedf21b3217892dce88f94d21192313f64736f6c634300050b0032", - "deployedBytecode": "0x608060405234801561001057600080fd5b506004361061004c5760003560e01c806320965255146100515780633bccbbc91461006f578063552410771461007957806385693803146100a7575b600080fd5b6100596100f5565b6040518082815260200191505060405180910390f35b6100776100fe565b005b6100a56004803603602081101561008f57600080fd5b810190808035906020019092919050505061010b565b005b6100f3600480360360408110156100bd57600080fd5b81019080803573ffffffffffffffffffffffffffffffffffffffff16906020019092919080359060200190929190505050610115565b005b60008054905090565b600061010957600080fd5b565b8060008190555050565b807f929af6b98ce8455e51b1e90cceeeec03ce38d7c7396f1f0b0233d043c898a29c83604051808273ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16815260200191505060405180910390a2505056fea265627a7a72315820d53bd5b7c0f2ec6f164aed5f4f24b456fedf21b3217892dce88f94d21192313f64736f6c634300050b0032", - "linkReferences": {}, - "deployedLinkReferences": {} + ], + "bytecode": "0x608060405234801561001057600080fd5b506102c0806100206000396000f3fe608060405234801561001057600080fd5b50600436106100625760003560e01c806320965255146100675780633bccbbc914610085578063552410771461008f578063844d6a32146100bd57806385693803146100c7578063ae012ede14610115575b600080fd5b61006f61011f565b6040518082815260200191505060405180910390f35b61008d610128565b005b6100bb600480360360208110156100a557600080fd5b8101908080359060200190929190505050610196565b005b6100c56101a0565b005b610113600480360360408110156100dd57600080fd5b81019080803573ffffffffffffffffffffffffffffffffffffffff16906020019092919080359060200190929190505050610216565b005b61011d61027e565b005b60008054905090565b6040517f08c379a00000000000000000000000000000000000000000000000000000000081526004018080602001828103825260148152602001807f524556455254454420574954482052455645525400000000000000000000000081525060200191505060405180910390fd5b8060008190555050565b6000610214576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004018080602001828103825260158152602001807f524556455254454420574954482052455155495245000000000000000000000081525060200191505060405180910390fd5b565b807f929af6b98ce8455e51b1e90cceeeec03ce38d7c7396f1f0b0233d043c898a29c83604051808273ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16815260200191505060405180910390a25050565b600061028957600080fd5b56fea265627a7a72315820884747d21b82b34af8a11b3a5556963c8f2c8cf51df66f4c73e3ea5d33b1d44964736f6c634300050c0032", + "deployedBytecode": "0x608060405234801561001057600080fd5b50600436106100625760003560e01c806320965255146100675780633bccbbc914610085578063552410771461008f578063844d6a32146100bd57806385693803146100c7578063ae012ede14610115575b600080fd5b61006f61011f565b6040518082815260200191505060405180910390f35b61008d610128565b005b6100bb600480360360208110156100a557600080fd5b8101908080359060200190929190505050610196565b005b6100c56101a0565b005b610113600480360360408110156100dd57600080fd5b81019080803573ffffffffffffffffffffffffffffffffffffffff16906020019092919080359060200190929190505050610216565b005b61011d61027e565b005b60008054905090565b6040517f08c379a00000000000000000000000000000000000000000000000000000000081526004018080602001828103825260148152602001807f524556455254454420574954482052455645525400000000000000000000000081525060200191505060405180910390fd5b8060008190555050565b6000610214576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004018080602001828103825260158152602001807f524556455254454420574954482052455155495245000000000000000000000081525060200191505060405180910390fd5b565b807f929af6b98ce8455e51b1e90cceeeec03ce38d7c7396f1f0b0233d043c898a29c83604051808273ffffffffffffffffffffffffffffffffffffffff1673ffffffffffffffffffffffffffffffffffffffff16815260200191505060405180910390a25050565b600061028957600080fd5b56fea265627a7a72315820884747d21b82b34af8a11b3a5556963c8f2c8cf51df66f4c73e3ea5d33b1d44964736f6c634300050c0032", + "linkReferences": {}, + "deployedLinkReferences": {} } diff --git a/test/sources/Basic.sol b/test/sources/Basic.sol index debc40558d6..0e76b8ef184 100644 --- a/test/sources/Basic.sol +++ b/test/sources/Basic.sol @@ -16,10 +16,18 @@ contract Basic { value = _value; } - function reverts() public { + function requireWithoutReason() public { require(false); } + function requireWithReason() public { + require(false, 'REVERTED WITH REQUIRE'); + } + + function reverts() public { + revert('REVERTED WITH REVERT'); + } + function firesEvent(address addr, uint val) public { emit BasicEvent(addr, val); } From 5f67c48638d10d84cb478cd8aa7c8de7e18f264f Mon Sep 17 00:00:00 2001 From: nivida Date: Mon, 2 Dec 2019 13:36:25 +0100 Subject: [PATCH 11/22] revert instruction test cases added to e2e.method.call and assertions in e2e.method.send improved --- test/e2e.method.call.js | 94 ++++++++++++++++++++++++++--------------- test/e2e.method.send.js | 32 +++++++------- 2 files changed, 77 insertions(+), 49 deletions(-) diff --git a/test/e2e.method.call.js b/test/e2e.method.call.js index db673aa08c9..e78ad73b69a 100644 --- a/test/e2e.method.call.js +++ b/test/e2e.method.call.js @@ -4,7 +4,7 @@ var Misc = require('./sources/Misc'); var utils = require('./helpers/test.utils'); var Web3 = utils.getWeb3(); -describe('method.call [ @E2E ]', function() { +describe('method.call [ @E2E ]', function () { var web3; var accounts; var basic; @@ -23,46 +23,74 @@ describe('method.call [ @E2E ]', function() { gas: 4000000 }; - before(async function(){ - web3 = new Web3('http://localhost:8545'); - accounts = await web3.eth.getAccounts(); + describe('http', function () { + before(async function () { + web3 = new Web3('http://localhost:8545'); + accounts = await web3.eth.getAccounts(); - basic = new web3.eth.Contract(Basic.abi, basicOptions); - instance = await basic.deploy().send({from: accounts[0]}); - }) + basic = new web3.eth.Contract(Basic.abi, basicOptions); + instance = await basic.deploy().send({from: accounts[0]}); + }) - it('retrieves a uint value', async function(){ - var expected = '1'; + it('retrieves a uint value', async function () { + var expected = '1'; - await instance - .methods - .setValue(expected) - .send({from: accounts[0]}); + await instance + .methods + .setValue(expected) + .send({from: accounts[0]}); + + var value = await instance + .methods + .getValue() + .call({from: accounts[0]}); + + assert.equal(value, expected); + }); - var value = await instance - .methods - .getValue() - .call({from: accounts[0]}); + it('errors correctly when abi and bytecode do not match', async function () { + // Misc n.eq Basic + var wrong = new web3.eth.Contract(Basic.abi, miscOptions); + var wrongInstance = await wrong.deploy().send({from: accounts[0]}); - assert.equal(value, expected); + try { + await wrongInstance + .methods + .getValue() + .call(); + + assert.fail(); + + } catch (err) { + assert(err.message.includes("Returned values aren't valid")); + assert(err.message.includes('the correct ABI')); + } + }) }); - it('errors correctly when abi and bytecode do not match', async function(){ - // Misc n.eq Basic - var wrong = new web3.eth.Contract(Basic.abi, miscOptions); - var wrongInstance = await wrong.deploy().send({from: accounts[0]}); + describe('revert handling', function () { + before(async function () { + web3 = new Web3('http://localhost:8545'); + accounts = await web3.eth.getAccounts(); - try { - await wrongInstance - .methods - .getValue() - .call(); + web3.eth.handleRevert = true; + basic = new web3.eth.Contract(Basic.abi, basicOptions); + instance = await basic.deploy().send({from: accounts[0]}); + }); - assert.fail(); + it('returns the expected revert reason string', async function () { + try { + await instance + .methods + .reverts() + .call({from: accounts[0]}); - } catch(err){ - assert(err.message.includes("Returned values aren't valid")); - assert(err.message.includes('the correct ABI')); - } - }) + assert.fail(); + } catch(error) { + assert(error.message.includes('reverted')); + assert.equal(error.reason, 'REVERTED WITH REVERT'); + assert.equal(error.signature, 'Error(String)'); + } + }); + }); }); diff --git a/test/e2e.method.send.js b/test/e2e.method.send.js index 268cb3d45e2..79b17a45191 100644 --- a/test/e2e.method.send.js +++ b/test/e2e.method.send.js @@ -224,8 +224,8 @@ describe('method.send [ @E2E ]', function () { assert.fail(); } catch (err) { - assert(err.signature === 'Error(String)'); - assert(err.reason === 'REVERTED WITH REVERT'); + assert.equal(err.signature, 'Error(String)'); + assert.equal(err.reason, 'REVERTED WITH REVERT'); assert(err.message.includes('reverted')); } }); @@ -240,8 +240,8 @@ describe('method.send [ @E2E ]', function () { assert.fail(); } catch (err) { - assert(err.signature === 'Error(String)'); - assert(err.reason === 'REVERTED WITH REQUIRE'); + assert.equal(err.signature, 'Error(String)'); + assert.equal(err.reason, 'REVERTED WITH REQUIRE'); assert(err.message.includes('reverted')); } }); @@ -258,9 +258,9 @@ describe('method.send [ @E2E ]', function () { } catch (err) { var receipt = utils.extractReceipt(err.message); - assert(receipt.status === false); - assert(err.signature === undefined); - assert(err.reason === undefined); + assert.equal(receipt.status, false); + assert.equal(err.signature, undefined); + assert.equal(err.reason, undefined); assert(err.message.includes('EVM')); } }); @@ -282,9 +282,9 @@ describe('method.send [ @E2E ]', function () { .requireWithoutReason() .send({from: accounts[0]}) .on('error', (err, receipt) => { - assert(receipt.status === false); - assert(err.signature === undefined); - assert(err.reason === undefined); + assert.equal(receipt.status, false); + assert.equal(err.signature, undefined); + assert.equal(err.reason, undefined); assert(err.message.includes('EVM')); done(); @@ -297,9 +297,9 @@ describe('method.send [ @E2E ]', function () { .requireWithReason() .send({from: accounts[0]}) .on('error', (err, receipt) => { - assert(receipt.status === false); - assert(err.signature === 'Error(String)'); - assert(err.reason === 'REVERTED WITH REQUIRE'); + assert.equal(receipt.status, false); + assert.equal(err.signature, 'Error(String)'); + assert.equal(err.reason, 'REVERTED WITH REQUIRE'); assert(err.message.includes('reverted')); done(); @@ -312,9 +312,9 @@ describe('method.send [ @E2E ]', function () { .reverts() .send({from: accounts[0]}) .on('error', (err, receipt) => { - assert(receipt.status === false); - assert(err.signature === 'Error(String)'); - assert(err.reason === 'REVERTED WITH REVERT'); + assert.equal(receipt.status, false); + assert.equal(err.signature, 'Error(String)'); + assert.equal(err.reason, 'REVERTED WITH REVERT'); assert(err.message.includes('reverted')); done(); From 2561b81628a9e42d31bf7b7d121f72b6174e2e77 Mon Sep 17 00:00:00 2001 From: nivida Date: Mon, 2 Dec 2019 13:38:29 +0100 Subject: [PATCH 12/22] eth.handleRevert test added --- test/eth.handleRevert.js | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 test/eth.handleRevert.js diff --git a/test/eth.handleRevert.js b/test/eth.handleRevert.js new file mode 100644 index 00000000000..f10f78e1bda --- /dev/null +++ b/test/eth.handleRevert.js @@ -0,0 +1,25 @@ +var chai = require('chai'); +var assert = chai.assert; +var Eth = require('../packages/web3-eth'); + +var eth = new Eth(); + +var setValue = true; + +describe('web3.eth', function () { + describe('handleRevert', function () { + it('should check if handleRevert is set to proper value', function () { + assert.equal(eth.handleRevert, false); + assert.equal(eth.Contract.handleRevert, false); + assert.equal(eth.getCode.method.handleRevert, false); + }); + it('should set handleRevert for all sub packages is set to proper value, if Eth package is changed', function () { + eth.handleRevert = setValue; + + assert.equal(eth.handleRevert, setValue); + assert.equal(eth.Contract.handleRevert, setValue); + assert.equal(eth.getCode.method.handleRevert, setValue); + }); + }); +}); + From 62a82140455d79e1b72556f491d45aece682c7df Mon Sep 17 00:00:00 2001 From: nivida Date: Mon, 2 Dec 2019 14:03:28 +0100 Subject: [PATCH 13/22] Inline comments added to the Method object because of the code complexity --- packages/web3-core-method/src/index.js | 9 ++++++--- test/eth.handleRevert.js | 3 ++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/web3-core-method/src/index.js b/packages/web3-core-method/src/index.js index 9f372c08819..e00d5d4e081 100644 --- a/packages/web3-core-method/src/index.js +++ b/packages/web3-core-method/src/index.js @@ -414,12 +414,13 @@ Method.prototype._confirmTransaction = function (defer, result, payload) { var revertMessage = null; if (method.handleRevert) { + // Get revert reason string with eth_call revertMessage = await method.getRevertReason( payload.params[0], receipt.blockNumber ); - if (revertMessage) { + if (revertMessage) { // Only throw a revert error if a revert reason is existing utils._fireError( errors.RevertInstructionError(revertMessage.reason, revertMessage.signature), defer.eventEmitter, @@ -428,12 +429,13 @@ Method.prototype._confirmTransaction = function (defer, result, payload) { receipt ); } else { - throw false; + throw false; // Throw false and let the try/catch statement handle the error correctly after } } else { - throw false; + throw false; // Throw false and let the try/catch statement handle the error correctly after } } catch (error) { + // Throw an normal revert error if no revert reason is given or the detection of it is disabled utils._fireError( new Error('Transaction has been reverted by the EVM:\n' + receiptJSON), defer.eventEmitter, @@ -443,6 +445,7 @@ Method.prototype._confirmTransaction = function (defer, result, payload) { ); } } else { + // Throw OOG if status is true and provided gas and used gas are equal utils._fireError( new Error('Transaction ran out of gas. Please provide more gas:\n' + receiptJSON), defer.eventEmitter, diff --git a/test/eth.handleRevert.js b/test/eth.handleRevert.js index f10f78e1bda..eae3b8db9bd 100644 --- a/test/eth.handleRevert.js +++ b/test/eth.handleRevert.js @@ -13,7 +13,8 @@ describe('web3.eth', function () { assert.equal(eth.Contract.handleRevert, false); assert.equal(eth.getCode.method.handleRevert, false); }); - it('should set handleRevert for all sub packages is set to proper value, if Eth package is changed', function () { + + it('should set handleRevert for all sub packages', function () { eth.handleRevert = setValue; assert.equal(eth.handleRevert, setValue); From 4e5f4c8647be116f2ad27fb4057ca6ad4aba7d1f Mon Sep 17 00:00:00 2001 From: nivida Date: Mon, 2 Dec 2019 14:14:32 +0100 Subject: [PATCH 14/22] web3-eth and web3-eth-contract documentation extended with 'handleRevert' --- docs/web3-eth-contract.rst | 20 ++++++++++++++++++++ docs/web3-eth.rst | 26 ++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/docs/web3-eth-contract.rst b/docs/web3-eth-contract.rst index 43db0d793c1..d1d7fb059ac 100644 --- a/docs/web3-eth-contract.rst +++ b/docs/web3-eth-contract.rst @@ -362,6 +362,26 @@ Returns ------------------------------------------------------------------------------ +.. _eth-contract-module-handlerevert: + +handleRevert +============ + +.. code-block:: javascript + + web3.eth.Contract.handleRevert + contract.handleRevert // on contract instance + +The ``handleRevert`` options property does default to ``false`` and will return the revert reason string if enabled on :ref:`send ` or :ref:`call ` of a contract method. + +------- +Returns +------- + +``boolean``: The current value of ``handleRevert`` (default: false) + +------------------------------------------------------------------------------ + options ========= diff --git a/docs/web3-eth.rst b/docs/web3-eth.rst index 905af97035d..4974bbc307d 100644 --- a/docs/web3-eth.rst +++ b/docs/web3-eth.rst @@ -402,6 +402,32 @@ Returns ------------------------------------------------------------------------------ +.. _web3-module-handlerevert: + +handleRevert +============ + +.. code-block:: javascript + + web3.eth.handleRevert + +The ``handleRevert`` options property does default to ``false`` and will return the revert reason string if enabled for the following methods: + +- :ref:`web3.eth.call() ` +- :ref:`web3.eth.sendTransaction() ` +- :ref:`web3.eth.sendSignedTransaction() ` +- :ref:`contract.methods.myMethod(...).send(...) ` +- :ref:`contract.methods.myMethod(...).call(...) ` + + +------- +Returns +------- + +``boolean``: The current value of ``handleRevert`` (default: false) + +------------------------------------------------------------------------------ + getProtocolVersion ===================== From 842e2b8214fabf9ab491021b413285bbbab0cb5a Mon Sep 17 00:00:00 2001 From: nivida Date: Mon, 2 Dec 2019 14:33:17 +0100 Subject: [PATCH 15/22] CHANGELOG.md updated --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21edc342b91..624467f656a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -107,6 +107,7 @@ Released with 1.0.0-beta.37 code base. - ``eth_getProof`` as ``getProof`` added to web3-eth package (#3220) - ``BN`` and ``BigNumber`` objects are now supported by the ``abi.encodeParameter(s)`` method (#3238) - ``getPendingTransactions`` added to web3-eth package (#3239) +- Revert instruction handling added which can get activated with the ``handleRevert`` module property (#3248) ### Changed From 9db572c4560f9780abdd4f2f0e84065412794cfd Mon Sep 17 00:00:00 2001 From: nivida Date: Mon, 2 Dec 2019 18:08:28 +0100 Subject: [PATCH 16/22] Contract options extended to be able to configure singles contracts without re-loading of the object methods --- packages/web3-eth-contract/src/index.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/web3-eth-contract/src/index.js b/packages/web3-eth-contract/src/index.js index 684d4fcb497..805cbf98363 100644 --- a/packages/web3-eth-contract/src/index.js +++ b/packages/web3-eth-contract/src/index.js @@ -180,13 +180,13 @@ var Contract = function Contract(jsonInterface, address, options) { // get default account from the Class var defaultAccount = this.constructor.defaultAccount; var defaultBlock = this.constructor.defaultBlock || 'latest'; - this.transactionBlockTimeout = this.constructor.transactionBlockTimeout; - this.transactionConfirmationBlocks = this.constructor.transactionConfirmationBlocks; - this.transactionPollingTimeout = this.constructor.transactionPollingTimeout; - this.defaultChain = this.constructor.defaultChain; - this.defaultHardfork = this.constructor.defaultHardfork; - this.defaultCommon = this.constructor.defaultCommon; - this.handleRevert = this.constructor.handleRevert; + this.transactionBlockTimeout = this.options.transactionBlockTimeout || this.constructor.transactionBlockTimeout; + this.transactionConfirmationBlocks = this.options.transactionConfirmationBlocks || this.constructor.transactionConfirmationBlocks; + this.transactionPollingTimeout = this.options.transactionPollingTimeout || this.constructor.transactionPollingTimeout; + this.defaultChain = this.options.defaultChain || this.constructor.defaultChain; + this.defaultHardfork = this.options.defaultHardfork || this.constructor.defaultHardfork; + this.defaultCommon = this.options.defaultCommon || this.constructor.defaultCommon; + this.handleRevert = this.options.handleRevert || this.constructor.handleRevert; Object.defineProperty(this, 'defaultAccount', { get: function () { From d76c6ebf59f040769db6e1f528bf623525cb22bf Mon Sep 17 00:00:00 2001 From: nivida Date: Wed, 4 Dec 2019 10:15:09 +0100 Subject: [PATCH 17/22] 'eth_sendRawTransaction' does get ignored for the revert handling, inline comment updated, and passed parameters for _fireError in Method object updated --- packages/web3-core-method/src/index.js | 6 +++--- packages/web3-eth/src/index.js | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/web3-core-method/src/index.js b/packages/web3-core-method/src/index.js index e00d5d4e081..e6fb0dac665 100644 --- a/packages/web3-core-method/src/index.js +++ b/packages/web3-core-method/src/index.js @@ -413,7 +413,7 @@ Method.prototype._confirmTransaction = function (defer, result, payload) { try { var revertMessage = null; - if (method.handleRevert) { + if (method.handleRevert && method.call === 'eth_sendTransaction') { // Get revert reason string with eth_call revertMessage = await method.getRevertReason( payload.params[0], @@ -425,7 +425,7 @@ Method.prototype._confirmTransaction = function (defer, result, payload) { errors.RevertInstructionError(revertMessage.reason, revertMessage.signature), defer.eventEmitter, defer.reject, - payload.callback, + null, receipt ); } else { @@ -445,7 +445,7 @@ Method.prototype._confirmTransaction = function (defer, result, payload) { ); } } else { - // Throw OOG if status is true and provided gas and used gas are equal + // Throw OOG if status is not existing and provided gas and used gas are equal utils._fireError( new Error('Transaction ran out of gas. Please provide more gas:\n' + receiptJSON), defer.eventEmitter, diff --git a/packages/web3-eth/src/index.js b/packages/web3-eth/src/index.js index 8cdb5c1f564..ed0815982b0 100644 --- a/packages/web3-eth/src/index.js +++ b/packages/web3-eth/src/index.js @@ -442,8 +442,7 @@ var Eth = function Eth() { name: 'sendSignedTransaction', call: 'eth_sendRawTransaction', params: 1, - inputFormatter: [null], - abiCoder: abi + inputFormatter: [null] }), new Method({ name: 'signTransaction', From cce490abe3991be40e35a0e22ed41c757b91dc69 Mon Sep 17 00:00:00 2001 From: nivida Date: Wed, 4 Dec 2019 10:26:01 +0100 Subject: [PATCH 18/22] TransactionRevertInstructionError added and related types updated --- packages/web3-core-helpers/src/errors.js | 7 +++++++ packages/web3-core-helpers/types/index.d.ts | 6 ++++++ packages/web3-core-helpers/types/tests/errors-test.ts | 3 +++ packages/web3-core-method/src/index.js | 2 +- packages/web3-eth/types/index.d.ts | 6 +++--- packages/web3-eth/types/tests/eth.tests.ts | 4 ++-- 6 files changed, 22 insertions(+), 6 deletions(-) diff --git a/packages/web3-core-helpers/src/errors.js b/packages/web3-core-helpers/src/errors.js index 96b29b5966f..29d7f030bd8 100644 --- a/packages/web3-core-helpers/src/errors.js +++ b/packages/web3-core-helpers/src/errors.js @@ -49,6 +49,13 @@ module.exports = { error.reason = reason; error.signature = signature; + return error; + }, + TransactionRevertInstructionError: function(reason, signature, receipt) { + var error = new Error('Transaction has been reverted by the EVM:\n' + JSON.stringify(receipt, null, 2)); + error.reason = reason; + error.signature = signature; + return error; } }; diff --git a/packages/web3-core-helpers/types/index.d.ts b/packages/web3-core-helpers/types/index.d.ts index 43aeacd36cc..b3c11dec001 100644 --- a/packages/web3-core-helpers/types/index.d.ts +++ b/packages/web3-core-helpers/types/index.d.ts @@ -67,6 +67,7 @@ export class errors { static InvalidResponse(result: Error): Error; static ConnectionTimeout(ms: string): Error; static RevertInstructionError(reason: string, signature: string): RevertInstructionError + static TransactionRevertInstructionError(reason: string, signature: string, receipt: object): TransactionRevertInstructionError } export class WebsocketProviderBase { @@ -188,3 +189,8 @@ export interface RevertInstructionError extends Error { reason: string; signature: string; } + +export interface TransactionRevertInstructionError extends Error { + reason: string; + signature: string; +} diff --git a/packages/web3-core-helpers/types/tests/errors-test.ts b/packages/web3-core-helpers/types/tests/errors-test.ts index 87448197af5..8dc6725900a 100644 --- a/packages/web3-core-helpers/types/tests/errors-test.ts +++ b/packages/web3-core-helpers/types/tests/errors-test.ts @@ -39,3 +39,6 @@ errors.ConnectionTimeout('timeout'); // $ExpectType RevertInstructionError errors.RevertInstructionError('reason', 'signature'); + +// $ExpectType TransactionRevertInstructionError +errors.TransactionRevertInstructionError('reason', 'signature', {}); diff --git a/packages/web3-core-method/src/index.js b/packages/web3-core-method/src/index.js index e6fb0dac665..ac6561c77e8 100644 --- a/packages/web3-core-method/src/index.js +++ b/packages/web3-core-method/src/index.js @@ -422,7 +422,7 @@ Method.prototype._confirmTransaction = function (defer, result, payload) { if (revertMessage) { // Only throw a revert error if a revert reason is existing utils._fireError( - errors.RevertInstructionError(revertMessage.reason, revertMessage.signature), + errors.TransactionRevertInstructionError(revertMessage.reason, revertMessage.signature, receipt), defer.eventEmitter, defer.reject, null, diff --git a/packages/web3-eth/types/index.d.ts b/packages/web3-eth/types/index.d.ts index 05473ac009b..2fc9df1b2fb 100644 --- a/packages/web3-eth/types/index.d.ts +++ b/packages/web3-eth/types/index.d.ts @@ -38,7 +38,7 @@ import { LogsOptions, PastLogsOptions } from 'web3-core'; -import {RevertInstructionError} from 'web3-core-helpers'; +import {RevertInstructionError, TransactionRevertInstructionError} from 'web3-core-helpers'; import {Subscription} from 'web3-core-subscriptions'; import {AbiCoder} from 'web3-eth-abi'; import {Accounts} from 'web3-eth-accounts'; @@ -295,12 +295,12 @@ export class Eth { sendTransaction( transactionConfig: TransactionConfig, callback?: (error: Error, hash: string) => void - ): PromiEvent; + ): PromiEvent; sendSignedTransaction( signedTransactionData: string, callback?: (error: Error, hash: string) => void - ): PromiEvent; + ): PromiEvent; sign( dataToSign: string, diff --git a/packages/web3-eth/types/tests/eth.tests.ts b/packages/web3-eth/types/tests/eth.tests.ts index 3aca485b6b0..688d642c239 100644 --- a/packages/web3-eth/types/tests/eth.tests.ts +++ b/packages/web3-eth/types/tests/eth.tests.ts @@ -393,9 +393,9 @@ eth.sendTransaction( (error: Error, hash: string) => {} ); -// $ExpectType PromiEvent +// $ExpectType PromiEvent eth.sendSignedTransaction('0xf889808609184e72a0008227109'); -// $ExpectType PromiEvent +// $ExpectType PromiEvent eth.sendSignedTransaction( '0xf889808609184e72a0008227109', (error: Error, hash: string) => {} From eba77c2e2fd0e5c6fefcc6212d8d7f911cb6e39a Mon Sep 17 00:00:00 2001 From: nivida Date: Wed, 4 Dec 2019 11:20:17 +0100 Subject: [PATCH 19/22] logic checked if the transaction does get signed locally on call of 'eth.sendTransaction' and eth types tests updated --- packages/web3-eth/types/tests/eth.tests.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/web3-eth/types/tests/eth.tests.ts b/packages/web3-eth/types/tests/eth.tests.ts index 688d642c239..2f7bac7de4a 100644 --- a/packages/web3-eth/types/tests/eth.tests.ts +++ b/packages/web3-eth/types/tests/eth.tests.ts @@ -379,12 +379,12 @@ eth.getTransactionCount( const code = '603d80600c6000396000f3007c0'; -// $ExpectType PromiEvent +// $ExpectType PromiEvent eth.sendTransaction({ from: '0xde0B295669a9FD93d5F28D9Ec85E40f4cb697BAe', data: 'code' }); -// $ExpectType PromiEvent +// $ExpectType PromiEvent eth.sendTransaction( { from: '0xde0B295669a9FD93d5F28D9Ec85E40f4cb697BAe', From 0623f2cb15892c625e226b4fe12360809d60329a Mon Sep 17 00:00:00 2001 From: nivida Date: Wed, 4 Dec 2019 11:52:00 +0100 Subject: [PATCH 20/22] contract options handling improved, contract test extended, and related documentation updated --- docs/web3-eth-contract.rst | 7 ++ packages/web3-eth-contract/src/index.js | 86 +++++++++++++++++-- test/contract.js | 105 ++++++++++++++++++++++++ 3 files changed, 191 insertions(+), 7 deletions(-) diff --git a/docs/web3-eth-contract.rst b/docs/web3-eth-contract.rst index d1d7fb059ac..5856653c181 100644 --- a/docs/web3-eth-contract.rst +++ b/docs/web3-eth-contract.rst @@ -403,6 +403,13 @@ Properties - ``from`` - ``String``: The address transactions should be made from. - ``gasPrice`` - ``String``: The gas price in wei to use for transactions. - ``gas`` - ``Number``: The maximum gas provided for a transaction (gas limit). +- ``handleRevert`` - ``Boolean``: It will otherwise use the default value provided from the Eth module. See :ref:`handleRevert `. +- ``transactionBlockTimeout`` - ``Number``: It will otherwise use the default value provided from the Eth module. See :ref:`transactionBlockTimeout `. +- ``transactionConfirmationBlocks`` - ``Number``: It will otherwise use the default value provided from the Eth module. See :ref:`transactionConfirmationBlocks `. +- ``transactionPollingTimeout`` - ``Number``: It will otherwise use the default value provided from the Eth module. See :ref:`transactionPollingTimeout `. +- ``chain`` - ``Number``: It will otherwise use the default value provided from the Eth module. See :ref:`defaultChain `. +- ``hardfork`` - ``Number``: It will otherwise use the default value provided from the Eth module. See :ref:`defaultHardfork `. +- ``common`` - ``Number``: It will otherwise use the default value provided from the Eth module. See :ref:`defaultCommon `. ------- diff --git a/packages/web3-eth-contract/src/index.js b/packages/web3-eth-contract/src/index.js index 805cbf98363..711cf9317b8 100644 --- a/packages/web3-eth-contract/src/index.js +++ b/packages/web3-eth-contract/src/index.js @@ -180,14 +180,86 @@ var Contract = function Contract(jsonInterface, address, options) { // get default account from the Class var defaultAccount = this.constructor.defaultAccount; var defaultBlock = this.constructor.defaultBlock || 'latest'; - this.transactionBlockTimeout = this.options.transactionBlockTimeout || this.constructor.transactionBlockTimeout; - this.transactionConfirmationBlocks = this.options.transactionConfirmationBlocks || this.constructor.transactionConfirmationBlocks; - this.transactionPollingTimeout = this.options.transactionPollingTimeout || this.constructor.transactionPollingTimeout; - this.defaultChain = this.options.defaultChain || this.constructor.defaultChain; - this.defaultHardfork = this.options.defaultHardfork || this.constructor.defaultHardfork; - this.defaultCommon = this.options.defaultCommon || this.constructor.defaultCommon; - this.handleRevert = this.options.handleRevert || this.constructor.handleRevert; + Object.defineProperty(this, 'handleRevert', { + get: function () { + if (_this.options.handleRevert === false || _this.options.handleRevert === true) { + return _this.options.handleRevert; + } + + return this.constructor.handleRevert; + }, + set: function (val) { + _this.options.handleRevert = val; + }, + enumerable: true + }); + Object.defineProperty(this, 'defaultCommon', { + get: function () { + return _this.options.common || this.constructor.defaultCommon; + }, + set: function (val) { + _this.options.common = val; + }, + enumerable: true + }); + Object.defineProperty(this, 'defaultHardfork', { + get: function () { + return _this.options.hardfork || this.constructor.defaultHardfork; + }, + set: function (val) { + _this.options.hardfork = val; + }, + enumerable: true + }); + Object.defineProperty(this, 'defaultChain', { + get: function () { + return _this.options.chain || this.constructor.defaultChain; + }, + set: function (val) { + _this.options.chain = val; + }, + enumerable: true + }); + Object.defineProperty(this, 'transactionPollingTimeout', { + get: function () { + if (_this.options.transactionPollingTimeout === 0) { + return _this.options.transactionPollingTimeout; + } + + return _this.options.transactionPollingTimeout || this.constructor.transactionPollingTimeout; + }, + set: function (val) { + _this.options.transactionPollingTimeout = val; + }, + enumerable: true + }); + Object.defineProperty(this, 'transactionConfirmationBlocks', { + get: function () { + if (_this.options.transactionConfirmationBlocks === 0) { + return _this.options.transactionConfirmationBlocks; + } + + return _this.options.transactionConfirmationBlocks || this.constructor.transactionConfirmationBlocks; + }, + set: function (val) { + _this.options.transactionConfirmationBlocks = val; + }, + enumerable: true + }); + Object.defineProperty(this, 'transactionBlockTimeout', { + get: function () { + if (_this.options.transactionBlockTimeout === 0) { + return _this.options.transactionBlockTimeout; + } + + return _this.options.transactionBlockTimeout || this.constructor.transactionBlockTimeout; + }, + set: function (val) { + _this.options.transactionBlockTimeout = val; + }, + enumerable: true + }); Object.defineProperty(this, 'defaultAccount', { get: function () { return defaultAccount; diff --git a/test/contract.js b/test/contract.js index 18d0a9c6c77..b727ee937b8 100644 --- a/test/contract.js +++ b/test/contract.js @@ -283,6 +283,111 @@ var runTests = function(contractFactory) { assert.throws(test); }); + it('should define the handleRevert object property if passed over the options', function() { + var provider = new FakeIpcProvider(); + var contract = contractFactory(abi, address, {handleRevert: true}, provider); + + assert.equal(contract.handleRevert, true); + assert.equal(contract.options.handleRevert, true); + }); + it('should update the handleRevert property in the options object', function() { + var provider = new FakeIpcProvider(); + var contract = contractFactory(abi, address, {handleRevert: false}, provider); + + contract.handleRevert = true; + + assert.equal(contract.options.handleRevert, true); + }); + it('should define the defaultCommon object property if passed over the options', function() { + var provider = new FakeIpcProvider(); + var contract = contractFactory(abi, address, {common: true}, provider); + + assert.equal(contract.defaultCommon, true); + assert.equal(contract.options.common, true); + }); + it('should update the defaultCommon property in the options object', function() { + var provider = new FakeIpcProvider(); + var contract = contractFactory(abi, address, {common: false}, provider); + + contract.defaultCommon = true; + + assert.equal(contract.options.common, true); + }); + it('should define the defaultHardfork object property if passed over the options', function() { + var provider = new FakeIpcProvider(); + var contract = contractFactory(abi, address, {hardfork: 'istanbul'}, provider); + + assert.equal(contract.defaultHardfork, 'istanbul'); + assert.equal(contract.options.hardfork, 'istanbul'); + }); + it('should update the defaultHardfork property in the options object', function() { + var provider = new FakeIpcProvider(); + var contract = contractFactory(abi, address, {hardfork: false}, provider); + + contract.defaultHardfork = true; + + assert.equal(contract.options.hardfork, true); + }); + it('should define the defaultChain object property if passed over the options', function() { + var provider = new FakeIpcProvider(); + var contract = contractFactory(abi, address, {chain: 'mainnet'}, provider); + + assert.equal(contract.defaultChain, 'mainnet'); + assert.equal(contract.options.chain, 'mainnet'); + }); + it('should update the defaultChain property in the options object', function() { + var provider = new FakeIpcProvider(); + var contract = contractFactory(abi, address, {chain: false}, provider); + + contract.defaultChain = true; + + assert.equal(contract.options.chain, true); + }); + it('should define the transactionPollingTimeout object property if passed over the options', function() { + var provider = new FakeIpcProvider(); + var contract = contractFactory(abi, address, {transactionPollingTimeout: 0}, provider); + + assert.equal(contract.transactionPollingTimeout, 0); + assert.equal(contract.options.transactionPollingTimeout, 0); + }); + it('should update the transactionPollingTimeout property in the options object', function() { + var provider = new FakeIpcProvider(); + var contract = contractFactory(abi, address, {transactionPollingTimeout: 1}, provider); + + contract.transactionPollingTimeout = 0; + + assert.equal(contract.options.transactionPollingTimeout, 0); + }); + it('should define the transactionConfirmationBlocks object property if passed over the options', function() { + var provider = new FakeIpcProvider(); + var contract = contractFactory(abi, address, {transactionConfirmationBlocks: 0}, provider); + + assert.equal(contract.transactionConfirmationBlocks, 0); + assert.equal(contract.options.transactionConfirmationBlocks, 0); + }); + it('should update the transactionConfirmationBlocks property in the options object', function() { + var provider = new FakeIpcProvider(); + var contract = contractFactory(abi, address, {transactionConfirmationBlocks: 1}, provider); + + contract.transactionConfirmationBlocks = 0; + + assert.equal(contract.options.transactionConfirmationBlocks, 0); + }); + it('should define the transactionBlockTimeout object property if passed over the options', function() { + var provider = new FakeIpcProvider(); + var contract = contractFactory(abi, address, {transactionBlockTimeout: 0}, provider); + + assert.equal(contract.transactionBlockTimeout, 0); + assert.equal(contract.options.transactionBlockTimeout, 0); + }); + it('should update the transactionBlockTimeout property in the options object', function() { + var provider = new FakeIpcProvider(); + var contract = contractFactory(abi, address, {transactionBlockTimeout: 1}, provider); + + contract.transactionBlockTimeout = 0; + + assert.equal(contract.options.transactionBlockTimeout, 0); + }); it('.clone() should properly clone the contract instance', function () { var provider = new FakeIpcProvider(); From 7fb7b0c6a07d97fb55f6d46ce0bede52aeeef971 Mon Sep 17 00:00:00 2001 From: nivida Date: Wed, 4 Dec 2019 11:58:51 +0100 Subject: [PATCH 21/22] web3-eth.rst updated --- docs/web3-eth.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/web3-eth.rst b/docs/web3-eth.rst index 4974bbc307d..06968f8027f 100644 --- a/docs/web3-eth.rst +++ b/docs/web3-eth.rst @@ -415,7 +415,6 @@ The ``handleRevert`` options property does default to ``false`` and will return - :ref:`web3.eth.call() ` - :ref:`web3.eth.sendTransaction() ` -- :ref:`web3.eth.sendSignedTransaction() ` - :ref:`contract.methods.myMethod(...).send(...) ` - :ref:`contract.methods.myMethod(...).call(...) ` From b0b7f3ae2e0bfb4f821109117ac6a46bbaf33e12 Mon Sep 17 00:00:00 2001 From: nivida Date: Fri, 6 Dec 2019 10:33:09 +0100 Subject: [PATCH 22/22] hint about the 'reason' and 'signature' property of the returnd error object added to the documentation --- docs/web3-eth-contract.rst | 2 ++ docs/web3-eth.rst | 1 + 2 files changed, 3 insertions(+) diff --git a/docs/web3-eth-contract.rst b/docs/web3-eth-contract.rst index 5856653c181..f50c7aa2e67 100644 --- a/docs/web3-eth-contract.rst +++ b/docs/web3-eth-contract.rst @@ -374,6 +374,8 @@ handleRevert The ``handleRevert`` options property does default to ``false`` and will return the revert reason string if enabled on :ref:`send ` or :ref:`call ` of a contract method. +.. note:: The revert reason string and the signature does exist as property on the returned error. + ------- Returns ------- diff --git a/docs/web3-eth.rst b/docs/web3-eth.rst index 06968f8027f..391f042172a 100644 --- a/docs/web3-eth.rst +++ b/docs/web3-eth.rst @@ -418,6 +418,7 @@ The ``handleRevert`` options property does default to ``false`` and will return - :ref:`contract.methods.myMethod(...).send(...) ` - :ref:`contract.methods.myMethod(...).call(...) ` +.. note:: The revert reason string and the signature does exist as property on the returned error. ------- Returns