-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New Migrations #1028
New Migrations #1028
Changes from 64 commits
fdff074
61af79f
f1f96bb
10242e4
3e62aac
d6cab54
9c71d07
9b3d10b
995f540
f53e932
2d15b92
e16cd4b
8348d62
c32b116
a409d36
deaebcc
40dbbd3
fd99765
6125d4a
1513236
63ae9eb
ca16e6f
de626c8
76ca3ae
baad4fa
e6e0ea4
ee056cc
fe5bb71
d63edcb
d05d236
e61eb36
0363f91
42ab1bb
48f7389
742652b
1a7caea
b2da71b
c73f035
e7a5ba8
332f2da
b0d0cce
22053ec
0c4f8cd
4b422a9
dc699c9
537bee4
b208571
314eea7
79e9d54
44370b5
2f70102
4f69fd2
9fd7389
fba3660
fa9c802
db2ca10
9c98ee8
663c96c
f325fd2
b8a5f17
73f0cfa
2af8387
54fd631
d2c03e0
509a543
d0ce12e
0e1f8dd
5d39172
d9ea3f3
c754c89
e4d3da9
05719d4
9218386
fb288f0
393f992
59067fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,12 +5,13 @@ var StatusError = require("./statuserror"); | |
var handlers = require("./handlers"); | ||
var override = require("./override"); | ||
|
||
var util = require('util'); | ||
var execute = { | ||
|
||
// ----------------------------------- Helpers -------------------------------------------------- | ||
|
||
/** | ||
* Retrieves gas estimate multiplied by the set gas multiplier for a `sendTransaction` call. | ||
* We're using low level rpc calls here | ||
* @param {Object} params `sendTransaction` parameters | ||
* @param {Number} blockLimit most recent network block.blockLimit | ||
* @return {Number} gas estimate | ||
|
@@ -20,23 +21,44 @@ var execute = { | |
var web3 = this.web3; | ||
|
||
return new Promise(function(accept, reject){ | ||
// Always prefer specified gas - this includes gas set by class_defaults | ||
if (params.gas) return accept(params.gas); | ||
if (!constructor.autoGas) return accept(); | ||
|
||
web3.eth | ||
.estimateGas(params) | ||
.then(gas => { | ||
var bestEstimate = Math.floor(constructor.gasMultiplier * gas); | ||
|
||
// Don't go over blockLimit | ||
(bestEstimate >= blockLimit) | ||
? accept(blockLimit - 1) | ||
: accept(bestEstimate); | ||
|
||
// We need to let txs that revert through. | ||
// Often that's exactly what you are testing. | ||
}).catch(err => accept()); | ||
let reason; | ||
|
||
const packet = { | ||
jsonrpc: "2.0", | ||
method: "eth_call", | ||
params: [params], | ||
id: new Date().getTime(), | ||
} | ||
|
||
// This rpc call extracts the reason string | ||
web3.currentProvider.send(packet, (err, response) => { | ||
if (response && (response.error || response.result)) { | ||
reason = execute.extractReason(response, web3) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah so There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
||
web3 | ||
.eth | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I kind of prefer the way you had it with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
.estimateGas(params) | ||
.then(gas => { | ||
// Always prefer specified gas - this includes gas set by class_defaults | ||
if (params.gas) return accept({gas: params.gas, error: null}); | ||
if (!constructor.autoGas) return accept({gas: null, error: null}); | ||
|
||
var bestEstimate = Math.floor(constructor.gasMultiplier * gas); | ||
|
||
// Don't go over blockLimit | ||
(bestEstimate >= blockLimit) | ||
? accept({gas: blockLimit - 1, error: null}) | ||
: accept({gas: bestEstimate, error: null}); | ||
}) | ||
.catch(err => { | ||
err.reason = reason; | ||
|
||
(params.gas) | ||
? accept({gas: params.gas, error: err}) | ||
: accept({gas: null, error: err}); | ||
}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is all really pretty |
||
}) | ||
}) | ||
}, | ||
|
||
|
@@ -65,6 +87,29 @@ var execute = { | |
return utils.is_object(arg) && !utils.is_big_number(arg); | ||
}, | ||
|
||
/** | ||
* Processes .call/.estimateGas errors and extracts a reason string if | ||
* | ||
* @param {[type]} err [description] | ||
* @return {[type]} [description] | ||
*/ | ||
extractReason(res, web3){ | ||
const isObject = res && typeof res === 'object' && res.error && res.error.data; | ||
const isString = res && typeof res === 'object' && typeof res.result === 'string'; | ||
|
||
if (isObject) { | ||
const data = res.error.data; | ||
const hash = Object.keys(data)[0]; | ||
|
||
if (data[hash].return && data[hash].return.includes('0x08c379a0')){ | ||
return web3.eth.abi.decodeParameter('string', data[hash].return.slice(10)) | ||
} | ||
|
||
} else if (isString && res.result.includes('0x08c379a0')){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's that value? I think let's make this a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mmm. yes There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess derived from this axic comment in the EIP ? 🙂
|
||
return web3.eth.abi.decodeParameter('string', res.result.slice(10)) | ||
} | ||
}, | ||
|
||
/** | ||
* Parses function arguments to discover if the terminal argument specifies the `defaultBlock` | ||
* to execute a call at. | ||
|
@@ -131,6 +176,7 @@ var execute = { | |
var args = Array.prototype.slice.call(arguments); | ||
var params = utils.getTxParams.call(constructor, args); | ||
var promiEvent = new Web3PromiEvent(); | ||
var reason; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't appear to be used |
||
|
||
var context = { | ||
contract: constructor, // Can't name this field `constructor` or `_constructor` | ||
|
@@ -145,10 +191,21 @@ var execute = { | |
execute | ||
.getGasEstimate | ||
.call(constructor, params, network.blockLimit) | ||
.then(gas => { | ||
params.gas = gas | ||
.then(result => { | ||
(result.error) | ||
? context.reason = result.error.reason | ||
: context.reason = null; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this getting attached at the receipt handler? Check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes it is - we also have tests for reason string handling across a matrix of the following:
|
||
params.gas = result.gas || undefined; | ||
deferred = web3.eth.sendTransaction(params); | ||
deferred.catch(override.start.bind(constructor, context)); | ||
|
||
// vmErrorsOnResponse path. Client emulator will | ||
// reject via the receipt handler | ||
deferred.catch(err => { | ||
err.reason = (result.error) ? result.error.reason : null; | ||
override.start.call(constructor, context, err) | ||
}); | ||
|
||
handlers.setup(deferred, context); | ||
}) | ||
.catch(promiEvent.reject) | ||
|
@@ -173,6 +230,7 @@ var execute = { | |
var web3 = constructor.web3; | ||
var params = utils.getTxParams.call(constructor, args); | ||
var deferred; | ||
var reason; | ||
|
||
var options = { | ||
data: constructor.binary, | ||
|
@@ -185,14 +243,17 @@ var execute = { | |
execute | ||
.getGasEstimate | ||
.call(constructor, params, blockLimit) | ||
.then(gas => { | ||
params.gas = gas; | ||
.then(result => { | ||
if (result.error) reason = result.error.reason; | ||
|
||
params.gas = result.gas || undefined; | ||
deferred = web3.eth.sendTransaction(params); | ||
handlers.setup(deferred, context); | ||
|
||
deferred.then(receipt => { | ||
if (parseInt(receipt.status) == 0){ | ||
var error = new StatusError(params, context.transactionHash, receipt); | ||
error.reason = reason; | ||
return context.promiEvent.reject(error) | ||
} | ||
|
||
|
@@ -201,9 +262,12 @@ var execute = { | |
|
||
context.promiEvent.resolve(new constructor(web3Instance)); | ||
|
||
// Manage web3's 50 blocks' timeout error. | ||
// Web3's own subscriptions go dead here. | ||
}).catch(override.start.bind(constructor, context)) | ||
// Manage web3's 50 blocks' timeout error. Web3's own subscriptions go dead here. | ||
// Also propagate any reason strings captured during estimate gas. | ||
}).catch(err => { | ||
err.reason = reason; | ||
override.start.call(constructor, context, err) | ||
}) | ||
}).catch(context.promiEvent.reject); | ||
}, | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,6 +104,7 @@ var handlers = { | |
// .method(): resolve/reject receipt in handler | ||
if (parseInt(receipt.status) == 0 && !context.onlyEmitReceipt){ | ||
var error = new StatusError(context.params, receipt.transactionHash, receipt); | ||
error.reason = context.reason; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a little bit worried about attaching Pardon me if I haven't seen it yet, but it might be worth making a special note somewhere in a comment, so we can encourage future work to be mindful of pre- vs. post-reason-attach errors. I know you've written a good matrix of tests around this, so it's probably safe enough, esp. since it's not likely to go without There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've moved this into the StatusError file where it (possibly) looks less arbitrary. Truffle-contract has actually attached quite a bit of stuff to the error traditionally - like the txHash for example (which we continue to do for backwards compatibility). |
||
return context.promiEvent.reject(error) | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?