v4 upgrade #393

asalzmann · 2022-11-25T23:55:54Z

Co-authored-by: Hieu Vu 72878483+hieuvubk@users.noreply.github.com
Co-authored-by: Jacob Gadikian jacobgadikian@gmail.com

Closes: #384

Context and purpose of the change

As part of the v3 upgrade, we added authz. However, we forgot to set the store key as part of the upgrade. As a consequence, after the upgrade, validators that attempted to prune had apphash errors. More details from @chillyvee here.

Brief Changelog

Add upgrade_tests to the v3 / v4 upgrade
Initialize the params store for the claim and authz modules
Add v4 upgrade handler and set the authzkeeper.StoreKey

Author's Checklist

I have...

Run and PASSED locally all GAIA integration tests
If the change is contentful, I either:
- Added a new unit test OR
- Added test cases to existing unit tests
OR this change is a trivial rework / code cleanup without any test coverage

If skipped any of the tests above, explain.

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

reviewed state machine logic
reviewed API design and naming
manually tested (if applicable)
confirmed the author wrote unit tests for new logic
reviewed documentation exists and is accurate

Documentation and Release Note

Does this pull request introduce a new feature or user-facing behavior changes?
Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md?
This pull request updates existing proto field values (and require a backend and frontend migration)?
Does this pull request change existing proto field names (and require a frontend migration)?
How is the feature or change documented?
- not applicable
- jira ticket XXX
- specification (x/<module>/spec/)
- README.md
- not documented

Co-authored-by: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Co-authored-by: Jacob Gadikian <jacobgadikian@gmail.com>

asalzmann · 2022-11-26T00:12:09Z

I couldn't push to the other branch (#384) to resolve the linting errors, so created a new one here. There are no changes aside from merging main into the original branch.

sampocs · 2022-11-26T01:26:09Z

See comments in #384
Also, can we add the param store key to the instructions inupgrades/README.md

faddat · 2022-11-27T21:27:11Z

Others should have a look also note that I merged in main.

sampocs · 2022-11-27T21:50:48Z

@asalzmann this diverged from #384 - wanna merge in the recent changes into this one?

Co-authored-by: Hieu Vu <72878483+hieuvubk@users.noreply.github.com>

asalzmann · 2022-11-27T22:15:08Z

Merged in. @faddat is it possible for me to get access to the notional stride fork? It would simplify the collaboration workflow (right now my workflow is to merge the notional branch into a separate stride branch, then that stride branch into main, which is a bit cumbersome)

sampocs

lgtm! Two nit comments

sampocs · 2022-11-27T22:56:34Z

app/apptesting/test_helpers.go

@@ -36,6 +39,7 @@ var (
 type AppTestHelper struct {
 	suite.Suite

+	Context 	sdk.Context


sampocs · 2022-11-27T22:56:39Z

app/apptesting/test_helpers.go

@@ -57,6 +61,7 @@ func (s *AppTestHelper) Setup() {
 		GRPCQueryRouter: s.App.GRPCQueryRouter(),
 		Ctx:             s.Ctx(),
 	}
+	s.Context = s.App.BaseApp.NewContext(false, tmtypes.Header{Height: 1, ChainID: StrideChainID, Time: time.Now().UTC()})


faddat · 2022-11-28T11:11:44Z

Yes, I'll add you there.

faddat · 2022-11-28T11:17:42Z

@asalzmann there's an invite for you at:

https://github.com/notional-labs/stride/invitations

faddat

This PR should change the import path, I think. If we'd like to do that separately, please let me know, and I'll un-request these changes :)

sampocs · 2022-11-29T15:19:34Z

@asalzmann was the renaming of Stride-Labs → Stride-labs intentional?

This reverts commit 56b41e6.

This reverts commit 4b89d37.

sampocs · 2022-11-29T19:14:40Z

I reverted the v4 import path updates - let's do that in a separate PR as we continue to debug this handler. @faddat can you remove the change request?

CHANGELOG.md

asalzmann · 2022-12-03T00:50:08Z

app/upgrades/v4/README.md

@@ -0,0 +1,6 @@
+# Upgrade v4 Changelog


Rm this file

chatted offline, leaving this in

asalzmann · 2022-12-03T01:10:14Z

app/app.go

@@ -1002,6 +1002,8 @@ func initParamsKeeper(appCodec codec.BinaryCodec, legacyAmino *codec.LegacyAmino
 	paramsKeeper.Subspace(icacallbacksmoduletypes.ModuleName)
 	// this line is used by starport scaffolding # stargate/app/paramSubspace

+	paramsKeeper.Subspace(claimtypes.ModuleName)
+	paramsKeeper.Subspace(authz.ModuleName)


Let's leave this out of this upgrade

asalzmann · 2022-12-03T01:10:32Z

app/upgrades.go

 		app.SetStoreLoader(upgradetypes.UpgradeStoreLoader(upgradeInfo.Height, storeUpgrades))
+	case "v4":
+		app.SetStoreLoader(AuthzHeightAdjustmentUpgradeStoreLoader(upgradeInfo.Height))


Let's move this to v5

asalzmann · 2022-12-03T01:10:41Z

app/upgrades/v4/upgrades_test.go

+				// make sure authz module was init
+				afterCtx := suite.Ctx().WithBlockHeight(dummyUpgradeHeight)
+				actGenState := suite.App.AuthzKeeper.ExportGenesis(afterCtx)
+				expGenState := authz.DefaultGenesisState()


Co-authored-by: Aidan Salzmann <aidan@stridelabs.co>

Co-authored-by: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Co-authored-by: Jacob Gadikian <jacobgadikian@gmail.com> Co-authored-by: sampocs <sam.pochyly@gmail.com> Co-authored-by: khanh-notional <50263489+catShaark@users.noreply.github.com> Co-authored-by: sampocs <sam@stridelabs.co>

Hieu/upgrade handler (#392)

7c9a5bf

Co-authored-by: Hieu Vu <72878483+hieuvubk@users.noreply.github.com> Co-authored-by: Jacob Gadikian <jacobgadikian@gmail.com>

github-actions bot added the C:app-wiring label Nov 25, 2022

asalzmann requested a review from a team November 25, 2022 23:57

asalzmann changed the title ~~Hieu/upgrade handler (#392)~~ v4 upgrade Nov 25, 2022

Merge branch 'main' into v4

b52ac1a

asalzmann marked this pull request as ready for review November 26, 2022 00:09

asalzmann mentioned this pull request Nov 27, 2022

v4/upgrade handler #384

Closed

19 tasks

Merge branch 'main' into v4

917b9ec

faddat approved these changes Nov 27, 2022

View reviewed changes

V4 updated (#399)

a547c6a

Co-authored-by: Hieu Vu <72878483+hieuvubk@users.noreply.github.com>

asalzmann added the v4 label Nov 27, 2022

asalzmann added 2 commits November 27, 2022 17:50

v4.0.0 and changelog

77379a9

Merge branch 'v4' of github.com:Stride-Labs/stride into v4

1d435b4

github-actions bot added the C:CLI label Nov 27, 2022

sampocs reviewed Nov 27, 2022

View reviewed changes

added store key instructions to readme

eca40f4

sampocs force-pushed the v4 branch from bd10678 to eca40f4 Compare November 27, 2022 23:03

faddat added 2 commits November 28, 2022 18:11

Merge branch 'main' into v4

1d328c1

fix linter

d676275

fix nits

7790f15

faddat requested changes Nov 28, 2022

View reviewed changes

increase timeout for golangci-lint

5a6f5ff

github-actions bot added the T:CI label Nov 28, 2022

sampocs added 4 commits November 29, 2022 10:42

added return nil to custom authz store loader

9d5935c

Revert "v3 -> v4"

cc899e1

This reverts commit 56b41e6.

Revert "v3 -> v4"

1a5de7d

This reverts commit 4b89d37.

go mod tidy

b0ddeb1

github-actions bot removed C:mint C:epochs C:records C:interchainquery C:proto C:simulator C:stakeibc C:test labels Nov 29, 2022

faddat approved these changes Dec 2, 2022

View reviewed changes

Merge branch 'main' into v4

d9a2054

asalzmann commented Dec 3, 2022

View reviewed changes

Merge branch 'main' into v4

77ccbcc

github-actions bot removed the T:build label Dec 3, 2022

sampocs and others added 7 commits December 2, 2022 19:21

removed authz store removal from upgrade

5a990e1

Update CHANGELOG.md

1d2b8f3

Co-authored-by: Aidan Salzmann <aidan@stridelabs.co>

fixed upgrade unit tests

a291585

Merge branch 'v4' of github.com:Stride-Labs/stride into v4

4735226

removed authz from params init

76c5ebf

updated v4 changelog in upgrades directory

30cf02d

revert store loader in upgrades

32b585d

sampocs force-pushed the v4 branch from 7cc996b to 32b585d Compare December 3, 2022 01:29

trimmed upgrade handler

6160c14

asalzmann merged commit 928618b into main Dec 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

v4 upgrade #393

v4 upgrade #393

asalzmann commented Nov 25, 2022 •

edited

Loading

asalzmann commented Nov 26, 2022

sampocs commented Nov 26, 2022

faddat commented Nov 27, 2022

sampocs commented Nov 27, 2022

asalzmann commented Nov 27, 2022

sampocs left a comment

sampocs Nov 27, 2022

sampocs Nov 27, 2022

faddat commented Nov 28, 2022

faddat commented Nov 28, 2022

faddat left a comment

sampocs commented Nov 29, 2022

sampocs commented Nov 29, 2022

asalzmann Dec 3, 2022

sampocs Dec 3, 2022 •

edited

Loading

asalzmann Dec 3, 2022

asalzmann Dec 3, 2022

sampocs Dec 3, 2022

asalzmann Dec 3, 2022

sampocs Dec 3, 2022

v4 upgrade #393

v4 upgrade #393

Conversation

asalzmann commented Nov 25, 2022 • edited Loading

Context and purpose of the change

Brief Changelog

Author's Checklist

Reviewers Checklist

Documentation and Release Note

asalzmann commented Nov 26, 2022

sampocs commented Nov 26, 2022

faddat commented Nov 27, 2022

sampocs commented Nov 27, 2022

asalzmann commented Nov 27, 2022

sampocs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

faddat commented Nov 28, 2022

faddat commented Nov 28, 2022

faddat left a comment

Choose a reason for hiding this comment

sampocs commented Nov 29, 2022

sampocs commented Nov 29, 2022

Choose a reason for hiding this comment

sampocs Dec 3, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asalzmann commented Nov 25, 2022 •

edited

Loading

sampocs Dec 3, 2022 •

edited

Loading