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

Rationale Behind Contract ABI Supporting GAS #3383

Closed
wants to merge 1 commit into from

Conversation

maektwain
Copy link

Supports cases like

 {
    name: 'setup',
    outputs: [],
    inputs: [{ type: 'address', name: 'token_addr' }],
    constant: false,
    payable: false,
    type: 'function',
    gas: 175875
}

Description

Please include a summary of the change and be sure you follow the contributions rules we do provide here

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • [Y ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • [Y ] I have selected the correct base branch.
  • [Y ] I have performed a self-review of my own code.
  • [ N] I have commented my code, particularly in hard-to-understand areas.
  • [ N] I have made corresponding changes to the documentation.
  • [ Y] My changes generate no new warnings.
  • Any dependent changes have been merged and published in downstream modules.
  • [ N] I ran npm run dtslint with success and extended the tests and types if necessary.
  • [ N] I ran npm run test:unit with success.
  • [ N] I have executed npm run test:cov and my test cases do cover all lines and branches of the added code.
  • 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.

Supports cases like 
```
 {
    name: 'setup',
    outputs: [],
    inputs: [{ type: 'address', name: 'token_addr' }],
    constant: false,
    payable: false,
    type: 'function',
    gas: 175875
```
@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.914% when pulling 379e5d9 on maektwain:patch-1 into 2ce573b on ethereum:2.x.

@cgewecke
Copy link
Collaborator

@maektwain Out of curiosity, which compiler/tool generates this gas property in the ABI?

@alcuadrado
Copy link

I believe vyper does, as it can always compute a bound of the gas for each function.

@cgewecke
Copy link
Collaborator

@alcuadrado Thanks! Confirmed at vyper 306.

@maektwain You've pointed this PR at Web3's 2.x branch which is not on a regular publishing schedule at the moment. Happy to approve this but it might be a while before it gets to npm.

Do you have any interest in retargeting it at 1.x?

@maektwain
Copy link
Author

@cgewecke Sure, I would like to look into it once more.

@nivida nivida added 2.x 2.0 related issues Review Needed Maintainer(s) need to review Types Incorrect or missing types labels Feb 28, 2020
maektwain pushed a commit to maektwain/web3.js that referenced this pull request Mar 30, 2020
As suggested by the web3#3383
@maektwain maektwain mentioned this pull request Mar 30, 2020
15 tasks
@github-actions
Copy link

This PR has been automatically marked as stale beacause it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.

@github-actions github-actions bot added the Stale Has not received enough activity label Jul 27, 2020
@github-actions github-actions bot closed this Aug 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.x 2.0 related issues Review Needed Maintainer(s) need to review Stale Has not received enough activity Types Incorrect or missing types
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants