Skip to content

Commit

Permalink
Merge pull request #3298 from ethereum/fix/get-code-failure
Browse files Browse the repository at this point in the history 
Prefer receipt status to code availability on contract deployment
  • Loading branch information
@nivida
nivida authored Jan 9, 2020
2 parents b89a4da + 509d73a commit b0295ea
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 39 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,4 @@ Released with 1.0.0-beta.37 code base.
- ``clearSubscriptions`` does no longer throw an error if no running subscriptions do exist (#3246)
- callback type definition for ``Accounts.signTransaction`` fixed (#3280)
- fix: export bloom functions on the index.js
- Prefer receipt status to code availability on contract deployment (#3298)
68 changes: 38 additions & 30 deletions packages/web3-core-method/src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ Method.prototype._confirmTransaction = function (defer, result, payload) {
isContractDeployment = _.isObject(payload.params[0]) &&
payload.params[0].data &&
payload.params[0].from &&
!payload.params[0].to;
!payload.params[0].to,
hasBytecode = isContractDeployment && payload.params[0].data.length > 2;

// add custom send Methods
var _ethereumCalls = [
Expand Down Expand Up @@ -330,7 +331,7 @@ Method.prototype._confirmTransaction = function (defer, result, payload) {
return receipt;
})
// CHECK for CONTRACT DEPLOYMENT
.then(function (receipt) {
.then(async function (receipt) {

if (isContractDeployment && !promiseResolved) {

Expand All @@ -351,43 +352,50 @@ Method.prototype._confirmTransaction = function (defer, result, payload) {
return;
}

_ethereumCall.getCode(receipt.contractAddress, function (e, code) {

if (!code) {
return;
}

var code;
try {
code = await _ethereumCall.getCode(receipt.contractAddress);
} catch(err){
// ignore;
}

if (code.length > 2) {
defer.eventEmitter.emit('receipt', receipt);
if (!code) {
return;
}

// if contract, return instance instead of receipt
if (method.extraFormatters && method.extraFormatters.contractDeployFormatter) {
defer.resolve(method.extraFormatters.contractDeployFormatter(receipt));
} else {
defer.resolve(receipt);
}
// If deployment is status.true and there was a real
// bytecode string, assume it was successful.
var deploymentSuccess = receipt.status === true && hasBytecode;

// need to remove listeners, as they aren't removed automatically when succesfull
if (canUnsubscribe) {
defer.eventEmitter.removeAllListeners();
}
if (deploymentSuccess || code.length > 2) {
defer.eventEmitter.emit('receipt', receipt);

// if contract, return instance instead of receipt
if (method.extraFormatters && method.extraFormatters.contractDeployFormatter) {
defer.resolve(method.extraFormatters.contractDeployFormatter(receipt));
} else {
utils._fireError(
errors.ContractCodeNotStoredError(receipt),
defer.eventEmitter,
defer.reject,
null,
receipt
);
defer.resolve(receipt);
}

// need to remove listeners, as they aren't removed automatically when succesfull
if (canUnsubscribe) {
sub.unsubscribe();
defer.eventEmitter.removeAllListeners();
}
promiseResolved = true;
});

} else {
utils._fireError(
errors.ContractCodeNotStoredError(receipt),
defer.eventEmitter,
defer.reject,
null,
receipt
);
}

if (canUnsubscribe) {
sub.unsubscribe();
}
promiseResolved = true;
}

return receipt;
Expand Down
71 changes: 62 additions & 9 deletions test/e2e.contract.deploy.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ describe('contract.deploy [ @E2E ]', function() {
var accounts;
var basic;
var reverts;
var noBytecode;
var options;

// Error message variants
var ganacheRevert = "revert";
var gethRevert = "code couldn't be stored";
var revertMessage = "revert";
var couldNotBeStoredMessage = "code couldn't be stored";
var creationWithoutDataMessage = "contract creation without any data provided";

var basicOptions = {
data: Basic.bytecode,
Expand All @@ -27,13 +29,20 @@ describe('contract.deploy [ @E2E ]', function() {
gas: 4000000
}

var noBytecodeOptions = {
data: '0x',
gasPrice: '1',
gas: 4000000
}

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);
reverts = new web3.eth.Contract(Reverts.abi, revertsOptions);
noBytecode = new web3.eth.Contract(Basic.abi, noBytecodeOptions);
})

it('returns an instance', async function(){
Expand All @@ -44,7 +53,9 @@ describe('contract.deploy [ @E2E ]', function() {
assert(web3.utils.isAddress(instance.options.address));
});

it('errors on OOG', async function(){
// Clients reject this kind of OOG is early because
// the gas is obviously way too low.
it('errors on "intrinic gas too low" OOG', async function(){
try {
await basic
.deploy()
Expand All @@ -53,9 +64,49 @@ describe('contract.deploy [ @E2E ]', function() {
assert.fail();
} catch(err){
assert(err.message.includes('gas'))
assert(err.receipt === undefined);
}
});

// Clients reject this kind of OOG when the EVM runs out of gas
// while running the code. A contractAddress is set on the
// receipt, but the status will be false.
it('errors on OOG reached while running EVM', async function(){
const estimate = await basic
.deploy()
.estimateGas()

const gas = estimate - 1000;

try {
await basic
.deploy()
.send({from: accounts[0], gas: gas});

assert.fail();
} catch(err){
assert(err.message.includes(couldNotBeStoredMessage));
assert(err.receipt.status === false);
}
});

// Geth immediately rejects a zero length bytecode without mining,
// Ganache accepts and mines it, returning a receipt with status === true
it('errors deploying a zero length bytecode', async function(){
try {
await noBytecode
.deploy()
.send({from: accounts[0]});

assert.fail();
} catch(err){
assert(
err.message.includes(creationWithoutDataMessage) ||
err.message.includes(couldNotBeStoredMessage)
);
}
})

it('errors on revert', async function(){
try {
await reverts
Expand All @@ -65,9 +116,11 @@ describe('contract.deploy [ @E2E ]', function() {
assert.fail();
} catch(err){
assert(
err.message.includes(gethRevert) ||
err.message.includes(ganacheRevert)
err.message.includes(couldNotBeStoredMessage) ||
err.message.includes(revertMessage)
);

assert(err.receipt.status === false);
}
});
});
Expand Down Expand Up @@ -115,8 +168,8 @@ describe('contract.deploy [ @E2E ]', function() {
assert.fail();
} catch(err){
assert(
err.message.includes(gethRevert) ||
err.message.includes(ganacheRevert)
err.message.includes(couldNotBeStoredMessage) ||
err.message.includes(revertMessage)
);
}
});
Expand Down Expand Up @@ -177,8 +230,8 @@ describe('contract.deploy [ @E2E ]', function() {
.send({from: accounts[0]})
.on('error', err => {
assert(
err.message.includes(gethRevert) ||
err.message.includes(ganacheRevert)
err.message.includes(couldNotBeStoredMessage) ||
err.message.includes(revertMessage)
);
done();
})
Expand Down

0 comments on commit b0295ea

Please sign in to comment.