fix: don't unescrow tokens that are moved between escrow accounts #230
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
packet-forward-middleware
* 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
Eric-Warehime
approved these changes
Feb 11, 2025
| // We move funds from the escrowAddress in both cases, | ||
| // update the total escrow amount for the denom. | ||
| k.unescrowToken(ctx, token) | ||
| k.unescrowToken(ctx, token) |
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
This was referenced Feb 11, 2025
Merged
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When pfm is moving packets from one escrow account to another in the refund flow of a failed forward, it is erroneously calling
unescrowTokenswhich is making the accounting in the total escrow wrong.includes migration to fix broken escrow state (h/t @drklee3)