Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefer receipt status to code availability on contract deployment #3298

Merged
merged 4 commits into from
Jan 9, 2020

Conversation

cgewecke
Copy link
Collaborator

@cgewecke cgewecke commented Jan 7, 2020

Description

Addresses problem described in truffle 2257 where the availability of the contract code in the client DB lags the delivery of the tx receipt on deployment. Web3 tries to fetch the code immediately, can't find anything and errors with

The contract code couldn't be stored, please check your gas limit.

At the moment people frequently see successful contract deployments reported as failures
Commit 17f72d7 adds additional tests cases which hit this error, to establish a behavioral baseline for the logic change.

  • OOG (where tx runs out of gas while the evm processes the code)
  • Deploying a zero length bytecode on ganache. (Ganache allows this, and can happen by accident when someone tries to deploy a Solidity 'interface' contract)

Commit aec5c08 adds logic to allow any deployment which had a 'real' bytecode and a true receipt status to be treated as successful.

The code length check is kept as an alternative for pre-byzantium chains and to preserve current behavior when deploying zero length bytecodes to ganache (e.g should error).

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have selected the correct base branch.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • Any dependent changes have been merged and published in downstream modules.
  • I ran npm run dtslint with success and extended the tests and types if necessary.
  • I ran npm run test:unit with success and extended the tests if necessary.
  • I ran npm run build-all and tested the resulting file/'s from dist folder in a browser.
  • I have updated the CHANGELOG.md file in the root folder.
  • I have tested my code on the live network.

@cgewecke cgewecke changed the title Stop running getCode for successful contract deployments Prefer receipt status to code availability on contract deployment Jan 7, 2020
@coveralls
Copy link

coveralls commented Jan 7, 2020

Coverage Status

Coverage increased (+0.02%) to 84.821% when pulling 509d73a on fix/get-code-failure into b89a4da on 1.x.

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";
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cgewecke cgewecke marked this pull request as ready for review January 7, 2020 19:02
@cgewecke cgewecke added 1.x 1.0 related issues Bug Addressing a bug Review Needed Maintainer(s) need to review labels Jan 7, 2020
@cgewecke cgewecke requested a review from nivida January 7, 2020 19:02
Copy link
Contributor

@nivida nivida left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for taking care of web3 related issues from Truffle 💪

Could you extend the CHANGELOG with this applied fix (you can reference this PR or this issue #1477)?
Edit: Updated the CHANGELOG quickly for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.0 related issues Bug Addressing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants