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.

Sign up for GitHub

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

Jump to bottom

change all dapp deploy scripts to install by hash bundle, not source bundle #4564

Open
warner opened this issue Feb 16, 2022 · 6 comments
Open

change all dapp deploy scripts to install by hash bundle, not source bundle #4564

warner opened this issue Feb 16, 2022 · 6 comments
Assignees
@michaelfig
Labels
Dapp & UI Support deployment Chain deployment mechanism (e.g. testnet) enhancement New feature or request vaults_triage DO NOT USE

Comments

@warner
Copy link
Member

warner commented Feb 16, 2022

What is the Problem Being Solved?

After #4563 gives Zoe the ability to E(zoe).install(bundleID) (instead of a bundle), and after #4396 gives cosmic-swingset the ability to accept "install bundle" transactions, the next step of the process is to change the deploy scripts in all dapps to use them. Currently, these deploy scripts use bundleSource() on the local contract code, then E(zoe).install(bundle), which creates a large (swingset/mailbox) message to deliver the bundle to the chain and install it. That should change to:

  • use bundleSource() to get both a bundle and it's bundleID (hash)
  • use a new client-side facility to construct and send the install-bundle transaction message #4396 install-bundle transaction to the chain
  • send E(zoe).install(bundleID)

Description of the Design

We need the client-side facility to create the new transaction types.

Security Considerations

On the real chain, for the Mainnet-1 timeframe, there will be a governance process to limit the installation of new contracts. The bundleID must be in an approved list before the install-bundle transaction will be accepted. Test chains won't have such a limitation. I think the real chain process will look like:

  • author develops the contract code, test, finalize, publish source code for review
  • author bundles and computes the bundleID
  • author submits governance proposal to allow the bundleID to be installed
    • voters will look at the alleged source code (a git tag in some repo) for correctness/safety
    • voters will rebuild the bundle, compute the ID, compare against proposal
  • voters approve the proposal
    • ID goes onto the approved-IDs list
    • E(zoe).install(ID) is called by the proposal code, waits for bundle to be installed
  • author runs deploy script
    • deploy script constructs install-bundle txn, signs, submits
    • txn goes into block, accepted by approved-IDs list, causes controller.validateAndInstallBundle()
    • E(zoe).install() wakes up because the promise that fires once a bundle is installed #4521 "bundle is now installed" promise fires
    • proposal code adds installation handle to.. the Board?
    • installation complete, available for instantiation

Test Plan

cc @michaelfig
@warner warner added enhancement New feature or request Dapp & UI Support labels Feb 16, 2022
@warner warner mentioned this issue Feb 16, 2022
Open
@warner warner added the deployment Chain deployment mechanism (e.g. testnet) label Feb 16, 2022
warner added a commit that referenced this issue Feb 16, 2022
@warner
feat(zoe): implement E(zoe).install(bundleOrBundleID)
9c960fd 
Previously, `E(zoe).install()` accepted a source bundle. Now it accepts
either a source bundle, or a "bundle ID". The ID is a hash-based identifier
string that refers to a bundle installed into the kernel via
`controller.validateAndInstallBundle()`, and is retrievable from
vatAdminService. Zoe exchanges the ID for a bundlecap, and retains the
bundlecap for future use (including passing to the new ZCF vat, which
converts it into a source bundle for evaluation at the last possible moment).

The kernel install step can happen either before or after `E(zoe).install`,
because the id-to-bundlecap conversion waits until the kernel install is
complete.

This begins the process of making Zoe work exclusively with (small)
bundlecaps, and not (large) source bundles. The next step is to modify all
unit tests and external callers (including deploy scripts, #4564) to
kernel-install their bundle and use a bundleID for the Zoe install()
invocation. After that is complete, #4565 will remove support for full
bundles, and `E(zoe).install(bundleID)` will be the only choice.

closes #4563
@warner warner mentioned this issue Feb 16, 2022
Closed
@Tartuffo
Copy link
Contributor

@kriskowal Is this something you could take on?

@kriskowal
Copy link
Member

I can. I would bump the estimate to 5. I’ll need to get up-to-speed on @warner’s bundle caps API and some kernel layers I haven’t touched yet. The extra time is probably a good investment.

@Tartuffo
Copy link
Contributor

Estimate bumped.

@kriskowal kriskowal self-assigned this Feb 24, 2022
@warner warner assigned warner and unassigned warner Feb 25, 2022
@Tartuffo Tartuffo added this to the Mainnet 1 milestone Mar 23, 2022
@kriskowal kriskowal changed the title change all dapp deploy scripts to install by ID, not bundle change all dapp deploy scripts to install by hash bundle, not source bundle Mar 31, 2022
@kriskowal kriskowal mentioned this issue Mar 31, 2022
Merged
This was referenced Apr 5, 2022
Closed
Closed
@Tartuffo
Copy link
Contributor

This needs to happen for the RUN Protocol Review release, but not for all DApps, just for the scripts that deploy the proposal(s) to start RUN protocol. We need someone to pick up this work. FYI @dckc

@michaelfig
Copy link
Member

The current plan that @kriskowal is working on does not require any code changes to deploy scripts. We may still change deploy scripts for other reasons (captured in other issues), but this is not one of them.

@dckc dckc mentioned this issue Apr 18, 2022
Closed
@kriskowal kriskowal mentioned this issue May 27, 2022
Merged
@kriskowal kriskowal reopened this May 27, 2022
@kriskowal
Copy link
Member

The plan has changed. Dapps will need to performa a migration like: Agoric/dapp-fungible-faucet#46

This is because bundleSource and publishBundle are orthogonal concerns. bundleSource should be pinned in yarn.lock, and cannot be pinned down if agoric-sdk is linked and not installed. publishBundle has an optional ConnectionSpec argument shouldn’t be threaded through bundleSource implicitly.

These aren’t absolute constraints, but I think strong enough to make the separation between the two functions clear in dapp deploy scripts.

@warner warner mentioned this issue Aug 13, 2022
Open
@Tartuffo Tartuffo removed this from the Mainnet 1 RC0 milestone Sep 21, 2022
@ivanlei ivanlei added the vaults_triage DO NOT USE label Dec 4, 2022
@ivanlei ivanlei added this to the Vaults RC0 milestone Feb 1, 2023
@ivanlei ivanlei assigned michaelfig and unassigned kriskowal Feb 9, 2023
@ivanlei ivanlei removed this from the Vaults Functional Testing milestone Feb 9, 2023
@ivanlei ivanlei removed the MUST-HAVE label Mar 20, 2023
@kriskowal kriskowal mentioned this issue Jun 17, 2024
Open
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@michaelfig michaelfig

Labels
Dapp & UI Support deployment Chain deployment mechanism (e.g. testnet) enhancement New feature or request vaults_triage DO NOT USE
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@warner @kriskowal @michaelfig @Tartuffo @ivanlei