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

refactor(hub-genesis): refactoring x/hub-genesis state #374

Merged
merged 8 commits into from
Apr 11, 2024

Conversation

mtsitrin
Copy link
Collaborator

@mtsitrin mtsitrin commented Apr 8, 2024

PR Standards

Opening a pull request should be able to meet the following requirements


Closes #XXX

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.

For Author:

  • Targeted PR against correct branch
  • included the correct type prefix in the PR title
  • Linked to Github issue with discussion and accepted design
  • Targets only one github issue
  • Wrote unit and integration tests
  • All CI checks have passed
  • Added relevant godoc comments

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 targets main branch, PR should be squashed and merged.
  • In case PR targets a release branch, PR should be rebased.

@mtsitrin mtsitrin requested a review from omritoptix April 8, 2024 19:09
@mtsitrin mtsitrin marked this pull request as ready for review April 8, 2024 19:09
@mtsitrin mtsitrin requested a review from a team as a code owner April 8, 2024 19:09
proto/hub-genesis/locked.proto Outdated Show resolved Hide resolved
x/hub-genesis/keeper/keeper.go Outdated Show resolved Hide resolved
x/hub-genesis/keeper/keeper.go Outdated Show resolved Hide resolved
@mtsitrin mtsitrin requested review from zale144 and danwt April 9, 2024 17:41
@omritoptix
Copy link
Contributor

@mtsitrin I see that still we check is_locked as an indicator. why not is_genesis_event to be more generic and conform to the hub?

 // is_locked is a boolean that indicates if the genesis event has occured
    bool is_locked = 1;

@mtsitrin
Copy link
Collaborator Author

@mtsitrin I see that still we check is_locked as an indicator. why not is_genesis_event to be more generic and conform to the hub?

 // is_locked is a boolean that indicates if the genesis event has occured
    bool is_locked = 1;
  1. I don't like hub's is_genesis_event (:
  2. it's bit different than the hub:
  • It's not mandatory on the rollapp, only if need to lock tokens
  • it has no other use than lock tokens

I'm fine with is_locked

@omritoptix omritoptix changed the title feat: refactoring x/hub-genesis state refactor(hub-genesis): refactoring x/hub-genesis state Apr 10, 2024
@zale144
Copy link
Contributor

zale144 commented Apr 11, 2024

@omritoptix @mtsitrin So if we trigger the genesis event on the hub, we mint the tokens for the rollapp genesis accounts, shouldn't we make sure that we lock the equal amount on the rollapp side?

@omritoptix
Copy link
Contributor

@zale144 yes this will be done on next phases when it's all done automatically by IBC otherwise not sure how we can acheive that now easily.

@omritoptix omritoptix merged commit a0a367f into main Apr 11, 2024
6 checks passed
@omritoptix omritoptix deleted the mtsitrin/363-the-hub-field-cannot-be-exported branch April 11, 2024 12:53
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 genesis trigger test to be based on boolean and not hub id The Hub field cannot be exported
3 participants