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

fix(async-flow): fix endowment equate bug #9736

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

erights
Copy link
Member

@erights erights commented Jul 18, 2024

closes: #XXXX
refs: #9722

Extracted from #9722 into an independently mergeable PR

Description

The failure at https://github.com/Agoric/agoric-sdk/pull/9719/files#diff-cbd091458bbd51ddc6640a3783cdb340d7f2588cb7ff27b330ba228cfbd0e18dR64-R87 revealed a bug introduced to async-flow when adding support for endowments. Because of the so-called "unwrapping" of some guests, there can be two guests corresponding to one host, with the host of course only mapping back to one of them -- the outer one. This makes bijection.js more complicated and irregular than an actual bijection.

equate(g, h) had a test for early return, if the g and h were already "equated", i.e., were corresponding guest and host. But the equate test was written before the elaboration of bijection. In fact, it should only test whether this guest g maps to the host h, irrespective of whether h maps back to this g.

Unfortunately:

This failure was not tested by any async-flow test before it failed for a user of async-flow.

  • This PR must add such a test before it becomes a candidate for merging

This PR should fix the failing test case https://github.com/Agoric/agoric-sdk/pull/9719/files#diff-cbd091458bbd51ddc6640a3783cdb340d7f2588cb7ff27b330ba228cfbd0e18dR64-R87 . Unfortunately, it does not. However, it now fails with different symptoms. Someone who understands #9719 better than I should investigate the nature of these new symptoms. If it still looks like it might be an async-flow problem, let me know!

Security Considerations

Scaling Considerations

Documentation Considerations

Testing Considerations

Upgrade Considerations

@erights erights self-assigned this Jul 18, 2024
@erights erights requested a review from mhofman July 18, 2024 22:17
Copy link

cloudflare-workers-and-pages bot commented Jul 18, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1b4f248
Status: ✅  Deploy successful!
Preview URL: https://5ae7a3d1.agoric-sdk.pages.dev
Branch Preview URL: https://markm-fix-async-flow-endowme-95hz.agoric-sdk.pages.dev

View logs

@erights erights force-pushed the markm-fix-async-flow-endowment-bug-2 branch from 6fb4df3 to 1b4f248 Compare July 22, 2024 01:05
@mhofman mhofman assigned mhofman and unassigned erights Jul 29, 2024
@mhofman mhofman added the asyncFlow related to membrane-based replay and upgrade of async functions label Jul 29, 2024
@mhofman mhofman linked an issue Aug 2, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
asyncFlow related to membrane-based replay and upgrade of async functions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

async-flow bijection is not always symmetric
2 participants