Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

fix: raise error when get invalid consensus params #1790

Closed
wants to merge 6 commits into from

Conversation

mmsqe
Copy link
Contributor

@mmsqe mmsqe commented Jun 15, 2023

Closes: #XXX

Description


For contributor use:

  • 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.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@mmsqe mmsqe marked this pull request as ready for review June 15, 2023 03:40
@mmsqe mmsqe requested a review from a team as a code owner June 15, 2023 03:40
@mmsqe mmsqe requested review from Vvaradinov and MalteHerrmann and removed request for a team June 15, 2023 03:40
@codecov
Copy link

codecov bot commented Jun 15, 2023

Codecov Report

Merging #1790 (da74c62) into main (10d9579) will not change coverage.
The diff coverage is 100.00%.

❗ Current head da74c62 differs from pull request most recent head 444b8e8. Consider uploading reports for the commit 444b8e8 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1790   +/-   ##
=======================================
  Coverage   68.89%   68.89%           
=======================================
  Files         117      117           
  Lines       10681    10681           
=======================================
  Hits         7359     7359           
  Misses       2900     2900           
  Partials      422      422           
Impacted Files Coverage Δ
x/feemarket/keeper/eip1559.go 90.16% <100.00%> (ø)

mmsqe added a commit to crypto-org-chain/ethermint that referenced this pull request Jun 15, 2023
* add nil check for consParams.Block

* add change log

* skip invalid consensus params

* panic on invalid consensus params
if consParams != nil && consParams.Block.MaxGas > -1 {
gasLimit = big.NewInt(consParams.Block.MaxGas)
if consParams == nil || consParams.Block == nil || consParams.Block.MaxGas <= -1 {
panic(fmt.Sprintf("get invalid consensus params: %s", consParams))

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
@mmsqe mmsqe changed the title fix: add nil check for consParams.Block fix: raise error when get invalid consensus params Jun 15, 2023
mmsqe added a commit to mmsqe/ethermint that referenced this pull request Jun 22, 2023
@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days-before-close if no further activity occurs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant