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

fix: remove max gas validation #895

Merged
merged 3 commits into from
Feb 13, 2023
Merged

fix: remove max gas validation #895

merged 3 commits into from
Feb 13, 2023

Conversation

0Tech
Copy link
Collaborator

@0Tech 0Tech commented Feb 13, 2023

Description

closes: #861

Checklist:

  • I followed the contributing guidelines and code of conduct.
  • I have added a relevant changelog to CHANGELOG.md
  • I have added tests to cover my changes.
  • I have updated the documentation accordingly.
  • I have updated API documentation client/docs/swagger-ui/swagger.yaml

@0Tech 0Tech self-assigned this Feb 13, 2023
@codecov
Copy link

codecov bot commented Feb 13, 2023

Codecov Report

Merging #895 (0c0f315) into main (5a8d776) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 0c0f315 differs from pull request most recent head 10752b7. Consider uploading reports for the commit 10752b7 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #895      +/-   ##
==========================================
- Coverage   62.31%   62.31%   -0.01%     
==========================================
  Files         653      653              
  Lines       79497    79480      -17     
==========================================
- Hits        49537    49525      -12     
+ Misses      27280    27276       -4     
+ Partials     2680     2679       -1     
Impacted Files Coverage Δ
x/auth/ante/setup.go 83.78% <ø> (+2.65%) ⬆️
baseapp/abci.go 58.05% <100.00%> (-0.07%) ⬇️
crypto/keys/internal/ecdsa/privkey.go 83.01% <0.00%> (+1.88%) ⬆️

@0Tech 0Tech marked this pull request as ready for review February 13, 2023 02:28
@zemyblue zemyblue added this to the v0.47.0-alpha1 milestone Feb 13, 2023
jaeseung-bae
jaeseung-bae previously approved these changes Feb 13, 2023
@0Tech 0Tech requested a review from jaeseung-bae February 13, 2023 04:57
Copy link
Member

@zemyblue zemyblue left a comment

Choose a reason for hiding this comment

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

Does this PR roll back #673 ?

@0Tech
Copy link
Collaborator Author

0Tech commented Feb 13, 2023

Does this PR roll back #673 ?

Yes, but partially. #673 does set consensus params to the deliver state. The upstream also sets these. So the relevant logic and tests would remain.

@0Tech 0Tech requested a review from zemyblue February 13, 2023 09:01
@0Tech 0Tech merged commit 302cb11 into Finschia:main Feb 13, 2023
@0Tech 0Tech deleted the max_gas branch February 13, 2023 09:05
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.

Remove max_gas verification from lbm-sdk
3 participants