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

9449 chain facades return vows #9547

Merged
merged 2 commits into from
Jun 21, 2024
Merged

9449 chain facades return vows #9547

merged 2 commits into from
Jun 21, 2024

Conversation

turadg
Copy link
Member

@turadg turadg commented Jun 21, 2024

refs: #9449

Description

More exos returning vows. Note these are heap vows due to

export const vowTools = prepareVowTools(makeHeapZone());
export const { watch, when, makeVowKit, allVows } = vowTools;

Security Considerations

none

Scaling Considerations

none

Documentation Considerations

none

Testing Considerations

CI

Upgrade Considerations

none

@turadg turadg requested review from dckc and 0xpatrickdev June 21, 2024 01:25
Copy link

cloudflare-workers-and-pages bot commented Jun 21, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: f9b6857
Status: ✅  Deploy successful!
Preview URL: https://6d80d363.agoric-sdk.pages.dev
Branch Preview URL: https://9449-chain-facades.agoric-sdk.pages.dev

View logs

Comment on lines +78 to +79
// FIXME use watch() from vowTools
return watch(allVows([icaP, V(icaP).getAddress()]), {
Copy link
Member

Choose a reason for hiding this comment

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

Why not destructure from L42 and wrap the watcher in a Far?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because I want it to be more likely to fail in testing.

@turadg turadg added automerge:rebase Automatically rebase updates, then merge force:integration Force integration tests to run on PR and removed automerge:rebase Automatically rebase updates, then merge labels Jun 21, 2024
@dckc
Copy link
Member

dckc commented Jun 21, 2024

Note these are heap vows due to...

In provideOrchestration, we seem to make durable vowTools:

   const vowTools = prepareVowTools(zone.subZone('vows'));

Do we not pass them around to the relevant places?

@turadg
Copy link
Member Author

turadg commented Jun 21, 2024

Do we not pass them around to the relevant places?

They're available. See https://github.com/Agoric/agoric-sdk/pull/9547/files#r1648321242

@turadg
Copy link
Member Author

turadg commented Jun 21, 2024

@Mergifyio requeue

Copy link
Contributor

mergify bot commented Jun 21, 2024

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

looks to me like some tiny changes would make the vows durable

orchestration,
storageNode,
timer,
vowTools: { allVows },
Copy link
Member

Choose a reason for hiding this comment

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

If we get watch from these vowTools, it'll make durable vows, right?

@@ -1,13 +1,14 @@
/** @file ChainAccount exo */
import { makeTracer } from '@agoric/internal';
import { V } from '@agoric/vow/vat.js';
import { V, watch } from '@agoric/vow/vat.js';
Copy link
Member

Choose a reason for hiding this comment

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

How about getting watch from the durable vowTools? See below.

omni.makeAccount(),
agoric.makeAccount(),
// XXX when() until membrane
when(omni.makeAccount()),
Copy link
Member

Choose a reason for hiding this comment

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

The vowTools from the call to provideOrchestration are durable, right? How about getting when from there?

@@ -4,6 +4,7 @@ import { withdrawFromSeat } from '@agoric/zoe/src/contractSupport/zoeHelpers.js'
import { Far } from '@endo/far';
import { deeplyFulfilled } from '@endo/marshal';
import { M, objectMap } from '@endo/patterns';
import { when } from '@agoric/vow/vat.js';
Copy link
Member

Choose a reason for hiding this comment

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

see below re durable vowTools...

@@ -1,5 +1,5 @@
/** @file ChainAccount exo */
import { V } from '@agoric/vow/vat.js';
import { V, watch } from '@agoric/vow/vat.js';
Copy link
Member

Choose a reason for hiding this comment

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

again, a durable version is reasonably handy...

makeLocalOrchestrationAccountKit,
localchain,
storageNode,
vowTools: { allVows },
Copy link
Member

Choose a reason for hiding this comment

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

a durable watch is available in these vowTools, right?

@mergify mergify bot merged commit 788647d into master Jun 21, 2024
77 checks passed
@mergify mergify bot deleted the 9449-chain-facades branch June 21, 2024 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge force:integration Force integration tests to run on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants