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

feat: allow for the default consensus params to be set by the application #317

Merged
merged 3 commits into from
May 12, 2023

Conversation

evan-forbes
Copy link
Member

Description

This PR adds the consensus params to the server context, so that the init cli command can add it to the genesis document. This allows for the consensus parameters to be set by the application.

@evan-forbes evan-forbes added this to the Mainnet milestone May 11, 2023
@evan-forbes evan-forbes self-assigned this May 11, 2023
@evan-forbes evan-forbes marked this pull request as ready for review May 11, 2023 20:10
@evan-forbes evan-forbes requested a review from liamsi as a code owner May 11, 2023 20:10
Copy link
Member

@rach-id rach-id left a comment

Choose a reason for hiding this comment

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

LGTM, would defer to other reviewers for final approval

x/genutil/client/cli/init_test.go Outdated Show resolved Hide resolved
Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>
@codecov-commenter
Copy link

Codecov Report

Merging #317 (b9a3c80) into release/v0.46.x-celestia (06c2fc2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@                    Coverage Diff                    @@
##           release/v0.46.x-celestia     #317   +/-   ##
=========================================================
  Coverage                     65.54%   65.55%           
=========================================================
  Files                           666      666           
  Lines                         70983    70986    +3     
=========================================================
+ Hits                          46529    46532    +3     
  Misses                        21876    21876           
  Partials                       2578     2578           
Impacted Files Coverage Δ
x/genutil/client/cli/init.go 68.14% <100.00%> (+0.86%) ⬆️

Copy link

@cmwaters cmwaters left a comment

Choose a reason for hiding this comment

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

FYI, it is possible that InitChain override these values. But otherwise looks good

@evan-forbes evan-forbes merged commit 7d7c38d into release/v0.46.x-celestia May 12, 2023
33 checks passed
@evan-forbes evan-forbes deleted the evan/add-consensus-params branch May 12, 2023 18:20
evan-forbes added a commit that referenced this pull request Sep 4, 2023
…tion (#317)

* feat: allow for the default consensus params to be set by the application

* chore: add test

* fix: spelling

Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>

---------

Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>
evan-forbes added a commit that referenced this pull request Sep 5, 2023
…tion (#317) (#345)

* feat: allow for the default consensus params to be set by the application

* chore: add test

* fix: spelling



---------

Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>
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.

None yet

4 participants