feat: make vat-localchain resumable #9488

0xpatrickdev · 2024-06-11T21:48:48Z

closes: #9484 closes: #9497

Description

Changes the type of every return in localchain.js to PromiseVow
Uses watch for .query() and .deposit() methods and returns a Vow
Updates call sites outside vat-localchain to expect and unwrap PromiseVows (using V)

Upgrade Considerations

These changes better prepare us for upgrades - consumers will know to expect a PromiseVow.

cloudflare-workers-and-pages · 2024-06-11T21:49:18Z

Deploying agoric-sdk with Cloudflare Pages

Latest commit:	`3027adf`
Status:	✅ Deploy successful!
Preview URL:	https://56427138.agoric-sdk.pages.dev
Branch Preview URL:	https://9484-localchain-vows.agoric-sdk.pages.dev

View logs

0xpatrickdev · 2024-06-11T22:03:59Z

packages/vats/src/localchain.js

+        .returns(M.promise()),
+    }),
+    () => ({}),
+    /** @type {Watcher<Brand<'nat'>, Amount<'nat'>>} */


We struggled a little on the return type of this handler (defined as depositPaymentHandler further down). Without this declaration - Watch<T, TResult1> - it thinks the return type is T (the value we get in onFulfilled). Returning the identity type is the default behavior of watch when no handler is supplied.

Really weird. This @type declaration looks incorrect to me... I think tsc is ignoring it. To have it be a typecast, you need to surround the next expression (the object literal containing the onFulfilled method) with parens, like /** @type {...} */ ({ /** @param ... */ onFulfilled(brand, ...) {} }).

https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html#casts

As an aside, a "Watcher" is what you're currently calling "Handler", and I think what you're calling "Watcher" is actually just a "promise" or "vow".

You're right, this typecast is incorrect here and not needed. I think there were typos in the depositPaymentHandler handler so we didn't have inferred types.

0xpatrickdev · 2024-06-12T00:43:26Z

packages/orchestration/src/exos/local-chain-account-kit.js

+          const [response] = await V(lca).executeTx([
            typedJson('/cosmos.staking.v1beta1.MsgUndelegate', {
              amount,
              validatorAddress,
              delegatorAddress,
            }),
          ]);
          trace('undelegate response', response);
+          // @ts-expect-error V is swallowing generic parameter?
          const { completionTime } = response;


It seems we are losing the inferred return type with V here (and other executeTx call sites). If I hover over executeTx with E, I see:

(method) executeTx<(MsgUndelegate & { '@type': "/cosmos.staking.v1beta1.MsgUndelegate"; })[]>(messages: (MsgUndelegate & { '@type': "/cosmos.staking.v1beta1.MsgUndelegate"; })[]): PromiseVow<...>

With V:

(method) executeTx(messages: { '@type': string; }[]): Promise<JsonSafe<{ '@type': string; }>[]>

This is a consequence of the when unwrapping that V implicitly does. I'm sure there's a way to fix it, but I'd have to spend some time coaxing it out... Vow typing is a finicky beast.

Created #9497 so we can address this outside of this PR

mhofman · 2024-06-12T00:54:22Z

packages/vats/src/localchain.js

+        const getBrandWatcher = watch(E(payment).getAllegedBrand());
+        return watch(getBrandWatcher, depositPaymentHandler, {


Why the double watch here?

Suggested change

const getBrandWatcher = watch(E(payment).getAllegedBrand());

return watch(getBrandWatcher, depositPaymentHandler, {

return watch(E(payment).getAllegedBrand(), depositPaymentHandler, {

Still learning conventions and thought this was more readable. Aside - it would be awesome to have a style guide / best practices doc for these (naming is tough as well). Happy to apply these changes

The result of an eventual send is durably watchable, so there is no need to wrap it in a vow, which is what watch without a handler effectively does.

You can keep it split over 2 lines, without the first watch, in which case I'd just name it result, or whatever the convention is (I'm am not the best source for that)

Suggested change

const getBrandWatcher = watch(E(payment).getAllegedBrand());

return watch(getBrandWatcher, depositPaymentHandler, {

const getBrandResult = E(payment).getAllegedBrand();

return watch(getBrandResult, depositPaymentHandler, {

mhofman · 2024-06-12T01:00:28Z

packages/vats/src/localchain.js

-        const allegedBrand = await E(payment).getAllegedBrand();
-        const purse = E(bank).getPurse(allegedBrand);
-        return E(purse).deposit(payment, optAmountShape);
+        const depositPaymentHandler = makeDepositPaymentHandler();


I thought that having a single depositPaymentHandler per LocalChain instance actually made sense. For that matter it could be a facet of a kit. bank would come from state, and payment and optAmountShape from bound values.

I thought that having a single depositPaymentHandler per LocalChain instance actually made sense

The revision where it was in state?

For that matter it could be a facet of a kit.

We considered a kit but thought this approach would result in fewer refactors. Can give a kit a go.

Yeah a singleton in state or a facet, both work IMO. No need to allocate a new deposit handler per invocation.

turadg · 2024-06-14T13:54:46Z

packages/internal/src/types.d.ts

+/**
+ * Potentially remote promises or settled references.
+ */
+export type FarRef<Primary, Local = DataOnly<Primary>> = ERef<


not a blocker for this PR but type should have a test.

turadg · 2024-06-14T13:55:31Z

packages/vow/src/E.js

@@ -327,7 +329,7 @@ export default makeE;
 /**
 * @template T
 * @typedef {{
- *   readonly [P in keyof T]: T[P] extends PromiseLike<infer U>
+ *   readonly [P in keyof T]: T[P] extends PromiseLike<infer _>


since the binding isn't used,

Suggested change

* readonly [P in keyof T]: T[P] extends PromiseLike<infer _>

* readonly [P in keyof T]: T[P] extends PromiseLike<any>

Done, among other infer _s.

turadg · 2024-06-14T13:58:23Z

packages/vow/src/E.js

+ *    ReturnType<T> extends EUnwrap<ReturnType<T>>
+ *    ? (...args: Parameters<T>) => Promise<ReturnType<T>>
+ *    : ReturnType<T> extends Promise<EUnwrap<ReturnType<T>>>
+ *    ? T
+ *    : (...args: Parameters<T>) => Promise<EUnwrap<ReturnType<T>>>


without nesting this is hard to read. here's how prettier-plugin-jsdoc formats it:

Suggested change

* ReturnType<T> extends EUnwrap<ReturnType<T>>

* ? (...args: Parameters<T>) => Promise<ReturnType<T>>

* : ReturnType<T> extends Promise<EUnwrap<ReturnType<T>>>

* ? T

* : (...args: Parameters<T>) => Promise<EUnwrap<ReturnType<T>>>

* @typedef {ReturnType<T> extends EUnwrap<ReturnType<T>>

* ? (...args: Parameters<T>) => Promise<ReturnType<T>>

* : ReturnType<T> extends Promise<EUnwrap<ReturnType<T>>>

* ? T

* : (...args: Parameters<T>) => Promise<EUnwrap<ReturnType<T>>>} ECallable

turadg · 2024-06-14T14:03:06Z

packages/vow/src/E.js

-const makeE = (
-  HandledPromise,
-  {
+const makeE = (HandledPromise, powers = {}) => {


why does SDK have it's own makeE? How does this differ from Endo's? Not in scope of this PR but very important to document.

Co-authored-by: Ikenna Omekam <ikenna@agoric.com>

michaelfig

Thanks, everybody. I'm merging this in now, with a few TODOs captured as future improvements.

closes: #9484 closes: #9497 ## Description - Changes the type of every return in `localchain.js` to `PromiseVow` - Uses `watch` for `.query()` and `.deposit()` methods and returns a `Vow` - Updates call sites outside `vat-localchain` to expect and unwrap PromiseVows (using `V`) ### Upgrade Considerations These changes better prepare us for upgrades - consumers will know to expect a `PromiseVow`.

Rebase todo: ``` # Branch fix-vow-make-watch-when-more-robust-against-loops-and-hangs-9487- label base-fix-vow-make-watch-when-more-robust-against-loops-and-hangs-9487- pick bcecf52 test(vow): add test of more vow upgrade scenarios pick d7135b2 test: switch vow test to run under xs for metering pick 99fccca test(vow): add test for resolving vow to external promise pick 6d3f88c test(vow): add test for vow based infinite vat ping pong pick c78ff0e test(vow): check vow consumers for busy loops or hangs pick 3c63cba fix(vow): prevent loops and hangs from watching promises pick 188c810 chore(vat-data): remove the deprecated `@agoric/vat-data/vow.js` pick 44a6d16 fix(vow): allow resolving vow to external promise label fix-vow-make-watch-when-more-robust-against-loops-and-hangs-9487- reset base-fix-vow-make-watch-when-more-robust-against-loops-and-hangs-9487- merge -C 4fca040 fix-vow-make-watch-when-more-robust-against-loops-and-hangs-9487- # fix(vow): make watch/when more robust against loops and hangs (#9487) # Branch ci-mergify-strip-merge-commit-HTML-comments-9499- label base-ci-mergify-strip-merge-commit-HTML-comments-9499- pick 63e21ab ci(mergify): strip merge commit HTML comments label ci-mergify-strip-merge-commit-HTML-comments-9499- reset base-ci-mergify-strip-merge-commit-HTML-comments-9499- merge -C 7b93671 ci-mergify-strip-merge-commit-HTML-comments-9499- # ci(mergify): strip merge commit HTML comments (#9499) # Pull request #9506 pick 249a5d4 fix(SwingSet): Undo deviceTools behavioral change from #9153 (#9506) # Pull request #9507 pick a19a964 fix(liveslots): promise watcher to cause unhandled rejection if no handler (#9507) # Branch feat-make-vat-localchain-resumable-9488- label base-feat-make-vat-localchain-resumable-9488- pick 76c17c6 feat: make vat-localchain resumable pick 40ccba1 fix(vow): correct the typing of `unwrap` pick 90e062c fix(localchain): work around TypeScript mapped tuple bug pick 3027adf fix(network): use new `ERef` and `FarRef` label feat-make-vat-localchain-resumable-9488- reset base-feat-make-vat-localchain-resumable-9488- merge -C 5856dc0 feat-make-vat-localchain-resumable-9488- # feat: make vat-localchain resumable (#9488) # Branch ci-mergify-clarify-queue-conditions-and-merge-conditions-9510- label base-ci-mergify-clarify-queue-conditions-and-merge-conditions-9510- pick 7247bd9 ci(mergify): clarify `queue_conditions` and `merge_conditions` label ci-mergify-clarify-queue-conditions-and-merge-conditions-9510- reset base-ci-mergify-clarify-queue-conditions-and-merge-conditions-9510- merge -C 30e56ae ci-mergify-clarify-queue-conditions-and-merge-conditions-9510- # ci(mergify): clarify `queue_conditions` and `merge_conditions` (#9510) # Branch fix-liveslots-cache-delete-does-not-return-a-useful-value-9509- label base-fix-liveslots-cache-delete-does-not-return-a-useful-value-9509- pick 42ea8a3 fix(liveslots): cache.delete() does not return a useful value label fix-liveslots-cache-delete-does-not-return-a-useful-value-9509- reset base-fix-liveslots-cache-delete-does-not-return-a-useful-value-9509- merge -C a2e54e1 fix-liveslots-cache-delete-does-not-return-a-useful-value-9509- # fix(liveslots): cache.delete() does not return a useful value (#9509) # Branch chore-swingset-re-enable-test-of-unrecognizable-orphan-cleanup-8694- label base-chore-swingset-re-enable-test-of-unrecognizable-orphan-cleanup-8694- pick 9930bd3 chore(swingset): re-enable test of unrecognizable orphan cleanup label chore-swingset-re-enable-test-of-unrecognizable-orphan-cleanup-8694- reset base-chore-swingset-re-enable-test-of-unrecognizable-orphan-cleanup-8694- merge -C bc53ef7 chore-swingset-re-enable-test-of-unrecognizable-orphan-cleanup-8694- # chore(swingset): re-enable test of unrecognizable orphan cleanup (#8694) # Pull request #9508 pick 513adc9 refactor(internal): move async helpers using AggregateError to node (#9508) # Branch report-bundle-sizing-in-agoric-run-9503- label base-report-bundle-sizing-in-agoric-run-9503- pick 68af59c refactor: inline addRunOptions pick a0115ed feat: writeCoreEval returns plan pick bd0edcb feat: stat-bundle and stat-plan scripts pick 0405202 feat: agoric run --verbose pick 22b43da feat(stat-bundle): show CLI to explode the bundle label report-bundle-sizing-in-agoric-run-9503- reset base-report-bundle-sizing-in-agoric-run-9503- merge -C 7b30169 report-bundle-sizing-in-agoric-run-9503- # report bundle sizing in agoric run (#9503) # Branch ci-test-boot-split-up-test-jobs-via-AVA-recipe-9511- label base-ci-test-boot-split-up-test-jobs-via-AVA-recipe-9511- pick 5f3c1d1 test(boot): use a single bundle directory for all tests pick 50229bd ci(all-packages): split tests according to AVA recipe label ci-test-boot-split-up-test-jobs-via-AVA-recipe-9511- reset base-ci-test-boot-split-up-test-jobs-via-AVA-recipe-9511- merge -C 5375019 ci-test-boot-split-up-test-jobs-via-AVA-recipe-9511- # ci(test-boot): split up test jobs via AVA recipe (#9511) # Pull request #9514 pick f908f89 fix: endow with original unstructured `assert` (#9514) # Pull request #9535 pick 989aa19 fix(swingset): log vat termination and upgrade better (#9535) # Branch types-zoe-setTestJig-param-type-optional-9533- label base-types-zoe-setTestJig-param-type-optional-9533- pick 426a3be types(zoe): setTestJig param type optional label types-zoe-setTestJig-param-type-optional-9533- reset base-types-zoe-setTestJig-param-type-optional-9533- merge -C bf9f03b types-zoe-setTestJig-param-type-optional-9533- # types(zoe): setTestJig param type optional (#9533) # Branch adopt-TypeScript-5-5-9476- label base-adopt-TypeScript-5-5-9476- pick 381b6a8 chore(deps): bump Typescript to 5.5 release label adopt-TypeScript-5-5-9476- reset base-adopt-TypeScript-5-5-9476- merge -C 0cfea88 adopt-TypeScript-5-5-9476- # adopt TypeScript 5.5 (#9476) # Branch retry-flaky-agoric-cli-integration-test-9550- label base-retry-flaky-agoric-cli-integration-test-9550- pick 2a68510 ci: retry agoric-cli integration test label retry-flaky-agoric-cli-integration-test-9550- reset base-retry-flaky-agoric-cli-integration-test-9550- merge -C c5c52ec retry-flaky-agoric-cli-integration-test-9550- # retry flaky agoric-cli integration test (#9550) # Pull Request #9556 pick 0af876f fix(vow): watcher args instead of context (#9556) # Pull Request #9561 pick a4f86eb fix(vow): handle resolution loops in vows (#9561) # Branch Restore-a3p-tests-9557- label base-Restore-a3p-tests-9557- pick d36382d chore(a3p): restore localchain test pick 5ff628e Revert "test: drop or clean-up old Tests" pick b5cf8bd fix(localchain): `callWhen`s return `PromiseVow` label Restore-a3p-tests-9557- reset base-Restore-a3p-tests-9557- merge -C c65915e Restore-a3p-tests-9557- # Restore a3p tests (#9557) # Pull Request #9559 pick 6073b2b fix(agoric): convey tx opts to `agoric wallet` and subcommands (#9559) # Branch explicit-heapVowTools-9548- label base-explicit-heapVowTools-9548- pick 100de68 feat!: export heapVowTools pick 8cb1ee7 refactor: use heapVowTools import pick 0ac6774 docs: vow vat utils pick 9128f27 feat: export heapVowE pick 3b0c8c1 refactor: use heapVowE pick 9b84bfa BREAKING CHANGE: drop V export pick 6623af5 chore(types): concessions to prepack label explicit-heapVowTools-9548- reset base-explicit-heapVowTools-9548- merge -C 4440ce1 explicit-heapVowTools-9548- # explicit heapVowTools (#9548) # Pull Request #9564 pick 44926a7 fix(bn-patch): fix bad html evasion (#9564) # Branch Fix-upgrade-behaviors-9526- label base-Fix-upgrade-behaviors-9526- pick ef1e0a2 feat: Upgrade Zoe pick e4cc97c Revert "fix(a3p-integration): workaround zoe issues" pick 84dd229 feat: repair KREAd contract on zoe upgrade pick cb77160 test: validate KREAd character purchase pick e1d961e test: move vault upgrade from test to use phase (#9536) label Fix-upgrade-behaviors-9526- reset base-Fix-upgrade-behaviors-9526- merge -C 8d05faf Fix-upgrade-behaviors-9526- # Fix upgrade behaviors (#9526) # Branch Support-for-snapshots-export-command-9563- label base-Support-for-snapshots-export-command-9563- pick 2a3976e refactor(cosmos): use DefaultBaseappOptions for newApp pick 84208e9 fix(cosmos): use dedicated dedicate app creator for non start commands pick 8c1a62d chore(cosmos): refactor cosmos command extension pick 4386f8e feat(cosmos): support snapshot export pick 2dabb52 test(a3p): add test for snapshots export and restore label Support-for-snapshots-export-command-9563- reset base-Support-for-snapshots-export-command-9563- merge -C 309c7e1 Support-for-snapshots-export-command-9563- # Support for `snapshots export` command (#9563) # Branch Swing-store-export-data-outside-of-genesis-file-9549- label base-Swing-store-export-data-outside-of-genesis-file-9549- pick f1eacbe fix(x/swingset): handle defer errors on export write pick 496a430 feat(cosmos): add hooking kv reader pick f476bd5 feat(cosmos): separate swing-store export data from genesis file pick 17a5374 test(a3p): add genesis fork acceptance test label Swing-store-export-data-outside-of-genesis-file-9549- reset base-Swing-store-export-data-outside-of-genesis-file-9549- merge -C 3aa5d66 Swing-store-export-data-outside-of-genesis-file-9549- # Swing-store export data outside of genesis file (#9549) # Branch Remove-scaled-price-authority-upgrade-9585- label base-Remove-scaled-price-authority-upgrade-9585- pick bce49e3 test: add test during upgradeVaults; vaults detect new prices pick 88f6500 test: repair test by dropping upgrade of scaledPriceAuthorities label Remove-scaled-price-authority-upgrade-9585- reset base-Remove-scaled-price-authority-upgrade-9585- merge -C 8376991 Remove-scaled-price-authority-upgrade-9585- # Remove scaled price authority upgrade (#9585) # Branch feat-make-software-upgrade-coreProposals-conditional-on-upgrade-plan-name-9575- label base-feat-make-software-upgrade-coreProposals-conditional-on-upgrade-plan-name-9575- pick 95174a2 feat(builders): non-ambient `strictPriceFeedProposalBuilder` in `priceFeedSupport.js` pick 5cc190d feat(app): establish mechanism for adding core proposals by `upgradePlan.name` pick 52f02b7 fix(builders): use proper `oracleBrand` subkey case pick ddc072d chore(cosmos): extract `app/upgrade.go` pick b3182a4 chore: fix error handling of upgrade vaults proposal pick ea568a2 fix: retry upgrade vaults price quote label feat-make-software-upgrade-coreProposals-conditional-on-upgrade-plan-name-9575- reset base-feat-make-software-upgrade-coreProposals-conditional-on-upgrade-plan-name-9575- merge -C cbe061c feat-make-software-upgrade-coreProposals-conditional-on-upgrade-plan-name-9575- # feat: make software upgrade `coreProposals` conditional on upgrade plan name (#9575) ``` This is followed by a commit to remove the `orchestration` and `async-flow` packages from the release, as these are not baked in yet and are not deployed anyway.

0xpatrickdev force-pushed the 9484/localchain-vows branch 2 times, most recently from c6adcc9 to ece3ec4 Compare June 11, 2024 21:53

0xpatrickdev commented Jun 11, 2024

View reviewed changes

0xpatrickdev commented Jun 12, 2024

View reviewed changes

mhofman reviewed Jun 12, 2024

View reviewed changes

0xpatrickdev mentioned this pull request Jun 12, 2024

Unwrap from @agoric/vow causes type information to be lost #9497

Closed

0xpatrickdev requested review from mhofman and michaelfig June 12, 2024 18:18

turadg reviewed Jun 14, 2024

View reviewed changes

feat: make vat-localchain resumable

76c17c6

Co-authored-by: Ikenna Omekam <ikenna@agoric.com>

michaelfig force-pushed the 9484/localchain-vows branch from a18dc71 to 24388f5 Compare June 14, 2024 16:01

michaelfig added 3 commits June 14, 2024 10:06

fix(vow): correct the typing of unwrap

40ccba1

fix(localchain): work around TypeScript mapped tuple bug

90e062c

fix(network): use new ERef and FarRef

3027adf

michaelfig force-pushed the 9484/localchain-vows branch from 24388f5 to 3027adf Compare June 14, 2024 16:07

michaelfig approved these changes Jun 14, 2024

View reviewed changes

michaelfig self-assigned this Jun 14, 2024

michaelfig added automerge:no-update (expert!) Automatically merge without updates asyncFlow related to membrane-based replay and upgrade of async functions labels Jun 14, 2024

mergify bot merged commit 5856dc0 into master Jun 14, 2024
74 checks passed

mergify bot deleted the 9484/localchain-vows branch June 14, 2024 16:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: make vat-localchain resumable #9488

feat: make vat-localchain resumable #9488

0xpatrickdev commented Jun 11, 2024 •

edited by michaelfig

Loading

cloudflare-workers-and-pages bot commented Jun 11, 2024 •

edited

Loading

0xpatrickdev Jun 11, 2024

michaelfig Jun 12, 2024

michaelfig Jun 12, 2024

0xpatrickdev Jun 12, 2024

0xpatrickdev Jun 12, 2024 •

edited

Loading

michaelfig Jun 12, 2024

0xpatrickdev Jun 12, 2024

mhofman Jun 12, 2024

0xpatrickdev Jun 12, 2024

mhofman Jun 12, 2024

mhofman Jun 12, 2024

0xpatrickdev Jun 12, 2024 •

edited

Loading

mhofman Jun 12, 2024

turadg Jun 14, 2024

michaelfig Jun 14, 2024

turadg Jun 14, 2024

michaelfig Jun 14, 2024

turadg Jun 14, 2024

michaelfig Jun 14, 2024

turadg Jun 14, 2024

michaelfig Jun 14, 2024

michaelfig left a comment

		const getBrandWatcher = watch(E(payment).getAllegedBrand());
		return watch(getBrandWatcher, depositPaymentHandler, {

	const getBrandWatcher = watch(E(payment).getAllegedBrand());
	return watch(getBrandWatcher, depositPaymentHandler, {
	return watch(E(payment).getAllegedBrand(), depositPaymentHandler, {

	* readonly [P in keyof T]: T[P] extends PromiseLike<infer _>
	* readonly [P in keyof T]: T[P] extends PromiseLike<any>

feat: make vat-localchain resumable #9488

feat: make vat-localchain resumable #9488

Conversation

0xpatrickdev commented Jun 11, 2024 • edited by michaelfig Loading

Description

Upgrade Considerations

cloudflare-workers-and-pages bot commented Jun 11, 2024 • edited Loading

Deploying agoric-sdk with Cloudflare Pages

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

0xpatrickdev Jun 12, 2024 • 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

0xpatrickdev Jun 12, 2024 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelfig left a comment

Choose a reason for hiding this comment

0xpatrickdev commented Jun 11, 2024 •

edited by michaelfig

Loading

cloudflare-workers-and-pages bot commented Jun 11, 2024 •

edited

Loading

0xpatrickdev Jun 12, 2024 •

edited

Loading

0xpatrickdev Jun 12, 2024 •

edited

Loading