Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(pfm): Migration to align expected escrow state #231

Merged
merged 18 commits into from
Feb 11, 2025

Conversation

drklee3
Copy link
Contributor

@drklee3 drklee3 commented Feb 11, 2025

Based off #230 to add state migration and upgrade tests

  • Migration & tests
  • Resolves some CI build issues

@github-actions github-actions bot added the packet-forward-middleware Label for items related to the packet forward middleware label Feb 11, 2025
@drklee3 drklee3 marked this pull request as ready for review February 11, 2025 20:09

// Migrate migrates the x/packetforward module state from the consensus version
// 2 to version 3
func Migrate(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not a very standard migration here since it affects other modules instead of the current module state, open to feedback! Alternatively, resolving state in it in an upgrade handler without bumping consensus versions could be an option

@wllmshao wllmshao merged commit 616afdf into cosmos:ws/keeper Feb 11, 2025
8 of 13 checks passed
wllmshao added a commit that referenced this pull request Feb 11, 2025
* fix: don't unescrow tokens that are moved between escrow accounts

* add more tests

* consolidate new tests

* feat(pfm): Migration to align expected escrow state (#231)

* feat(packetforward): Bump consensus version from 2 to 3

* feat(packetforward): Add empty migration setup

* test(packetforward): Initialize mocks and tests for migration

* feat(packetforward): Implement migration

* fix(packetforward): Use correct expected keeper method names and parameters

* docs: Migration test steps

* test(packetforward): Upgrade e2e test

* fix: Upgrade in app

* fix: Use preblock for upgrade

* fix: Set missing app PreBlocker

* fix: Use corrected 8.1.0 pre-upgrade image

SetPreBlocker was not set, so upgrade module did not trigger. Also,
upgrade handlers should not be set in the previous binary.

* ci: Update deprecated artifacts actions

* ci: Update golangci/golangci-lint-action

* docs: Remove inaccurate previous image upgrade info

* ci: Update e2e test to use 8.1.0 pre-upgrade docker image

* docs: Test case docs

* build: Replace 8.1.0 pre-upgrade image with linux/amd64

* test: Enable upgrade handlers in local simapp for upgrade e2e test

---------

Co-authored-by: drklee3 <github@dlee.dev>
mergify bot pushed a commit that referenced this pull request Feb 11, 2025
* fix: don't unescrow tokens that are moved between escrow accounts

* add more tests

* consolidate new tests

* feat(pfm): Migration to align expected escrow state (#231)

* feat(packetforward): Bump consensus version from 2 to 3

* feat(packetforward): Add empty migration setup

* test(packetforward): Initialize mocks and tests for migration

* feat(packetforward): Implement migration

* fix(packetforward): Use correct expected keeper method names and parameters

* docs: Migration test steps

* test(packetforward): Upgrade e2e test

* fix: Upgrade in app

* fix: Use preblock for upgrade

* fix: Set missing app PreBlocker

* fix: Use corrected 8.1.0 pre-upgrade image

SetPreBlocker was not set, so upgrade module did not trigger. Also,
upgrade handlers should not be set in the previous binary.

* ci: Update deprecated artifacts actions

* ci: Update golangci/golangci-lint-action

* docs: Remove inaccurate previous image upgrade info

* ci: Update e2e test to use 8.1.0 pre-upgrade docker image

* docs: Test case docs

* build: Replace 8.1.0 pre-upgrade image with linux/amd64

* test: Enable upgrade handlers in local simapp for upgrade e2e test

---------

Co-authored-by: drklee3 <github@dlee.dev>
(cherry picked from commit 515bdca)

# Conflicts:
#	middleware/packet-forward-middleware/Dockerfile
#	middleware/packet-forward-middleware/e2e/upgrade_test.go
#	middleware/packet-forward-middleware/packetforward/types/expected_keepers.go
#	middleware/packet-forward-middleware/testing/simapp/app.go
mergify bot pushed a commit that referenced this pull request Feb 11, 2025
* fix: don't unescrow tokens that are moved between escrow accounts

* add more tests

* consolidate new tests

* feat(pfm): Migration to align expected escrow state (#231)

* feat(packetforward): Bump consensus version from 2 to 3

* feat(packetforward): Add empty migration setup

* test(packetforward): Initialize mocks and tests for migration

* feat(packetforward): Implement migration

* fix(packetforward): Use correct expected keeper method names and parameters

* docs: Migration test steps

* test(packetforward): Upgrade e2e test

* fix: Upgrade in app

* fix: Use preblock for upgrade

* fix: Set missing app PreBlocker

* fix: Use corrected 8.1.0 pre-upgrade image

SetPreBlocker was not set, so upgrade module did not trigger. Also,
upgrade handlers should not be set in the previous binary.

* ci: Update deprecated artifacts actions

* ci: Update golangci/golangci-lint-action

* docs: Remove inaccurate previous image upgrade info

* ci: Update e2e test to use 8.1.0 pre-upgrade docker image

* docs: Test case docs

* build: Replace 8.1.0 pre-upgrade image with linux/amd64

* test: Enable upgrade handlers in local simapp for upgrade e2e test

---------

Co-authored-by: drklee3 <github@dlee.dev>
(cherry picked from commit 515bdca)

# Conflicts:
#	.github/workflows/packet-forward-middleware.yml
#	middleware/packet-forward-middleware/Dockerfile
#	middleware/packet-forward-middleware/packetforward/keeper/keeper.go
#	middleware/packet-forward-middleware/packetforward/module.go
#	middleware/packet-forward-middleware/packetforward/types/expected_keepers.go
#	middleware/packet-forward-middleware/test/mock/transfer_keeper.go
#	middleware/packet-forward-middleware/testing/previous_images/README.md
#	middleware/packet-forward-middleware/testing/simapp/app.go
#	middleware/packet-forward-middleware/testing/simapp/upgrades/upgrades.go
mergify bot added a commit that referenced this pull request Feb 12, 2025
…between escrow accounts (#232)

* fix: don't unescrow tokens that are moved between escrow accounts (#230)

* fix: don't unescrow tokens that are moved between escrow accounts

* add more tests

* consolidate new tests

* feat(pfm): Migration to align expected escrow state (#231)

* feat(packetforward): Bump consensus version from 2 to 3

* feat(packetforward): Add empty migration setup

* test(packetforward): Initialize mocks and tests for migration

* feat(packetforward): Implement migration

* fix(packetforward): Use correct expected keeper method names and parameters

* docs: Migration test steps

* test(packetforward): Upgrade e2e test

* fix: Upgrade in app

* fix: Use preblock for upgrade

* fix: Set missing app PreBlocker

* fix: Use corrected 8.1.0 pre-upgrade image

SetPreBlocker was not set, so upgrade module did not trigger. Also,
upgrade handlers should not be set in the previous binary.

* ci: Update deprecated artifacts actions

* ci: Update golangci/golangci-lint-action

* docs: Remove inaccurate previous image upgrade info

* ci: Update e2e test to use 8.1.0 pre-upgrade docker image

* docs: Test case docs

* build: Replace 8.1.0 pre-upgrade image with linux/amd64

* test: Enable upgrade handlers in local simapp for upgrade e2e test

---------

Co-authored-by: drklee3 <github@dlee.dev>
(cherry picked from commit 515bdca)

# Conflicts:
#	middleware/packet-forward-middleware/Dockerfile
#	middleware/packet-forward-middleware/e2e/upgrade_test.go
#	middleware/packet-forward-middleware/packetforward/types/expected_keepers.go
#	middleware/packet-forward-middleware/testing/simapp/app.go

* fix conflicts

* remove v8 previous image

* fix import

* fix wrong pfm versions

* use pfm 7.2.0 for upgrade tests

---------

Co-authored-by: William Shao <wllmshao@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
packet-forward-middleware Label for items related to the packet forward middleware
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants