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

[R4R] Change gas & related fields to unsigned integer type #2839

Merged
merged 11 commits into from
Nov 19, 2018

Conversation

alexanderbez
Copy link
Contributor

@alexanderbez alexanderbez commented Nov 15, 2018

Update gas consumption and fields to utilize uint64 over int64. I believe the corresponding changes should also be made in TM for abci.ResponseDeliverTx, abci.ResponseCheckTx, and BlockSize.MaxGas so we don't have to cast.

closes: #2815


  • Targeted PR against correct branch (see CONTRIBUTING.md)

  • Linked to github-issue with discussion and accepted design OR link to spec that describes this work.

  • Wrote tests

  • Updated relevant documentation (docs/)

  • Added entries in PENDING.md with issue #

  • rereviewed Files changed in the github PR explorer


For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@codecov
Copy link

codecov bot commented Nov 15, 2018

Codecov Report

Merging #2839 into develop will decrease coverage by 0.17%.
The diff coverage is 80%.

@@             Coverage Diff             @@
##           develop    #2839      +/-   ##
===========================================
- Coverage       57%   56.83%   -0.18%     
===========================================
  Files          131      131              
  Lines         8581     8575       -6     
===========================================
- Hits          4892     4874      -18     
- Misses        3359     3370      +11     
- Partials       330      331       +1

Copy link
Contributor

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

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

Consume gas should check for overflow

@alexanderbez
Copy link
Contributor Author

Yes @ValarDragon -- the PR is not complete yet.

@alexanderbez alexanderbez changed the title [WIP] Change gas & related fields to unsigned integer type [R4R] Change gas & related fields to unsigned integer type Nov 16, 2018
@alexanderbez
Copy link
Contributor Author

Failing due to a regression in #2781.

Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

Tested ACK

x/auth/ante.go Show resolved Hide resolved
@jackzampolin jackzampolin dismissed ValarDragon’s stale review November 19, 2018 17:13

This has been addressed by subsequent commits.

@jackzampolin jackzampolin merged commit 6e813ab into develop Nov 19, 2018
@jackzampolin jackzampolin deleted the bez/2815-gas-uint branch November 19, 2018 17:13
chillyvee pushed a commit to chillyvee/cosmos-sdk that referenced this pull request Mar 1, 2024
* Updates for Gaia v14 (aka v14.1.0)

* Some minor updates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change Gas to Uint
5 participants