add support for Vows to smart-wallet #9308

turadg · 2024-04-30T22:29:55Z

What is the Problem Being Solved?

Currently when an OfferResult is a promise, smartWallet awaits it with watchPromise to record the offer result. If, as in the case of an async-flow from orchestration, the result is a Vow for a primitive or for invitationMakers, the current smart wallet implementation will discard the Vow rather than handling what it fulfills to.

agoric-sdk/packages/smart-wallet/src/offerWatcher.js

Lines 30 to 40 in fbd8633

    
           /** 
        
            * @param {OutcomeWatchers} watchers 
        
            * @param {UserSeat} seat 
        
            */ 
        
           const watchForOfferResult = ({ resultWatcher }, seat) => { 
        
             const p = E(seat).getOfferResult(); 
        
             watchPromise(p, resultWatcher, seat); 
        
             return p; 
        
           };

Description of the Design

We could use watch in VowTools in place of the current watchPromises.

We might want to go further and use asyncFlow to refactor much of the smart wallet state handling.

Security Considerations

Scaling Considerations

Test Plan

Upgrade Considerations

The text was updated successfully, but these errors were encountered:

dckc · 2024-06-04T15:07:42Z

Currently when an OfferResult is a promise, smartWallet awaits it with watchPromise to record the offer result.

how is that a problem?

mhofman · 2024-06-04T15:11:03Z

how is that a problem?

watchPromise is not sufficient to handle vows as result

dckc · 2024-06-04T18:14:00Z

"handle" in what sense? "sufficient" in what sense? In what cases do we have vows as offer results?

mhofman · 2024-06-05T08:28:58Z

"handle" in what sense? "sufficient" in what sense? In what cases do we have vows as offer results?

An offer handler that wants to survive upgrade will likely return a vow. This will happen automatically if implementing the offer handler as an async-flow. Maybe the problem is wider than the smart wallet, I am not sufficiently familiar with offers and zoe, but a vow returned by the offer handler would not represent the final result. For that, the vow needs to be watched to get the final result (I believe it's already possible for an offer handler to return a value before the offer is considered "done" or whatever the terminology is there)

dckc · 2024-06-05T21:51:59Z

in particular, an async-flow that intends to result in invitationMakers needs support

- smart wallet does not know the right thing to do when it receives a vow. happy paths may 'seem' ok since the offer result is Vow object. For failure paths, the smart wallet thinks a Vow object holding a Promise rejection is a successful offer result - until #9308, this makes bootstrap tests pass

0xpatrickdev · 2024-06-27T21:14:26Z

This task blocks us on #9591 (#9449, part of async-flow integration) without making some compromises (disabling boot tests where offer handler failures are tested).

* Make smart wallet use `when` to accept a Vow for the result of an offer NOTE: this enables upgrade of the contract, NOT upgrade of the smart-wallet refs: #9308 Co-authored-by: Dean Tribble <tribble@agoric.com>

closes: #XXXX refs: #9449 refs: #9308 ## Description Changes in this PR are related to #9449, primarily for `local-orchestration-account.js` and `cosmos-orchestration-account.js`, `service.js`, and `orchestrator.js`. Attenuated versions of these kit are what the caller of `...getChain('agoric' | 'cosmos').makeAccount()` receives. This PR also includes a change to `smart-wallet` so it understands how to unwrap **offerResult**s that are **Vow**s. This uses **heapVowTools** which means it does _**not**_ cover resumability in the scenario of `smart-wallet` upgrading - only the contracts that rely on it. Other small changes included changes: - `PromiseToVow` and `VowifyAll` type helpers, so we can still reference the idealized API spec in code that returns Vows - fix/refactor of `getTimeoutTimestampNS` logic that removes an unnecessary network call when the caller supplies values - lint rule: adds a warning when importing `heapVowE` in resumable code - resumable lint rule changes to **error**, now that all warnings are resolved ### Security Considerations N/A ### Scaling Considerations N/A ### Documentation Considerations N/A ### Testing Considerations Adds a test boot and unit tests for a rejected Delegate wallet offer, which is helpful for ensuring errors are properly propagated through the vow chain. Upgrade tests would be helpful here, but we haven't started on these yet. I don't think we need those to land this PR, although they are certainly critical before release. Would love to pair with someone on this and learn more about how we do that. ### Upgrade Considerations This PR contains a change to **smart-wallet**.

- refs: #9308

refs: #9308 ## Description - adds `asPromise` helper to `VowTools` for unwrapping `Vow|Promise`, ensuring proper handling of ephemeral promises - updates watch-utils to better handle values that are not storable durably, such as promises ### Testing Considerations This does not include tests that simulate an upgrade - only a heap zone is used in the included tests. See #9631 ### Upgrade Considerations This PR includes changes we'd like to be in the initial release of vows.

0xpatrickdev · 2024-07-01T23:31:49Z

#9620 was revised to only include changes in @agoric/vow. The previous revision was preserved at pc/watch-utils-as-promise, and the diff was pulled out to 9308-smart-wallet-vows.

- refs: #9308

refs: #9308 ## Description - adds `asPromise` helper to `VowTools` for unwrapping `Vow|Promise`, ensuring proper handling of ephemeral promises - updates watch-utils to better handle values that are not storable durably, such as promises ### Testing Considerations This does not include tests that simulate an upgrade - only a heap zone is used in the included tests. See #9631 ### Upgrade Considerations This PR includes changes we'd like to be in the initial release of vows.

- refs: #9308 Co-authored-by: Turadg Aleahmad <turadg@agoric.com>

turadg added the enhancement New feature or request label Apr 30, 2024

erights added the asyncFlow related to membrane-based replay and upgrade of async functions label Jun 4, 2024

dckc added the wallet label Jun 5, 2024

turadg assigned dtribble Jun 17, 2024

0xpatrickdev mentioned this issue Jun 27, 2024

chore(orchestration): endowments are membrane-friendly #9591

Merged

dckc mentioned this issue Jun 27, 2024

Calypso - review of contract code (MN2 checklist) #8822

Open

LuqiPan mentioned this issue Jun 29, 2024

explore a design that would only require watched promises in the smart-wallet for returned vows in contract offer results #9616

Open

0xpatrickdev added a commit that referenced this issue Jun 30, 2024

feat(smart-wallet): use durable zone for vows

c2e3097

- refs: #9308

0xpatrickdev mentioned this issue Jun 30, 2024

feat(vows): improve handling of ephemeral values #9620

Merged

0xpatrickdev added a commit that referenced this issue Jul 1, 2024

feat(smart-wallet): use durable zone for vows

7373e55

- refs: #9308

0xpatrickdev added a commit that referenced this issue Jul 1, 2024

feat(smart-wallet): use durable zone for vows

e5b6971

- refs: #9308

turadg assigned turadg and unassigned dtribble Jul 1, 2024

turadg pushed a commit that referenced this issue Jul 2, 2024

feat(smart-wallet): use durable zone for vows

9d6b883

- refs: #9308

turadg mentioned this issue Jul 2, 2024

offer result vows in smart wallet #9635

Merged

turadg pushed a commit that referenced this issue Jul 2, 2024

feat(smart-wallet): use durable zone for vows

e7e3769

- refs: #9308

turadg added a commit that referenced this issue Jul 2, 2024

feat(smart-wallet): use durable zone for vows

46b8881

- refs: #9308 Co-authored-by: Turadg Aleahmad <turadg@agoric.com>

turadg added a commit that referenced this issue Jul 2, 2024

feat(smart-wallet): use durable zone for vows

ea22df6

- refs: #9308 Co-authored-by: Turadg Aleahmad <turadg@agoric.com>

turadg added a commit that referenced this issue Jul 3, 2024

feat(smart-wallet): use durable zone for vows

571b665

- refs: #9308 Co-authored-by: Turadg Aleahmad <turadg@agoric.com>

turadg added a commit that referenced this issue Jul 4, 2024

feat(smart-wallet): use durable zone for vows

b82e2b6

- refs: #9308 Co-authored-by: Turadg Aleahmad <turadg@agoric.com>

turadg added a commit that referenced this issue Jul 5, 2024

feat(smart-wallet): use durable zone for vows

1126e9f

- refs: #9308 Co-authored-by: Turadg Aleahmad <turadg@agoric.com>

mergify bot closed this as completed in #9635 Jul 5, 2024

mergify bot closed this as completed in 115adc4 Jul 5, 2024

mhofman mentioned this issue Jul 18, 2024

audit smart-wallet for any watchPromise during contract start #9738

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support for Vows to smart-wallet #9308

add support for Vows to smart-wallet #9308

turadg commented Apr 30, 2024 •

edited by dckc

Loading

dckc commented Jun 4, 2024

mhofman commented Jun 4, 2024 •

edited

Loading

dckc commented Jun 4, 2024

mhofman commented Jun 5, 2024

dckc commented Jun 5, 2024

0xpatrickdev commented Jun 27, 2024

0xpatrickdev commented Jul 1, 2024

add support for Vows to smart-wallet #9308

add support for Vows to smart-wallet #9308

Comments

turadg commented Apr 30, 2024 • edited by dckc Loading

What is the Problem Being Solved?

Description of the Design

Security Considerations

Scaling Considerations

Test Plan

Upgrade Considerations

dckc commented Jun 4, 2024

mhofman commented Jun 4, 2024 • edited Loading

dckc commented Jun 4, 2024

mhofman commented Jun 5, 2024

dckc commented Jun 5, 2024

0xpatrickdev commented Jun 27, 2024

0xpatrickdev commented Jul 1, 2024

turadg commented Apr 30, 2024 •

edited by dckc

Loading

mhofman commented Jun 4, 2024 •

edited

Loading