Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add proposal for terminating KREAd vats #4

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

Jorge-Lopes
Copy link
Collaborator

@Jorge-Lopes Jorge-Lopes commented Jan 10, 2025

closes: https://github.com/Agoric/BytePitchPartnerEng/issues/46

Description

As part of the sunsetting process for the KREAd application, it is necessary to terminate the KREAd contract, governor, committee, and charter vats. This step is essential to ensure a clean and secure shutdown of the application and to free up associated resources.

This PR introduces:

  • A new core-eval script to handle the termination of the KREAd-related vats.
  • An a3p-integration proposal to validate the script's behavior.

Key Changes

Core-Eval Script:

  • Added a new script to execute the termination of KREAd vats.

A3P-Integration Enhancements:

  • Copied the agoric-sdk/a3p-integration directory, including the z:acceptance proposal.
  • Removed kread.test.js from the acceptance test phase.
  • Introduced a new x:kread proposal to test the KREAd termination process.

Environment Setup:

  • Added a .env file requiring the AGORIC_SDK_ROOT variable to be updated with the full path to the agoric-sdk root directory.
  • Included a script (agoric-sdk-symlink.sh) to create a symbolic link to the agoric-sdk.

Testing Considerations

The x:kread proposal was introduced to test the behaviour of the executing the KREAd terminating core-eval, as well as test how it handles new offer requests to the KREAd contract after terminated.
The z:acceptance proposal confirms that the Agoric chain remains functional after the termination of KREAd vats.

Note that it is necessary to update the value of AGORIC_SDK_ROOT at the agoric/a3p-integration/.env with the one from your local environment. And then execute the agoric/a3p-integration/agoric-sdk-symlink.sh script.
After executing these 2 steps, the remaining steps are the same as usual.

Additional Considerations

After merging this PR, the next logical step would be to create a new release. This release should include artifacts generated by the agoric run command, which can then be utilized with the Cosmos Proposal Builder.
Generated artifacts will be available in agoric/a3p-integration/proposals/x:kread/kread-submission.

@anilhelvaci
Copy link
Collaborator

anilhelvaci pushed a commit that referenced this pull request Jan 13, 2025
## Description

The purpose of this PR is to update the KREAd frontend code to use the
JSON API instead of the deprecated RPC method to query the Vstorage.

With this in mind, it was necessary to update the `@agoric/rpc` and
`@agoric/web-components` packages, and the respective imported functions
being used by KREAd.

Changes made to original code:
- At commit 5c6bdaf the packages
mentioned above are updated to the latest version.
- At commit 42f886f 
- the `fetchChainInfo` function is updated to return the `api` along
with the `rpc` and `chainName`
- the `connectAgoric` function is updated to pass the `rpc` as argument
to `makeAgoricWalletConnection`
- the `startWatching` function is updated to pass the `api` as argument
to `makeAgoricChainStorageWatcher`
- At commit fbeeedb on all calls of the
`watchLatest` method of `chainStorageWatcher`, the `onError` argument
was removed, since it is no longer expected.

## Related Issues

Fixes the following issues:
- [#4](Jorge-Lopes#4)
- [#9145](Agoric/agoric-sdk#9145)

## Checklist

Make sure all items are checked before submitting the pull request.
Remove any items that are not applicable.

- [x] The PR title is clear and concise.
- [x] Are there changes in the /fronted folder? Make sure `cd frontend
&& yarn build` runs successfully.;
@Jorge-Lopes Jorge-Lopes changed the title Add proposal for terminating relevant vats Add proposal for terminating KREAd vats Jan 14, 2025
@Chris-Hibbert
Copy link

n:upgrade-next which allows testing against the current chain state.
The present state of n: upgrade-next reflects tests that upgrade-18 succeeds. Now that U18 has been run on mainNet #211 will remove most of those tests and move a few to z:acceptance. This PR should probably mostly follow that.

x:kread/test.sh doesn't invoke ava. Doesn't it need yarn ava ./*.test.js or yarn ava or something in order to start the tests?

The test calls evalBundles('kread-submission'), but there's no such directory checked in, nor instructions in the package.json to generate it. What's the plan?

@Jorge-Lopes
Copy link
Collaborator Author

Jorge-Lopes commented Jan 15, 2025

The present state of n: upgrade-next reflects tests that upgrade-18 succeeds. Now that U18 has been run on mainNet #211 will remove most of those tests and move a few to z:acceptance. This PR should probably mostly follow that.

The purpose of including the n:upgrade-next proposal was to test the KREAd termination in a state that already included the PR #10267.
If I understand your suggestion correctly, since U18 is now on mainNet, there is no longer a need to include this proposal, since the agoric-3-proposals:latest image should provide the necessary chain state.
@Chris-Hibbert Can you please confirm if this understanding is accurate?

x:kread/test.sh doesn't invoke ava. Doesn't it need yarn ava ./*.test.js or yarn ava or something in order to start the tests?

The are no post-build tests for the x:kreadproposal, only during the eval phase.
I’ve updated the test.sh script to make this clearer.

The test calls evalBundles('kread-submission'), but there's no such directory checked in, nor instructions in the package.json to generate it. What's the plan?

Thank you very much for bringing this to my attention. I failed to notice that the submissions were not being commited since they are included in the gitignore. I’ve fixed this issue

@Jorge-Lopes Jorge-Lopes force-pushed the jorge/terminating-contract branch from b7e413c to 8bdf0e6 Compare January 15, 2025 16:01
@Chris-Hibbert
Copy link

I'm confused. The new state only has one changed file (src/proposal/terminate-kread.js). I was expecting to see an updated a3p-integration test based on post-upgrade-18 chain state. This development may have to wait on the post-U18 cleanup.

since U18 is now on mainNet, there is no longer a need to include this proposal, since the agoric-3-proposals:latest image should provide the necessary chain state.

After #211, the ability to terminate vats should be present and n:upgrade-next should be unnecessary. But we'll still need to add x:kread in order to kill the various vats, and z:acceptance to observe that the chain returns to health afterwards.

@Jorge-Lopes
Copy link
Collaborator Author

Jorge-Lopes commented Jan 15, 2025

I'm confused. The new state only has one changed file (src/proposal/terminate-kread.js). I was expecting to see an updated a3p-integration test based on post-upgrade-18 chain state. This development may have to wait on the post-U18 cleanup.

@Chris-Hibbert sorry, I have decided to improve my implementation to allow the use of the yarn build command without the previous limitations.
As you suggested, I removed the n:upgrade-next.

@Jorge-Lopes Jorge-Lopes force-pushed the jorge/terminating-contract branch from ff43505 to 8bdf0e6 Compare January 16, 2025 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants