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

Document process for creating proposals and submissions in a3p-integration. #9093

Merged
merged 4 commits into from
Apr 12, 2024

Conversation

Chris-Hibbert
Copy link
Contributor

refs: #8740

Description

Improved description of creating proposals and submissions in a3p-integration

Security Considerations

None

Scaling Considerations

None

Documentation Considerations

That's it.

Testing Considerations

None

Upgrade Considerations

None

@Chris-Hibbert Chris-Hibbert added the documentation Improvements or additions to documentation label Mar 14, 2024
@Chris-Hibbert Chris-Hibbert requested a review from turadg March 14, 2024 21:23
@Chris-Hibbert Chris-Hibbert self-assigned this Mar 14, 2024
@Chris-Hibbert
Copy link
Contributor Author

@turadg I needed to write this down again so I could refactor a proposal. Would you help me make this more correct? Or at least correct enough to check in.

@Chris-Hibbert Chris-Hibbert force-pushed the 8740-replacePriceFeeds branch from f57210e to 841c9c9 Compare March 14, 2024 23:44

For an example, see `a:upgrade-next` in master.

### Core-eval proposal

The `type` of a core-eval proposal is `"/agoric.swingset.CoreEvalProposal"`, and content is submitted from a `submission` subfolder.
The `type` of a core-eval proposal is `"/agoric.swingset.CoreEvalProposal"`, and content is submitted from a subfolder whose name defaults to `submission`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The `type` of a core-eval proposal is `"/agoric.swingset.CoreEvalProposal"`, and content is submitted from a subfolder whose name defaults to `submission`.
The `type` of a core-eval proposal is `"/agoric.swingset.CoreEvalProposal"`. By default the submission in the subdir `submission` is evaluated. The proposal package can override this by providing an `eval.sh` that is executed instead. See [run_eval.sh](https://github.com/Agoric/agoric-3-proposals/blob/main/packages/synthetic-chain/public/upgrade-test-scripts/run_eval.sh)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


If the proposal is planned to be executed after the chain software upgrade, and the source of the proposal is in `agoric-sdk`, it's recommended to not check-in the `submission` content in source control and instead generate it automatically when testing. Since proposals cannot access SDK code, a script can be used to generate the `submission` content. Until there is [native support for build scripts in the `synthetic-chain` tool](https://github.com/Agoric/agoric-3-proposals/issues/87), `a3p-integration`'s `build:submissions` step invokes `/script/generate-a3p-submission.sh` in `agoric-sdk` before starting the upgrade test.
If the proposal is planned to be executed after the chain software upgrade, and the source of the proposal is in `agoric-sdk`, it's recommended to not check-in the `submission` content in source control and instead generate it automatically when testing. Since proposals cannot access SDK code, a script can be used to generate the `submission` content. Until there is [native support for build scripts in the `synthetic-chain` tool](https://github.com/Agoric/agoric-3-proposals/issues/87), `a3p-integration`'s `build:submissions` step invokes `/script/generate-a3p-submissions.sh` in `agoric-sdk` before starting the upgrade test.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
If the proposal is planned to be executed after the chain software upgrade, and the source of the proposal is in `agoric-sdk`, it's recommended to not check-in the `submission` content in source control and instead generate it automatically when testing. Since proposals cannot access SDK code, a script can be used to generate the `submission` content. Until there is [native support for build scripts in the `synthetic-chain` tool](https://github.com/Agoric/agoric-3-proposals/issues/87), `a3p-integration`'s `build:submissions` step invokes `/script/generate-a3p-submissions.sh` in `agoric-sdk` before starting the upgrade test.
If the proposal is planned to be executed after the chain software upgrade, and the source of the proposal is in `agoric-sdk`, don't commit the built proposal because CI will test that instead of the current code.
Instead generate it automatically in each test run. Since proposals cannot access SDK code, a script can be used to generate the `submission` content. Until there is [native support for build scripts in the `synthetic-chain` tool](https://github.com/Agoric/agoric-3-proposals/issues/87), `a3p-integration`'s `build:submissions` step invokes `generate-a3p-submissions.sh` before starting the upgrade test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


Submissions that don't need to pass in options or generate references to source
bundles can be written directly in a `foo-submission` subdirectory of the
proposal. These would incude a .js or .tjs file, accompanied by a similarly-named
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
proposal. These would incude a .js or .tjs file, accompanied by a similarly-named
proposal. These would include a .js script, accompanied by a similarly-named

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

parameter.

If the submission does require a bundle reference, or other options to be
provided, it should be written as two parts. The proposal usually goes in
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
provided, it should be written as two parts. The proposal usually goes in
provided, it should be written as two parts: a core eval and a builder for it. E.g.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

provided, it should be written as two parts. The proposal usually goes in
`.../vats/proposals`, and the script in `.../builders/scripts/vats`.

### Proposal
Copy link
Member

Choose a reason for hiding this comment

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

From here down is not about a3p. I think it should go in deploy-script-support. cc @michaelfig @dckc for input.

Copy link
Member

Choose a reason for hiding this comment

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

sounds about right.

I wonder about folding it into docs on Permissioned Contract Deployment too.

Meanwhile, I added it to

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Parts of the section on scripts describes how to lay out files and directories in a3p. I don't think those considerations apply to deploy-script-support. I'm trying to tease those sections apart, and understand what can be said over there.

@@ -7,7 +7,8 @@
"coreProposals": []
},
"sdk-generate": [
"probe-zcf-bundle probe-submission"
"probe-zcf-bundle probe-submission",
Copy link
Member

Choose a reason for hiding this comment

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

please limit this PR to only Markdown and comments changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops. Wrong commit base.

@Chris-Hibbert Chris-Hibbert force-pushed the 8740-replacePriceFeeds branch 2 times, most recently from c3c8034 to 8296522 Compare March 19, 2024 22:43
@Chris-Hibbert
Copy link
Contributor Author

apologies for rebasing, but I wanted to look at it on top of #9044.

@Chris-Hibbert Chris-Hibbert requested a review from turadg March 20, 2024 00:26
Base automatically changed from 8740-replacePriceFeeds to master March 20, 2024 16:58
@Chris-Hibbert
Copy link
Contributor Author

Please take another look. It's now based directly on master, so all the extraneous stuff is gone.



**REVIEWERS**: _I don't know how much of this applies to building proposals for
Copy link
Member

Choose a reason for hiding this comment

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

a3p is running a synthetic mainnet, so it's the same either way.

remember to remove this comment before merge

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks.

Copy link

cloudflare-workers-and-pages bot commented Apr 12, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0083abf
Status: ✅  Deploy successful!
Preview URL: https://0f302220.agoric-sdk.pages.dev
Branch Preview URL: https://8740-docs.agoric-sdk.pages.dev

View logs

@Chris-Hibbert Chris-Hibbert added the automerge:squash Automatically squash merge label Apr 12, 2024
@mergify mergify bot merged commit 6578834 into master Apr 12, 2024
67 checks passed
@mergify mergify bot deleted the 8740-docs branch April 12, 2024 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:squash Automatically squash merge documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants