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

Implement an isPassable that does not rely on try/catch. #2096

Open
erights opened this issue Feb 21, 2024 · 0 comments
Open

Implement an isPassable that does not rely on try/catch. #2096

erights opened this issue Feb 21, 2024 · 0 comments
Assignees
Labels
bug Something isn't working

Comments

@erights
Copy link
Contributor

erights commented Feb 21, 2024

@agoric/base-zone has an implementation of isPassable that emulates the proper behavior using a try/catch to turn the thrown failure of passStyleOf into a return false;.

As of #2042 , that is duplicated into @endo/pass-style , as a first step of migrating isPassable from @agoric/base-zone into @endo/pass-style. Once synced, we should also delete the one in @agoric/base-zone and change its users to import from @endo/pass-style instead.

passStyleOf should be rewritten into the form of our other checkFoo functions to take a Checker argument, so it can be used to create either an assertPassable that throws or an isPassable that never throws but rather returns false iff assertPassable would have thrown. passStyleOf would then be like assertPassable except that the success case also returns a string.

This will require a significant refactoring of passStyleOf, which is already performance critical, so we'll need to be careful.

@erights erights added the bug Something isn't working label Feb 21, 2024
@erights erights self-assigned this Feb 21, 2024
@erights erights changed the title Implement an isPassable that does not rely on try/catch. Implement an isPassable that does not rely on try/catch. Apr 14, 2024
mergify bot added a commit to Agoric/agoric-sdk that referenced this issue Apr 30, 2024
closes: #XXXX
refs: endojs/endo#2096
endojs/endo#2042

## Description

Deleting @agoric/base-zone's implementation of `isPassable` completes
the migration of `isPassable` from @agoric/base-zone to @endo/pass-style
explained in endojs/endo#2096 and started in
endojs/endo#2042

The remaining issue explained in
endojs/endo#2096 , changing how `isPassable`
is implemented, remains to be done in @endo/pass-style. But this need no
longer concern us here since that difference will now be encapsulated
from us.

### Security Considerations
None
### Scaling Considerations

We know that `passStyleOf` remains a performance hotspot that needs
attention. This PR does not affect that at all. But I'll note that the
remaining suggested change from
endojs/endo#2096 --- to implement `isPassable`
and `passStyleOf` in terms of a more expressive internal function
parameterized by a checker --- might make this performance issue worse.
Just something to keep in mind as we tune `passStyleOf`. Attn @gibson042

### Documentation Considerations

none
### Testing Considerations

none
### Upgrade Considerations

As of this PR, @agoric/base-zone also no longer exports `isPassable`,
potentially breaking importers outside agoric-sdk until they are
modified to import it from @endo/pass-style as well. This PR does take
care of all such import sites within agoric-sdk.

If this turns out to be a problem in practice, this PR could be changed
to have @agoric/base-zone reexport the `isPassable` it imports from
@endo/pass-style, but deprecate that reexport, leaving it to future work
to change those old import sites outside agoric-sdk.

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant