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

[BP: release/v6 <- #230] fix: don't unescrow tokens that are moved between escrow accounts #233

Closed
wants to merge 1 commit into from

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Feb 11, 2025

When pfm is moving packets from one escrow account to another in the refund flow of a failed forward, it is erroneously calling unescrowTokens which is making the accounting in the total escrow wrong.

includes migration to fix broken escrow state (h/t @drklee3)


This is an automatic backport of pull request #230 done by Mergify.

* 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
Copy link
Author

mergify bot commented Feb 11, 2025

Cherry-pick of 515bdca has failed:

On branch mergify/bp/release/v6/pr-230
Your branch is up to date with 'origin/release/v6'.

You are currently cherry-picking commit 515bdca.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   middleware/packet-forward-middleware/e2e/packet_forward_test.go
	modified:   middleware/packet-forward-middleware/e2e/upgrade_test.go
	new file:   middleware/packet-forward-middleware/packetforward/keeper/migrator.go
	new file:   middleware/packet-forward-middleware/packetforward/migrations/v3/migrate.go
	new file:   middleware/packet-forward-middleware/packetforward/migrations/v3/migrate_test.go
	modified:   middleware/packet-forward-middleware/test/mock/bank_keeper.go
	modified:   middleware/packet-forward-middleware/test/mock/channel_keeper.go
	new file:   middleware/packet-forward-middleware/testing/previous_images/pfm_8_1_0.tar

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)
	deleted by us:   .github/workflows/packet-forward-middleware.yml
	deleted by us:   middleware/packet-forward-middleware/Dockerfile
	both modified:   middleware/packet-forward-middleware/packetforward/keeper/keeper.go
	both modified:   middleware/packet-forward-middleware/packetforward/module.go
	both modified:   middleware/packet-forward-middleware/packetforward/types/expected_keepers.go
	both modified:   middleware/packet-forward-middleware/test/mock/transfer_keeper.go
	deleted by us:   middleware/packet-forward-middleware/testing/previous_images/README.md
	deleted by us:   middleware/packet-forward-middleware/testing/simapp/app.go
	deleted by us:   middleware/packet-forward-middleware/testing/simapp/upgrades/upgrades.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@wllmshao wllmshao marked this pull request as draft February 11, 2025 23:55
@wllmshao wllmshao closed this Feb 12, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant