use contract "start" consistently #8045

turadg · 2023-07-12T18:16:45Z

Description

While describing contract upgrade to @carlos-kryha I couldn't come up with a good explanation for why upgradable contracts name the start functionprepare.

This keeps the start name and makes an explicit declaration under meta of the upgradability the contract claims.

Security Considerations

n/a

Scaling Considerations

n/a

Documentation Considerations

Any other docs about "prepare" will need to be updated.

This is a breaking change to downstream consumers, but will appear in the changelogs and is trivial to comply with.

Testing Considerations

CI

toliaqat · 2023-07-12T18:52:50Z

packages/zoe/src/contractFacet/zcfZygote.js

    buildRootObject === undefined ||
      Fail`Did you provide a vat bundle instead of a contract bundle?`;
-    Fail`contract exports missing start/prepare`;
+    Fail`contract exports missing start`;


Will this break current users of agoric using prepare? Should we communicate this change to our partners?

toliaqat · 2023-07-12T18:55:13Z

packages/zoe/src/contractFacet/zcfZygote.js


          const areDurable = objectMap(
            { creatorFacet, publicFacet, creatorInvitation },
            canBeDurable,
          );
          const allDurable = Object.values(areDurable).every(Boolean);
-          if (prepare) {
+          if (takesBaggage) {
            allDurable ||
              Fail`values from prepare() must be durable ${areDurable}`;


Replace prepare with start in the error message.

toliaqat · 2023-07-12T18:55:49Z

packages/governance/src/contractGovernor.js

@@ -147,7 +147,7 @@ harden(validateQuestionFromCounter);
 * }>}
 * @param {import('@agoric/vat-data').Baggage} baggage
 */
-export const prepare = async (zcf, privateArgs, baggage) => {
+export const start = async (zcf, privateArgs, baggage) => {
  trace('prepare');


replace prepare with start in trace statements here and bellow in this file.

turadg · 2023-07-12T21:28:17Z

Closing as we need to support the case of a contract that will eventually be upgradable but isn't already. That is, it takes baggage but doesn't yet return durable facets. Fortunately this was detected in CI because auctioneer.js contract works that way.

Chris-Hibbert

This works for me.

packages/zoe/src/contractFacet/zcfZygote.js

Chris-Hibbert

@toliaqat's comments merit a response.

LGTM.

Chris-Hibbert · 2023-07-24T22:23:55Z

packages/governance/src/contractGovernor.js

+export const meta = {
+  upgradability: 'canUpgrade',
+};
+


harden(meta);

Chris-Hibbert · 2023-07-24T22:27:39Z

packages/inter-protocol/src/vaultFactory/vaultFactory.js

+export const meta = {
+  privateArgsShape: M.splitRecord(
+    harden({
+      marshaller: M.remotable('Marshaller'),
+      storageNode: StorageNodeShape,
+    }),
+    harden({
+      // only necessary on first invocation, not subsequent
+      feeMintAccess: FeeMintAccessShape,
+      initialPoserInvitation: InvitationShape,
+      initialShortfallInvitation: InvitationShape,
+    }),
+  ),
+  upgradability: 'canUpgrade',
+};


Suggested change

export const meta = {

privateArgsShape: M.splitRecord(

harden({

marshaller: M.remotable('Marshaller'),

storageNode: StorageNodeShape,

}),

harden({

// only necessary on first invocation, not subsequent

feeMintAccess: FeeMintAccessShape,

initialPoserInvitation: InvitationShape,

initialShortfallInvitation: InvitationShape,

}),

),

upgradability: 'canUpgrade',

};

export const meta = {

privateArgsShape: M.splitRecord(

{

marshaller: M.remotable('Marshaller'),

storageNode: StorageNodeShape,

},

{

// only necessary on first invocation, not subsequent

feeMintAccess: FeeMintAccessShape,

initialPoserInvitation: InvitationShape,

initialShortfallInvitation: InvitationShape,

},

),

upgradability: 'canUpgrade',

};

thanks. I thought splitRecord required hardened args

Chris-Hibbert · 2023-07-24T22:30:37Z

packages/inter-protocol/src/psm/psm.js

+  privateArgsShape: M.splitRecord(
+    harden({
+      marshaller: M.remotable('Marshaller'),
+      storageNode: StorageNodeShape,
+    }),
+    harden({
+      // only necessary on first invocation, not subsequent
+      feeMintAccess: FeeMintAccessShape,
+      initialPoserInvitation: InvitationShape,
+    }),
+  ),


Suggested change

privateArgsShape: M.splitRecord(

harden({

marshaller: M.remotable('Marshaller'),

storageNode: StorageNodeShape,

}),

harden({

// only necessary on first invocation, not subsequent

feeMintAccess: FeeMintAccessShape,

initialPoserInvitation: InvitationShape,

}),

),

privateArgsShape: M.splitRecord(

{

marshaller: M.remotable('Marshaller'),

storageNode: StorageNodeShape,

},

{

// only necessary on first invocation, not subsequent

feeMintAccess: FeeMintAccessShape,

initialPoserInvitation: InvitationShape,

},

),

Chris-Hibbert · 2023-07-24T22:31:21Z

packages/inter-protocol/src/psm/psm.js

+/** @typedef {Awaited<ReturnType<typeof start>>['publicFacet']} PsmPublicFacet */
+harden(start);


Suggested change

/** @typedef {Awaited<ReturnType<typeof start>>['publicFacet']} PsmPublicFacet */

harden(start);

harden(start);

/** @typedef {Awaited<ReturnType<typeof start>>['publicFacet']} PsmPublicFacet */

Chris-Hibbert · 2023-07-24T22:32:26Z

packages/vats/src/provisionPool.js

+export const meta = {
+  privateArgsShape: M.splitRecord(
+    harden({
+      poolBank: M.eref(M.remotable('bank')),
+      storageNode: M.eref(M.remotable('storageNode')),
+      marshaller: M.eref(M.remotable('marshaller')),
+    }),
+    harden({
+      // only necessary on first invocation, not subsequent
+      initialPoserInvitation: InvitationShape,
+      metricsOverride: M.recordOf(M.string()),
+    }),
+  ),
+  upgradability: 'canUpgrade',
+};


Suggested change

export const meta = {

privateArgsShape: M.splitRecord(

harden({

poolBank: M.eref(M.remotable('bank')),

storageNode: M.eref(M.remotable('storageNode')),

marshaller: M.eref(M.remotable('marshaller')),

}),

harden({

// only necessary on first invocation, not subsequent

initialPoserInvitation: InvitationShape,

metricsOverride: M.recordOf(M.string()),

}),

),

upgradability: 'canUpgrade',

};

export const meta = {

privateArgsShape: M.splitRecord(

{

poolBank: M.eref(M.remotable('bank')),

storageNode: M.eref(M.remotable('storageNode')),

marshaller: M.eref(M.remotable('marshaller')),

},

{

// only necessary on first invocation, not subsequent

initialPoserInvitation: InvitationShape,

metricsOverride: M.recordOf(M.string()),

},

),

upgradability: 'canUpgrade',

};

'start' is sufficient

Chris-Hibbert · 2023-07-26T16:59:01Z

packages/zoe/src/contractFacet/zcfZygote.js

+   * buildRootObject: undefined,
+   * start: ContractStartFn,
+   * meta?: ContractMeta,
+}>} */


The formatting looks broken here. Is github confused?

That is janky. I must have messed it up.

prettier-plugin-jsdoc would format it as,

/** * @type {() => Promise< * | { * buildRootObject: any; * start: undefined; * meta: undefined; * } * | { * buildRootObject: undefined; * start: ContractStartFn; * meta?: ContractMeta; * } * >} */

Since that automated our whitespace, I'll address this when we opt packages/zoe into pretty jsdoc

I started noticing that VS code is not auto formatting for some packages (i.e. cosmic-swingset). It does for SwingSet. Maybe I started noticing this in cosmic-swingset because I just now made some changes in it.

Please ignore my comment. I was missing settings file for cosmic-swingset in my project.

Chris-Hibbert

The formatting of one comment in zcfZygote looks broken. otherwise, looks great.

use contract "start" consistently

turadg changed the title ~~ta/contract start~~ use contract "start" consistently Jul 12, 2023

turadg force-pushed the ta/contract-start branch 2 times, most recently from 37aa6be to 5fe5330 Compare July 12, 2023 18:42

toliaqat reviewed Jul 12, 2023

View reviewed changes

Chris-Hibbert reviewed Jul 18, 2023

View reviewed changes

packages/zoe/src/contractFacet/zcfZygote.js Outdated Show resolved Hide resolved

turadg force-pushed the ta/contract-start branch 3 times, most recently from 3bd57b9 to f846088 Compare July 24, 2023 16:20

turadg requested a review from Chris-Hibbert July 24, 2023 21:19

turadg marked this pull request as ready for review July 24, 2023 21:19

Chris-Hibbert reviewed Jul 24, 2023

View reviewed changes

turadg force-pushed the ta/contract-start branch from f846088 to e7c9a9c Compare July 24, 2023 22:49

turadg enabled auto-merge July 24, 2023 22:49

turadg added 3 commits July 25, 2023 11:52

refactor: "meta" grouping of arg validation

af72d68

BREAKING CHANGE: remove 'prepare'

0bf7693

'start' is sufficient

feat: start fn upgradability by meta

5ae46e4

turadg force-pushed the ta/contract-start branch from e7c9a9c to 5ae46e4 Compare July 25, 2023 18:53

Chris-Hibbert reviewed Jul 26, 2023

View reviewed changes

Chris-Hibbert approved these changes Jul 26, 2023

View reviewed changes

turadg added this pull request to the merge queue Jul 26, 2023

Merged via the queue into master with commit ad3aa42 Jul 26, 2023

turadg deleted the ta/contract-start branch July 26, 2023 19:09

turadg mentioned this pull request Aug 4, 2023

8045 backwards compatibility #8146

Merged

mhofman pushed a commit that referenced this pull request Aug 7, 2023

Merge pull request #8045 from Agoric/ta/contract-start

3613f92

use contract "start" consistently

turadg mentioned this pull request Aug 7, 2023

test of restarting all vats #8158

Open

turadg mentioned this pull request Sep 26, 2023

contract upgrade test of KREAd #8386

Closed

Chris-Hibbert mentioned this pull request Jun 17, 2024

describe upgrade contract meta info Agoric/documentation#1126

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use contract "start" consistently #8045

use contract "start" consistently #8045

turadg commented Jul 12, 2023 •

edited

Loading

toliaqat Jul 12, 2023

toliaqat Jul 12, 2023 •

edited

Loading

turadg Jul 24, 2023

toliaqat Jul 12, 2023

turadg Jul 24, 2023

turadg commented Jul 12, 2023

Chris-Hibbert left a comment

Chris-Hibbert left a comment

Chris-Hibbert Jul 24, 2023

Chris-Hibbert Jul 24, 2023

turadg Jul 24, 2023 •

edited

Loading

Chris-Hibbert Jul 24, 2023

Chris-Hibbert Jul 24, 2023

Chris-Hibbert Jul 24, 2023

Chris-Hibbert Jul 26, 2023

turadg Jul 26, 2023

toliaqat Aug 4, 2023

toliaqat Aug 4, 2023

Chris-Hibbert left a comment

		/** @typedef {Awaited<ReturnType<typeof start>>['publicFacet']} PsmPublicFacet */
		harden(start);

use contract "start" consistently #8045

use contract "start" consistently #8045

Conversation

turadg commented Jul 12, 2023 • edited Loading

Description

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Choose a reason for hiding this comment

toliaqat Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turadg commented Jul 12, 2023

Chris-Hibbert left a comment

Choose a reason for hiding this comment

Chris-Hibbert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

turadg Jul 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chris-Hibbert left a comment

Choose a reason for hiding this comment

turadg commented Jul 12, 2023 •

edited

Loading

toliaqat Jul 12, 2023 •

edited

Loading

turadg Jul 24, 2023 •

edited

Loading