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

chore(app): move initialization logic outside of the app and into keepers package #975

Merged
merged 16 commits into from
Jul 15, 2024

Conversation

zale144
Copy link
Contributor

@zale144 zale144 commented Jul 3, 2024

Description

In the PR #972 the upgrade handlers are refactored so that they use an interface for the upgrades themselves and keepers are extracted to be in a separate package.

Here we go a step further and move the initializing logic out of app.go and into the new package keepers.


Closes #320

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.

PR review checkboxes:

I have...

  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Targeted PR against the correct branch
  • included the correct type prefix in the PR title
  • Linked to the GitHub issue with discussion and accepted design
  • Targets only one GitHub issue
  • Wrote unit and integration tests
  • Wrote relevant migration scripts if necessary
  • All CI checks have passed
  • Added relevant godoc comments
  • Updated the scripts for local run, e.g genesis_config_commands.sh if the PR changes parameters
  • Add an issue in the e2e-tests repo if necessary

SDK Checklist

  • Import/Export Genesis
  • Registered Invariants
  • Registered Events
  • Updated openapi.yaml
  • No usage of go map
  • No usage of time.Now()
  • Used fixed point arithmetic and not float arithmetic
  • Avoid panicking in Begin/End block as much as possible
  • No unexpected math Overflow
  • Used sendCoin and not SendCoins
  • Out-of-block compute is bounded
  • No serialized ID at the end of store keys
  • UInt to byte conversion should use BigEndian

Full security checklist here

----;

For Reviewer:

  • Confirmed the correct type prefix in the PR title
  • Reviewers assigned
  • Confirmed all author checklist items have been addressed

---;

After reviewer approval:

  • In case the PR targets the main branch, PR should not be squash merge in order to keep meaningful git history.
  • In case the PR targets a release branch, PR must be rebased.

@zale144 zale144 self-assigned this Jul 3, 2024
@zale144 zale144 requested a review from a team as a code owner July 3, 2024 11:01
@zale144 zale144 changed the base branch from main to zale144/paginate-delete-rollapp-packets July 3, 2024 11:02
app/keepers/keys.go Fixed Show fixed Hide fixed
@zale144 zale144 force-pushed the zale144/paginate-delete-rollapp-packets-refactoring branch from 7af6793 to 5f893bd Compare July 3, 2024 11:36
@omritoptix
Copy link
Contributor

@zale144 please give it a meaningful pr title. thanks.

@zale144 zale144 changed the title chore(app) Further refactor on top of PR #972 chore(app) App refactor after PR #972, move initialization logic outside of the app and into keepers package Jul 4, 2024
danwt
danwt previously approved these changes Jul 5, 2024
Copy link
Contributor

@danwt danwt left a comment

Choose a reason for hiding this comment

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

nice refactor
I understand the keeprs pckage now

app/keepers/keys.go Outdated Show resolved Hide resolved
app/keepers/keepers.go Outdated Show resolved Hide resolved
@omritoptix omritoptix changed the title chore(app) App refactor after PR #972, move initialization logic outside of the app and into keepers package chore(app): App refactor after PR #972, move initialization logic outside of the app and into keepers package Jul 8, 2024
@omritoptix omritoptix changed the title chore(app): App refactor after PR #972, move initialization logic outside of the app and into keepers package chore(app): Move initialization logic outside of the app and into keepers package Jul 8, 2024
@omritoptix omritoptix changed the title chore(app): Move initialization logic outside of the app and into keepers package chore(app): move initialization logic outside of the app and into keepers package Jul 8, 2024
Base automatically changed from zale144/paginate-delete-rollapp-packets to main July 9, 2024 11:06
@mtsitrin mtsitrin dismissed danwt’s stale review July 9, 2024 11:06

The base branch was changed.

Comment on lines +207 to +209
for acc := range maccPerms {
modAccAddrs[authtypes.NewModuleAddress(acc).String()] = true
}

Check warning

Code scanning / CodeQL

Iteration over map Warning

Iteration over map may be a possible source of non-determinism
app/keepers/keepers.go Outdated Show resolved Hide resolved
@zale144 zale144 force-pushed the zale144/paginate-delete-rollapp-packets-refactoring branch from fdb319e to a551a5d Compare July 15, 2024 13:22
@mtsitrin mtsitrin merged commit 90104b6 into main Jul 15, 2024
93 of 96 checks passed
@mtsitrin mtsitrin deleted the zale144/paginate-delete-rollapp-packets-refactoring branch July 15, 2024 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants