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.

Sign up for GitHub

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

Jump to bottom

feat: integrate interchain accounts #1669

Merged
merged 13 commits into from
Sep 8, 2022
Merged

feat: integrate interchain accounts #1669

merged 13 commits into from
Sep 8, 2022

Conversation

gsk967
Copy link
Contributor

@gsk967 gsk967 commented Jul 16, 2022

name: Pull Request
about: Submit a pull request
title: ''
labels: ''
assignees: '@boz @gosuri'

Description

Motivation and Context

  • I have raised an issue to propose this change (required)
  • My issue has received approval from the maintainers or lead with the design/approved label

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@gsk967 gsk967 marked this pull request as ready for review July 18, 2022 06:05
@anilcse anilcse changed the title feat: add interchain accounts feat: integrate interchain accounts Jul 18, 2022
anilcse
anilcse reviewed Jul 18, 2022
View reviewed changes
app/app.go Outdated Show resolved Hide resolved
anilcse
anilcse suggested changes Jul 18, 2022
View reviewed changes
Copy link
Contributor

@anilcse anilcse left a comment

Choose a reason for hiding this comment

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

add upgrade handler to handle ica genesis state. Allow all the messages. @gosuri @boz do you want to restrict to only specific message-types?

@github-actions
Copy link

Marked as stale; will be closed in five days.

Cut bait or go fishing!

@github-actions github-actions bot added the stale label Jul 31, 2022
@anilcse anilcse removed the stale label Jul 31, 2022
@gsk967 gsk967 requested a review from anilcse August 1, 2022 07:06
@gsk967
Copy link
Contributor Author

gsk967 commented Aug 1, 2022

Simulations are failing due to issue from ibc-go , ibc-go does n't implement the simulation interfaces
cosmos/ibc-go#1352

boz
boz requested changes Aug 2, 2022
View reviewed changes
Copy link
Contributor

@boz boz left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

Please fix the lint failures (errors).

@troian any thoughts on when would be a good time to merge this? I don't want it to impact the provider extraction.

app/app.go Outdated Show resolved Hide resolved
app/app.go Outdated
@@ -500,6 +557,43 @@ func (app *AkashApp) registerUpgradeHandlers() {
// configure store loader that checks if version == upgradeHeight and applies store upgrades
app.SetStoreLoader(upgradetypes.UpgradeStoreLoader(upgradeInfo.Height, &storeUpgrades))
}

// ica upgrade
const upgradeName = "akash_ica_upgrade"
Copy link
Contributor

Choose a reason for hiding this comment

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

are upgrades ordered by name? if so, can you make this "01-ica-upgrade" to leave room for "02-foo"?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, it doesn't care about the order. It just cares about the upgrade height. But we can have a better upgrade name.

x/icaauth/client/cli/flags.go Outdated Show resolved Hide resolved
x/icaauth/client/cli/query.go Outdated Show resolved Hide resolved
x/icaauth/ibc_module.go Outdated Show resolved Hide resolved
x/icaauth/keeper/keeper.go Show resolved Hide resolved
@boz boz added the keepalive Exempt these when managing stale PRs label Aug 2, 2022
@gsk967 gsk967 requested review from boz and anilcse and removed request for hydrogen18 August 2, 2022 09:24
@gsora gsora mentioned this pull request Aug 3, 2022
Closed
@boz
Copy link
Contributor

boz commented Aug 5, 2022

Simulations are failing due to issue from ibc-go , ibc-go does n't implement the simulation interfaces cosmos/ibc-go#1352

@alessio and team provided us a fix for this, I will DM it to you.

@gsk967
Copy link
Contributor Author

gsk967 commented Aug 17, 2022

@boz I fix the simulation issue, please can you look into changes

boz
boz reviewed Aug 22, 2022
View reviewed changes
Copy link
Contributor

@boz boz left a comment

Choose a reason for hiding this comment

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

Thanks @anilcse @gsk967!

We have a bit of release engineering to do before we merge this, will get it done soon!

@troian - defer to you on best way to handle this given with the repo split.

@troian
Copy link
Member

troian commented Aug 22, 2022

@gsk967 we'll incorporate the change right after #1646

@gosuri
Copy link
Member

gosuri commented Sep 7, 2022

@troian now that #1646 is merged, can we move forward with this one?

@troian
Copy link
Member

troian commented Sep 7, 2022

@gosuri yeah, will merge it

@troian
Copy link
Member

troian commented Sep 7, 2022

@gsk967 mind rebasing pr

@gsk967
Copy link
Contributor Author

gsk967 commented Sep 7, 2022

@gsk967 mind rebasing pr

sure

@troian troian merged commit 03749e3 into akash-network:master Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@boz boz boz requested changes

@troian troian troian approved these changes

@anilcse anilcse Awaiting requested review from anilcse

Assignees
No one assigned
Labels
keepalive Exempt these when managing stale PRs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@gsk967 @boz @troian @gosuri @anilcse