fix: adopt VTRANSFER_IBC_EVENT as an action-type
#9671
Merged
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
toliaqat
approved these changes
Jul 9, 2024
iomekam
approved these changes
Jul 9, 2024
Deploying agoric-sdk with
|
| Latest commit: |
526a5cf
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7ecb9a45.agoric-sdk.pages.dev |
| Branch Preview URL: | https://mfig-wire-vtransfer-ibc-even.agoric-sdk.pages.dev |
gibson042
approved these changes
Jul 9, 2024
| @@ -15,3 +15,4 @@ export const VBANK_BALANCE_UPDATE = 'VBANK_BALANCE_UPDATE'; | |||
| export const WALLET_ACTION = 'WALLET_ACTION'; | |||
| export const WALLET_SPEND_ACTION = 'WALLET_SPEND_ACTION'; | |||
| export const INSTALL_BUNDLE = 'INSTALL_BUNDLE'; | |||
| export const VTRANSFER_IBC_EVENT = 'VTRANSFER_IBC_EVENT'; | |||
There was a problem hiding this comment.
I'd like to see a comment above declaration of IBC_EVENT that following exports describe internal events for which handling code is expected in packages/cosmic-swingset/src/launch-chain.js.
There was a problem hiding this comment.
I'm not sure why this action needs a comment any more than any other of the actions above? I'd be ok restructuring to highlight which actions come through the queue and which come through the pipe.
mhofman
reviewed
Jul 9, 2024
| @@ -15,3 +15,4 @@ export const VBANK_BALANCE_UPDATE = 'VBANK_BALANCE_UPDATE'; | |||
| export const WALLET_ACTION = 'WALLET_ACTION'; | |||
| export const WALLET_SPEND_ACTION = 'WALLET_SPEND_ACTION'; | |||
| export const INSTALL_BUNDLE = 'INSTALL_BUNDLE'; | |||
| export const VTRANSFER_IBC_EVENT = 'VTRANSFER_IBC_EVENT'; | |||
There was a problem hiding this comment.
I'm not sure why this action needs a comment any more than any other of the actions above? I'd be ok restructuring to highlight which actions come through the queue and which come through the pipe.
| @@ -1,6 +1,7 @@ | |||
| // @ts-check | |||
| import { E } from '@endo/far'; | |||
| import { BridgeId as BRIDGE_ID, VTRANSFER_IBC_EVENT } from '@agoric/internal'; | |||
| import { BridgeId as BRIDGE_ID } from '@agoric/internal'; | |||
| import { VTRANSFER_IBC_EVENT } from '@agoric/internal/src/action-types.js'; | |||
There was a problem hiding this comment.
I am really confused why the transfer vat needs to be aware of the action type in the first place. This seems like a layering violation. No other action type leaks past the cosmic-swingset layer.
gibson042
pushed a commit
that referenced
this pull request
Jul 9, 2024
closes: #9670 Ensure that the `VTRANSFER_IBC_EVENT` messages are plumbed through the `cosmic-swingset` layer. Prevents chain halt when the `x/vtransfer` facilities are used. This bug was overlooked because end-to-end testing of this facility was not completed until a late stage. Earlier unit testing used mocks of the bridge device that did not have the same problematic behaviour. More automated end-to-end IBC testing in a close-to-production setting is needed. Fixing this `cosmic-swingset` behaviour does not require upgrade of any other components.
gibson042
added a commit
that referenced
this pull request
Jul 10, 2024
## Description Includes commits from the following PRs: * #9671 * #9672 ...plus a new commit introducing upgrade name "agoric-upgrade-16-2". Constructed using the following `git rebase -i HEAD` todo list: ``` # pull request #9671 # resolve conflicts by: # * `git rm packages/orchestration/test/supports.ts` # * in packages/vats/src/proposals/transfer-proposal.js, take the proposed imports # * in packages/vats/test/localchain.test.js, leave `import { NonNullish } from '@agoric/assert'` # but take the VTRANSFER_IBC_EVENT change pick 217005a fix: adopt `VTRANSFER_IBC_EVENT` as an action-type (#9671) # pull request #9672 branch mfig-update-swingset-configs label base-mfig-update-swingset-configs pick 870d205 fix(vm-config): always use `init-localchain` and `init-transfer` pick 236a3f0 chore(vm-config): remove obsolete `pegasus/init-core.js` pick 7f7a8bd chore(bank): demote noisy logs to `debug` level pick 9b317a0 docs: purpose of itest-vaults config label mfig-update-swingset-configs reset base-mfig-update-swingset-configs merge -C 8f019c0 mfig-update-swingset-configs # fix(vm-config): always use `init-localchain` and `init-transfer` (#9672) ```
gibson042
added a commit
that referenced
this pull request
Jul 10, 2024
## Packages that have NEWS.md updates ```diff --- c/packages/boot/CHANGELOG.md +++ w/packages/boot/CHANGELOG.md @@ -3,6 +3,15 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +## [0.2.0-u16.1](https://github.com/Agoric/agoric-sdk/compare/@agoric/[email protected]...@agoric/[email protected]) (2024-07-10) + + +### Bug Fixes + +* adopt `VTRANSFER_IBC_EVENT` as an action-type ([#9671](#9671)) ([67569d4](67569d4)), closes [#9670](#9670) + + + ## 0.2.0-u16.0 (2024-07-02) --- c/packages/cosmic-swingset/CHANGELOG.md +++ w/packages/cosmic-swingset/CHANGELOG.md @@ -3,6 +3,16 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +## [0.42.0-u16.1](https://github.com/Agoric/agoric-sdk/compare/@agoric/[email protected]...@agoric/[email protected]) (2024-07-10) + + +### Bug Fixes + +* adopt `VTRANSFER_IBC_EVENT` as an action-type ([#9671](#9671)) ([67569d4](67569d4)), closes [#9670](#9670) +* **vm-config:** always use `init-localchain` and `init-transfer` ([1db4ed6](1db4ed6)) + + + ## [0.42.0-u16.0](https://github.com/Agoric/agoric-sdk/compare/@agoric/[email protected]...@agoric/[email protected]) (2024-07-02) --- c/packages/internal/CHANGELOG.md +++ w/packages/internal/CHANGELOG.md @@ -3,6 +3,15 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +## [0.4.0-u16.1](https://github.com/Agoric/agoric-sdk/compare/@agoric/[email protected]...@agoric/[email protected]) (2024-07-10) + + +### Bug Fixes + +* adopt `VTRANSFER_IBC_EVENT` as an action-type ([#9671](#9671)) ([67569d4](67569d4)), closes [#9670](#9670) + + + ## [0.4.0-u16.0](https://github.com/Agoric/agoric-sdk/compare/@agoric/[email protected]...@agoric/[email protected]) (2024-07-02) --- c/packages/vats/CHANGELOG.md +++ w/packages/vats/CHANGELOG.md @@ -3,6 +3,15 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +## [0.16.0-u16.1](https://github.com/Agoric/agoric-sdk/compare/@agoric/[email protected]...@agoric/[email protected]) (2024-07-10) + + +### Bug Fixes + +* adopt `VTRANSFER_IBC_EVENT` as an action-type ([#9671](#9671)) ([67569d4](67569d4)), closes [#9670](#9670) + + + ## [0.16.0-u16.0](https://github.com/Agoric/agoric-sdk/compare/@agoric/[email protected]...@agoric/[email protected]) (2024-07-02) --- c/packages/vm-config/CHANGELOG.md +++ w/packages/vm-config/CHANGELOG.md @@ -3,6 +3,15 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +### [0.1.1-u16.1](https://github.com/Agoric/agoric-sdk/compare/@agoric/[email protected]...@agoric/[email protected]) (2024-07-10) + + +### Bug Fixes + +* **vm-config:** always use `init-localchain` and `init-transfer` ([1db4ed6](1db4ed6)) + + + ### 0.1.1-u16.0 (2024-07-02) ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in 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.
closes: #9670
Description
Ensure that the
VTRANSFER_IBC_EVENTmessages are plumbed through thecosmic-swingsetlayer.Security Considerations
Prevents chain halt when the
x/vtransferfacilities are used.Testing Considerations
This bug was overlooked because end-to-end testing of this facility was not completed until a late stage. Earlier unit testing used mocks of the bridge device that did not have the same problematic behaviour. More automated end-to-end IBC testing in a close-to-production setting is needed.
Upgrade Considerations
Fixing this
cosmic-swingsetbehaviour does not require upgrade of any other components.